diff --git a/.changeset/dark-falcons-pump.md b/.changeset/dark-falcons-pump.md new file mode 100644 index 000000000..6350244c8 --- /dev/null +++ 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 index 6cde35762..9574450ec 100644 --- 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 index 1c2df541b..28eda09ce 100644 --- a/packages/language-server/src/lib/documents/DocumentManager.ts +++ b/packages/language-server/src/lib/documents/DocumentManager.ts @@ -50,6 +50,7 @@ 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; @@ -57,8 +58,6 @@ export class DocumentManager { 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 index f89a6285e..edcaae746 100644 --- 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 index 2e900f626..f94cbe483 100644 --- 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 index a681c658a..8c293643c 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -7,7 +7,8 @@ import { ensureRealSvelteFilePath, getExtensionFromScriptKind, isSvelteFilePath, - isVirtualSvelteFilePath + isVirtualSvelteFilePath, + toVirtualSvelteFilePath } from './utils'; const CACHE_KEY_SEPARATOR = ':::'; @@ -15,7 +16,7 @@ const CACHE_KEY_SEPARATOR = ':::'; * Caches resolved modules. */ class ModuleResolutionCache { - private cache = new FileMap(); + private cache = new FileMap(); private pendingInvalidations = new FileSet(); private getCanonicalFileName = createGetCanonicalFileName(ts.sys.useCaseSensitiveFileNames); @@ -23,7 +24,10 @@ class ModuleResolutionCache { * 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 { 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 index cffba1ae0..5271a5224 100644 --- 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 index db5ad798e..833118cb0 100644 --- 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 index b6227bfb7..1d1a32f7c 100644 --- 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, ''); + 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 new file mode 100644 index 000000000..66ba21346 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport2.svelte @@ -0,0 +1,4 @@ + \ No newline at end of file