Avoid storing duplicate SourceFile in createVirtualTypeScriptEnvironment (#3614)

packages/typescript-vfs/src/index.ts

@@ -14,8 +14,8 @@ interface LocalStorageLike {
   removeItem(key: string): void
 }
 
-declare var localStorage: LocalStorageLike | undefined;
-declare var fetch: FetchLike | undefined;
+declare var localStorage: LocalStorageLike | undefined
+declare var fetch: FetchLike | undefined
 
 const hasProcess = typeof process !== `undefined`
 
@@ -25,14 +25,13 @@ const hasProcess = typeof process !== `undefined`
 // module evaluation for every consumer, so skip the probe entirely in a bare Node
 // process, where `process.env.DEBUG` below is the only reachable way to opt in anyway.
 // DOM-bearing hosts that also expose `process` (Electron renderers) still get probed.
-const isBareNodeProcess =
-  hasProcess && typeof process.versions?.node === `string` && !(`window` in globalThis)
+const isBareNodeProcess = hasProcess && typeof process.versions?.node === `string` && !(`window` in globalThis)
 
 let hasLocalStorage = false
 if (!isBareNodeProcess) {
   try {
     hasLocalStorage = typeof localStorage !== `undefined` && typeof localStorage.getItem === `function`
-  } catch (error) { }
+  } catch (error) {}
 }
 
 const shouldDebug = (hasLocalStorage && localStorage!.getItem("DEBUG")) || (hasProcess && process.env.DEBUG)
@@ -90,7 +89,7 @@ export function createVirtualTypeScriptEnvironment(
     getSourceFile: fileName => languageService.getProgram()?.getSourceFile(fileName),
 
     createFile: (fileName, content) => {
-      updateFile(ts.createSourceFile(fileName, content, mergedCompilerOpts.target!, false))
+      updateFile({ fileName, text: content })
     },
     updateFile: (fileName, content, optPrevTextSpan) => {
       const prevSourceFile = languageService.getProgram()!.getSourceFile(fileName)
@@ -105,19 +104,12 @@ export function createVirtualTypeScriptEnvironment(
         prevFullContents.slice(0, prevTextSpan.start) +
         content +
         prevFullContents.slice(prevTextSpan.start + prevTextSpan.length)
-      const newSourceFile = ts.updateSourceFile(prevSourceFile, newText, {
-        span: prevTextSpan,
-        newLength: content.length,
-      })
 
-      updateFile(newSourceFile)
+      updateFile({ fileName, text: newText })
     },
     deleteFile(fileName) {
-      const sourceFile = languageService.getProgram()!.getSourceFile(fileName)
-      if (sourceFile) {
-        deleteFile(sourceFile)
-      }
-    }
+      deleteFile(fileName)
+    },
   }
 }
 
@@ -262,7 +254,7 @@ export const knownLibFilesForCompilerOptions = (compilerOptions: CompilerOptions
     "lib.esnext.symbol.d.ts",
     "lib.esnext.temporal.d.ts",
     "lib.esnext.typedarrays.d.ts",
-    "lib.esnext.weakref.d.ts"
+    "lib.esnext.weakref.d.ts",
   ]
 
   const targetToCut = ts.ScriptTarget[target]
@@ -407,7 +399,7 @@ export const createDefaultMapFromCDN = (
           contents.forEach((text, index) => fsMap.set("/" + files[index], text))
         })
         // Return a NOOP for .d.ts files which aren't in the current build of TypeScript
-        .catch(() => { })
+        .catch(() => {})
     )
   }
 
@@ -438,7 +430,7 @@ export const createDefaultMapFromCDN = (
                 return t
               })
               // Return a NOOP for .d.ts files which aren't in the current build of TypeScript
-              .catch(() => { })
+              .catch(() => {})
           )
         } else {
           return Promise.resolve(unzip(content))
@@ -488,11 +480,13 @@ const defaultCompilerOptions = (ts: typeof import("typescript")): CompilerOption
     suppressOutputPathCheck: true,
     skipLibCheck: true,
     skipDefaultLibCheck: true,
-    ...(ts.versionMajorMinor && Number(ts.versionMajorMinor.split(".")[0]) >= 6 ? {
-      ignoreDeprecations: "6.0" ,
-    } : {
-      moduleResolution: ts.ModuleResolutionKind.NodeJs,
-    }),
+    ...(ts.versionMajorMinor && Number(ts.versionMajorMinor.split(".")[0]) >= 6
+      ? {
+          ignoreDeprecations: "6.0",
+        }
+      : {
+          moduleResolution: ts.ModuleResolutionKind.NodeJs,
+        }),
   }
 }
 
