perf: use failed path to invalidate module cache (#2853)

jasonlyu123 · web-flow · commit ec7be4bf74e8 · 2025-09-19T14:25:26.000+02:00
* perf: use failed path to invalidate module cache

* Don't emit both document events in open
This makes the test do extra work after everything is done and creates weird race conditions, which I have already tried to fix many times

* changeset

* remove unnecessary Array.from
diff --git a/.changeset/dark-falcons-pump.md b/.changeset/dark-falcons-pump.md
@@ -0,0 +1,6 @@
+---
+'svelte-language-server': patch
+'svelte-check': patch
+---
+
+perf: more precise module cache invalidation
diff --git a/packages/language-server/package.json b/packages/language-server/package.json
@@ -10,7 +10,7 @@
         "./bin/server.js": "./bin/server.js"
     },
     "scripts": {
-        "test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.test.ts\"",
+        "test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.test.ts\" --no-experimental-strip-types",
         "build": "tsc",
         "prepublishOnly": "npm run build",
         "watch": "tsc -w"
diff --git a/packages/language-server/src/lib/documents/DocumentManager.ts b/packages/language-server/src/lib/documents/DocumentManager.ts
@@ -50,15 +50,14 @@ export class DocumentManager {
             // open state should only be updated when the document is closed
             document.openedByClient ||= openedByClient;
             document.setText(textDocument.text);
+            this.notify('documentChange', document);
         } else {
             document = this.createDocument(textDocument);
             document.openedByClient = openedByClient;
             this.documents.set(textDocument.uri, document);
             this.notify('documentOpen', document);
         }
 
-        this.notify('documentChange', document);
-
         return document;
     }
 
diff --git a/packages/language-server/src/plugins/css/CSSPlugin.ts b/packages/language-server/src/plugins/css/CSSPlugin.ts
@@ -99,9 +99,11 @@ export class CSSPlugin
             this.updateConfigs();
         });
 
-        docManager.on('documentChange', (document) =>
-            this.cssDocuments.set(document, new CSSDocument(document, this.cssLanguageServices))
-        );
+        const sync = (document: Document) => {
+            this.cssDocuments.set(document, new CSSDocument(document, this.cssLanguageServices));
+        };
+        docManager.on('documentChange', sync);
+        docManager.on('documentOpen', sync);
         docManager.on('documentClose', (document) => this.cssDocuments.delete(document));
     }
 
diff --git a/packages/language-server/src/plugins/html/HTMLPlugin.ts b/packages/language-server/src/plugins/html/HTMLPlugin.ts
@@ -76,9 +76,11 @@ export class HTMLPlugin
         configManager.onChange(() =>
             this.lang.setDataProviders(false, this.getCustomDataProviders())
         );
-        docManager.on('documentChange', (document) => {
+        const sync = (document: Document) => {
             this.documents.set(document, document.html);
-        });
+        };
+        docManager.on('documentChange', sync);
+        docManager.on('documentOpen', sync);
     }
 
     doHover(document: Document, position: Position): Hover | null {
diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts
@@ -7,23 +7,27 @@ import {
     ensureRealSvelteFilePath,
     getExtensionFromScriptKind,
     isSvelteFilePath,
-    isVirtualSvelteFilePath
+    isVirtualSvelteFilePath,
+    toVirtualSvelteFilePath
 } from './utils';
 
 const CACHE_KEY_SEPARATOR = ':::';
 /**
  * Caches resolved modules.
  */
 class ModuleResolutionCache {
-    private cache = new FileMap<ts.ResolvedModule | undefined>();
+    private cache = new FileMap<ts.ResolvedModuleWithFailedLookupLocations>();
     private pendingInvalidations = new FileSet();
     private getCanonicalFileName = createGetCanonicalFileName(ts.sys.useCaseSensitiveFileNames);
 
     /**
      * Tries to get a cached module.
      * Careful: `undefined` can mean either there's no match found, or that the result resolved to `undefined`.
      */
-    get(moduleName: string, containingFile: string): ts.ResolvedModule | undefined {
+    get(
+        moduleName: string,
+        containingFile: string
+    ): ts.ResolvedModuleWithFailedLookupLocations | undefined {
         return this.cache.get(this.getKey(moduleName, containingFile));
     }
 
@@ -37,7 +41,11 @@ class ModuleResolutionCache {
     /**
      * Caches resolved module (or undefined).
      */
-    set(moduleName: string, containingFile: string, resolvedModule: ts.ResolvedModule | undefined) {
+    set(
+        moduleName: string,
+        containingFile: string,
+        resolvedModule: ts.ResolvedModuleWithFailedLookupLocations
+    ) {
         this.cache.set(this.getKey(moduleName, containingFile), resolvedModule);
     }
 
@@ -48,7 +56,11 @@ class ModuleResolutionCache {
     delete(resolvedModuleName: string): void {
         resolvedModuleName = this.getCanonicalFileName(resolvedModuleName);
         this.cache.forEach((val, key) => {
-            if (val && this.getCanonicalFileName(val.resolvedFileName) === resolvedModuleName) {
+            if (
+                val.resolvedModule &&
+                this.getCanonicalFileName(val.resolvedModule.resolvedFileName) ===
+                    resolvedModuleName
+            ) {
                 this.cache.delete(key);
                 this.pendingInvalidations.add(key.split(CACHE_KEY_SEPARATOR).shift() || '');
             }
@@ -59,17 +71,10 @@ class ModuleResolutionCache {
      * Deletes everything from cache that resolved to `undefined`
      * and which might match the path.
      */
-    deleteUnresolvedResolutionsFromCache(path: string): void {
-        const fileNameWithoutEnding =
-            getLastPartOfPath(this.getCanonicalFileName(path)).split('.').shift() || '';
+    deleteByValues(list: ts.ResolvedModuleWithFailedLookupLocations[]): void {
         this.cache.forEach((val, key) => {
-            if (val) {
-                return;
-            }
-            const [containingFile, moduleName = ''] = key.split(CACHE_KEY_SEPARATOR);
-            if (moduleName.includes(fileNameWithoutEnding)) {
+            if (list.includes(val)) {
                 this.cache.delete(key);
-                this.pendingInvalidations.add(containingFile);
             }
         });
     }
@@ -181,13 +186,13 @@ export function createSvelteModuleLoader(
             svelteSys.deleteFromCache(path);
             moduleCache.delete(path);
         },
-        deleteUnresolvedResolutionsFromCache: (path: string) => {
+        scheduleResolutionFailedLocationCheck: (path: string) => {
             svelteSys.deleteFromCache(path);
-            moduleCache.deleteUnresolvedResolutionsFromCache(path);
-            pendingFailedLocationCheck.add(path);
-
-            tsModuleCache.clear();
-            typeReferenceCache.clear();
+            if (isSvelteFilePath(path)) {
+                pendingFailedLocationCheck.add(toVirtualSvelteFilePath(path));
+            } else {
+                pendingFailedLocationCheck.add(path);
+            }
         },
         resolveModuleNames,
         resolveTypeReferenceDirectiveReferences,
@@ -206,8 +211,9 @@ export function createSvelteModuleLoader(
         containingSourceFile?: ts.SourceFile | undefined
     ): Array<ts.ResolvedModule | undefined> {
         return moduleNames.map((moduleName, index) => {
-            if (moduleCache.has(moduleName, containingFile)) {
-                return moduleCache.get(moduleName, containingFile);
+            const cached = moduleCache.get(moduleName, containingFile);
+            if (cached) {
+                return cached.resolvedModule;
             }
 
             const resolvedModule = resolveModuleName(
@@ -221,7 +227,7 @@ export function createSvelteModuleLoader(
 
             cacheResolutionWithFailedLookup(resolvedModule, containingFile);
 
-            moduleCache.set(moduleName, containingFile, resolvedModule?.resolvedModule);
+            moduleCache.set(moduleName, containingFile, resolvedModule);
             return resolvedModule?.resolvedModule;
         });
     }
@@ -347,17 +353,15 @@ export function createSvelteModuleLoader(
     }
 
     function invalidateFailedLocationResolution() {
+        const toRemoves: ts.ResolvedModuleWithFailedLookupLocations[] = [];
         resolutionWithFailedLookup.forEach((resolvedModule) => {
-            if (
-                !resolvedModule.resolvedModule ||
-                !resolvedModule.files ||
-                !resolvedModule.failedLookupLocations
-            ) {
+            if (!resolvedModule.failedLookupLocations) {
                 return;
             }
+
             for (const location of resolvedModule.failedLookupLocations) {
                 if (pendingFailedLocationCheck.has(location)) {
-                    moduleCache.delete(resolvedModule.resolvedModule.resolvedFileName);
+                    toRemoves.push(resolvedModule);
                     resolvedModule.files?.forEach((file) => {
                         failedLocationInvalidated.add(file);
                     });
@@ -366,6 +370,16 @@ export function createSvelteModuleLoader(
             }
         });
 
+        if (toRemoves.length) {
+            moduleCache.deleteByValues(toRemoves);
+            resolutionWithFailedLookup.forEach((r) => {
+                if (toRemoves.includes(r)) {
+                    resolutionWithFailedLookup.delete(r);
+                }
+            });
+            tsModuleCache.clear();
+            tsTypeReferenceDirectiveCache.clear();
+        }
         pendingFailedLocationCheck.clear();
     }
 }
diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts
@@ -531,7 +531,7 @@ async function createLanguageService(
         for (const filePath of filePaths) {
             const normalizedPath = normalizePath(filePath);
             svelteModuleLoader.deleteFromModuleCache(normalizedPath);
-            svelteModuleLoader.deleteUnresolvedResolutionsFromCache(normalizedPath);
+            svelteModuleLoader.scheduleResolutionFailedLocationCheck(normalizedPath);
 
             scheduleUpdate(normalizedPath);
         }
@@ -559,7 +559,7 @@ async function createLanguageService(
         const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig);
 
         if (!prevSnapshot) {
-            svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath);
+            svelteModuleLoader.scheduleResolutionFailedLocationCheck(filePath);
             if (configFileForOpenFiles.get(filePath) === '' && services.size > 1) {
                 configFileForOpenFiles.delete(filePath);
             }
@@ -580,6 +580,7 @@ async function createLanguageService(
             return prevSnapshot;
         }
 
+        svelteModuleLoader.scheduleResolutionFailedLocationCheck(filePath);
         return createSnapshot(filePath);
     }
 
@@ -600,6 +601,8 @@ async function createLanguageService(
             return undefined;
         }
 
+        // don't invalidate the module cache here
+        // this only get called if we already know the file exists
         return createSnapshot(
             svelteModuleLoader.svelteFileExists(fileName) ? svelteFileName : fileName
         );
@@ -613,11 +616,12 @@ async function createLanguageService(
             return doc;
         }
 
+        // don't invalidate the module cache here
+        // this only get called if we already know the file exists
         return createSnapshot(fileName);
     }
 
     function createSnapshot(fileName: string) {
-        svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName);
         const doc = DocumentSnapshot.fromFilePath(
             fileName,
             docContext.createDocument,
@@ -730,7 +734,7 @@ async function createLanguageService(
 
     function updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void {
         if (!snapshotManager.has(fileName)) {
-            svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName);
+            svelteModuleLoader.scheduleResolutionFailedLocationCheck(fileName);
         }
         snapshotManager.updateTsOrJsFile(fileName, changes);
     }
diff --git a/packages/language-server/test/lib/documents/DocumentManager.test.ts b/packages/language-server/test/lib/documents/DocumentManager.test.ts
@@ -68,19 +68,6 @@ describe('Document Manager', () => {
         assert.throws(() => manager.updateDocument(textDocument, []));
     });
 
-    it('emits a document change event on open and update', () => {
-        const manager = new DocumentManager(createTextDocument);
-        const cb = sinon.spy();
-
-        manager.on('documentChange', cb);
-
-        manager.openClientDocument(textDocument);
-        sinon.assert.calledOnce(cb);
-
-        manager.updateDocument(textDocument, []);
-        sinon.assert.calledTwice(cb);
-    });
-
     it('update document in case-insensitive fs with different casing', () => {
         const textDocument: TextDocumentItem = {
             uri: 'file:///hello2.svelte',
diff --git a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts
@@ -92,6 +92,28 @@ describe('DiagnosticsProvider', function () {
         }
     }).timeout(this.timeout() * 2.5);
 
+    it('notices changes of module resolution because of new svelte file', async () => {
+        const { plugin, document, lsAndTsDocResolver } = setup('unresolvedimport2.svelte');
+
+        const diagnostics1 = await plugin.getDiagnostics(document);
+        assert.deepStrictEqual(diagnostics1.length, 1);
+
+        // back-and-forth-conversion normalizes slashes
+        const newSvelteFilePath = normalizePath(path.join(testDir, 'doesntexistyet.svelte')) || '';
+
+        writeFileSync(newSvelteFilePath, '<script lang="ts"></script>');
+        assert.ok(existsSync(newSvelteFilePath));
+        await lsAndTsDocResolver.invalidateModuleCache([newSvelteFilePath]);
+
+        try {
+            const diagnostics2 = await plugin.getDiagnostics(document);
+            assert.deepStrictEqual(diagnostics2.length, 0);
+            await lsAndTsDocResolver.deleteSnapshot(newSvelteFilePath);
+        } finally {
+            unlinkSync(newSvelteFilePath);
+        }
+    }).timeout(this.timeout() * 2.5);
+
     it('notices update of imported module', async () => {
         const { plugin, document, lsAndTsDocResolver } = setup(
             'diagnostics-imported-js-update.svelte'
diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport2.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport2.svelte
@@ -0,0 +1,4 @@
+<script lang="ts">
+  import foo from './doesntexistyet.svelte';
+  foo;
+</script>

Original file line number	Diff line number	Diff line change
`@@ -531,7 +531,7 @@ async function createLanguageService(`
`531`	`531`	`for (const filePath of filePaths) {`
`532`	`532`	`const normalizedPath = normalizePath(filePath);`
`533`	`533`	`svelteModuleLoader.deleteFromModuleCache(normalizedPath);`
`534`		`- svelteModuleLoader.deleteUnresolvedResolutionsFromCache(normalizedPath);`
	`534`	`+ svelteModuleLoader.scheduleResolutionFailedLocationCheck(normalizedPath);`
`535`	`535`
`536`	`536`	`scheduleUpdate(normalizedPath);`
`537`	`537`	`}`
`@@ -559,7 +559,7 @@ async function createLanguageService(`
`559`	`559`	`const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig);`
`560`	`560`
`561`	`561`	`if (!prevSnapshot) {`
`562`		`- svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath);`
	`562`	`+ svelteModuleLoader.scheduleResolutionFailedLocationCheck(filePath);`
`563`	`563`	`if (configFileForOpenFiles.get(filePath) === '' && services.size > 1) {`
`564`	`564`	`configFileForOpenFiles.delete(filePath);`
`565`	`565`	`}`
`@@ -580,6 +580,7 @@ async function createLanguageService(`
`580`	`580`	`return prevSnapshot;`
`581`	`581`	`}`
`582`	`582`
	`583`	`+ svelteModuleLoader.scheduleResolutionFailedLocationCheck(filePath);`
`583`	`584`	`return createSnapshot(filePath);`
`584`	`585`	`}`
`585`	`586`
`@@ -600,6 +601,8 @@ async function createLanguageService(`
`600`	`601`	`return undefined;`
`601`	`602`	`}`
`602`	`603`
	`604`	`+ // don't invalidate the module cache here`
	`605`	`+ // this only get called if we already know the file exists`
`603`	`606`	`return createSnapshot(`
`604`	`607`	`svelteModuleLoader.svelteFileExists(fileName) ? svelteFileName : fileName`
`605`	`608`	`);`
`@@ -613,11 +616,12 @@ async function createLanguageService(`
`613`	`616`	`return doc;`
`614`	`617`	`}`
`615`	`618`
	`619`	`+ // don't invalidate the module cache here`
	`620`	`+ // this only get called if we already know the file exists`
`616`	`621`	`return createSnapshot(fileName);`
`617`	`622`	`}`
`618`	`623`
`619`	`624`	`function createSnapshot(fileName: string) {`
`620`		`- svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName);`
`621`	`625`	`const doc = DocumentSnapshot.fromFilePath(`
`622`	`626`	`fileName,`
`623`	`627`	`docContext.createDocument,`
`@@ -730,7 +734,7 @@ async function createLanguageService(`
`730`	`734`
`731`	`735`	`function updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void {`
`732`	`736`	`if (!snapshotManager.has(fileName)) {`
`733`		`- svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName);`
	`737`	`+ svelteModuleLoader.scheduleResolutionFailedLocationCheck(fileName);`
`734`	`738`	`}`
`735`	`739`	`snapshotManager.updateTsOrJsFile(fileName, changes);`
`736`	`740`	`}`