@@ -525,7 +519,7 @@ export function createSystem(files: Map<string, string>): System {
     writeFile: (fileName, contents) => {
       files.set(fileName, contents)
     },
-    deleteFile: (fileName) => {
+    deleteFile: fileName => {
       files.delete(fileName)
     },
   }
@@ -607,7 +601,7 @@ export function createFSBackedSystem(
     writeFile: (fileName, contents) => {
       files.set(fileName, contents)
     },
-    deleteFile: (fileName) => {
+    deleteFile: fileName => {
       files.delete(fileName)
     },
     realpath: nodeSys.realpath,
@@ -665,7 +659,7 @@ export function createVirtualCompilerHost(sys: System, compilerOptions: Compiler
       sourceFiles.delete(sourceFile.fileName)
       sys.deleteFile!(sourceFile.fileName)
       return alreadyExists
-    }
+    },
   }
   return vHost
 }
@@ -681,11 +675,13 @@ export function createVirtualLanguageServiceHost(
   customTransformers?: CustomTransformers
 ) {
   const fileNames = [...rootFiles]
-  const { compilerHost, updateFile, deleteFile } = createVirtualCompilerHost(sys, compilerOptions, ts)
   const fileVersions = new Map<string, string>()
   let projectVersion = 0
   const languageServiceHost: LanguageServiceHost = {
-    ...compilerHost,
+    ...sys,
+    getDefaultLibFileName: options => "/" + ts.getDefaultLibFileName(options || compilerOptions),
+    getNewLine: () => sys.newLine,
+    useCaseSensitiveFileNames: () => sys.useCaseSensitiveFileNames,
     getProjectVersion: () => projectVersion.toString(),
     getCompilationSettings: () => compilerOptions,
     getCustomTransformers: () => customTransformers,
@@ -713,8 +709,8 @@ export function createVirtualLanguageServiceHost(
 
   type Return = {
     languageServiceHost: LanguageServiceHost
-    updateFile: (sourceFile: import("typescript").SourceFile) => void
-    deleteFile: (sourceFile: import("typescript").SourceFile) => void
+    updateFile: (sourceFile: import("typescript").SourceFile | { fileName: string; text: string }) => void
+    deleteFile: (sourceFile: import("typescript").SourceFile | { fileName: string } | string) => void
   }
 
   const lsHost: Return = {
@@ -725,17 +721,18 @@ export function createVirtualLanguageServiceHost(
       if (!fileNames.includes(sourceFile.fileName)) {
         fileNames.push(sourceFile.fileName)
       }
-      updateFile(sourceFile)
+      sys.writeFile(sourceFile.fileName, sourceFile.text)
     },
     deleteFile: sourceFile => {
+      const fileName = typeof sourceFile === "string" ? sourceFile : sourceFile.fileName
       projectVersion++
-      fileVersions.set(sourceFile.fileName, projectVersion.toString())
-      const index = fileNames.indexOf(sourceFile.fileName)
+      fileVersions.set(fileName, projectVersion.toString())
+      const index = fileNames.indexOf(fileName)
       if (index !== -1) {
         fileNames.splice(index, 1)
       }
-      deleteFile(sourceFile)
-    }
+      sys.deleteFile?.(fileName)
+    },
   }
   return lsHost
 }

packages/typescript-vfs/test/fsbacked.test.ts

@@ -88,11 +88,10 @@ it("can delete files in the virtual fs", () => {
 
   const system = createFSBackedSystem(fsMap, monorepoRoot, ts)
   const env = createVirtualTypeScriptEnvironment(system, [index, exporter], ts, compilerOpts)
-  
+
   expect(env.getSourceFile(index)).toBeTruthy()
 
-  env.deleteFile(index);
+  env.deleteFile(index)
 
   expect(env.getSourceFile(index)).toBeFalsy()
-
 })

packages/typescript-vfs/test/index.test.ts

@@ -5,6 +5,7 @@ import {
   createDefaultMapFromCDN,
   knownLibFilesForCompilerOptions,
   createVirtualCompilerHost,
+  createVirtualLanguageServiceHost,
 } from "../src"
 
 import ts from "typescript"
@@ -243,5 +244,109 @@ it("moduleDetection options", async () => {
     host: host.compilerHost,
   })
   program.emit()
-  expect(fsMap.get("index.js")).toEqual(`define(["require", "exports"], function (require, exports) {\n    "use strict";\n    Object.defineProperty(exports, "__esModule", { value: true });\n    let foo = 'foo';\n});\n`)
+  expect(fsMap.get("index.js")).toEqual(
+    `define(["require", "exports"], function (require, exports) {\n    "use strict";\n    Object.defineProperty(exports, "__esModule", { value: true });\n    let foo = 'foo';\n});\n`
+  )
+})
+
+it("does not parse duplicate SourceFiles on createFile and updateFile", () => {
+  let createSourceFileCalls = 0
+  let updateSourceFileCalls = 0
+  const tsProxy = new Proxy(ts, {
+    get(target, prop) {
+      if (prop === "createSourceFile") {
+        return (...args: any[]) => {
+          createSourceFileCalls++
+          return (target as any).createSourceFile(...args)
+        }
+      }
+      if (prop === "updateSourceFile") {
+        return (...args: any[]) => {
+          updateSourceFileCalls++
+          return (target as any).updateSourceFile(...args)
+        }
+      }
+      return (target as any)[prop]
+    },
+  })
+
+  const fsMap = new Map<string, string>()
+  const system = createSystem(fsMap)
+  const env = createVirtualTypeScriptEnvironment(system, [], tsProxy, {})
+
+  // Creating a file should not invoke ts.createSourceFile directly
+  env.createFile("sample.ts", "const message = 'hello';")
+  expect(createSourceFileCalls).toBe(0)
+
+  // Language service parses and returns the source file on demand
+  const sf = env.getSourceFile("sample.ts")
+  expect(sf).toBeDefined()
+  expect(sf?.text).toBe("const message = 'hello';")
+
+  // Updating a file should not invoke ts.updateSourceFile directly
+  env.updateFile("sample.ts", "const message = 'world';")
+  expect(updateSourceFileCalls).toBe(0)
+
+  // Language service returns the updated source file on demand
+  const updatedSf = env.getSourceFile("sample.ts")
+  expect(updatedSf).toBeDefined()
+  expect(updatedSf?.text).toBe("const message = 'world';")
+
+  // Deleting a file removes it from both env and system
+  env.deleteFile("sample.ts")
+  expect(env.getSourceFile("sample.ts")).toBeUndefined()
+  expect(system.fileExists("sample.ts")).toBe(false)
+})
+
+it("supports updateFile with replaceTextSpan without creating duplicate SourceFiles", () => {
+  let updateSourceFileCalls = 0
+  const tsProxy = new Proxy(ts, {
+    get(target, prop) {
+      if (prop === "updateSourceFile") {
+        return (...args: any[]) => {
+          updateSourceFileCalls++
+          return (target as any).updateSourceFile(...args)
+        }
+      }
+      return (target as any)[prop]
+    },
+  })
+
+  const fsMap = new Map<string, string>()
+  const system = createSystem(fsMap)
+  const env = createVirtualTypeScriptEnvironment(system, [], tsProxy, {})
+
+  env.createFile("test.ts", "const val = 123;")
+  expect(env.getSourceFile("test.ts")?.text).toBe("const val = 123;")
+
+  // Replace '123' with '456'
+  const span = ts.createTextSpan(12, 3)
+  env.updateFile("test.ts", "456", span)
+  expect(updateSourceFileCalls).toBe(0)
+  expect(env.getSourceFile("test.ts")?.text).toBe("const val = 456;")
+})
+
+it("allows updating and deleting files via createVirtualLanguageServiceHost", () => {
+  const fsMap = new Map<string, string>()
+  const system = createSystem(fsMap)
+  const lsHost = createVirtualLanguageServiceHost(system, ["init.ts"], {}, ts)
+
+  // Can update using a SourceFile or { fileName, text }
+  lsHost.updateFile({ fileName: "file1.ts", text: "const x = 1;" })
+  expect(system.readFile("file1.ts")).toBe("const x = 1;")
+  expect(lsHost.languageServiceHost.getScriptFileNames()).toContain("file1.ts")
+
+  const sf = ts.createSourceFile("file2.ts", "const y = 2;", ts.ScriptTarget.ES2015)
+  lsHost.updateFile(sf)
+  expect(system.readFile("file2.ts")).toBe("const y = 2;")
+  expect(lsHost.languageServiceHost.getScriptFileNames()).toContain("file2.ts")
+
+  // Can delete using SourceFile, object or string
+  lsHost.deleteFile("file1.ts")
+  expect(lsHost.languageServiceHost.getScriptFileNames()).not.toContain("file1.ts")
+  expect(system.fileExists("file1.ts")).toBe(false)
+
+  lsHost.deleteFile(sf)
+  expect(lsHost.languageServiceHost.getScriptFileNames()).not.toContain("file2.ts")
+  expect(system.fileExists("file2.ts")).toBe(false)
 })