diff --git a/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts b/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts index e353856b4..024a37a9a 100644 --- a/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts +++ b/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts @@ -105,6 +105,9 @@ export class LSAndTSDocResolver { this.docManager.releaseDocument(pathToUrl(filePath)); } + /** + * @internal Public for tests only + */ async getSnapshotManager(filePath: string): Promise { return (await this.getTSService(filePath)).snapshotManager; } diff --git a/packages/language-server/src/plugins/typescript/SnapshotManager.ts b/packages/language-server/src/plugins/typescript/SnapshotManager.ts index 376323716..c91b84b92 100644 --- a/packages/language-server/src/plugins/typescript/SnapshotManager.ts +++ b/packages/language-server/src/plugins/typescript/SnapshotManager.ts @@ -8,6 +8,9 @@ export interface TsFilesSpec { exclude?: readonly string[]; } +/** + * Should only be used by `service.ts` + */ export class SnapshotManager { private documents: Map = new Map(); private lastLogged = new Date(new Date().getTime() - 60_001); diff --git a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts index a887afcc3..c07a6553a 100644 --- a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts +++ b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts @@ -60,7 +60,8 @@ import { SignatureHelpProviderImpl } from './features/SignatureHelpProvider'; import { UpdateImportsProviderImpl } from './features/UpdateImportsProvider'; import { isNoTextSpanInGeneratedCode, SnapshotFragmentMap } from './features/utils'; import { LSAndTSDocResolver } from './LSAndTSDocResolver'; -import { ignoredBuildDirectories, SnapshotManager } from './SnapshotManager'; +import { LanguageServiceContainer } from './service'; +import { ignoredBuildDirectories } from './SnapshotManager'; import { convertToLocationRange, getScriptKindFromFileName, symbolKindFromString } from './utils'; export class TypeScriptPlugin @@ -350,7 +351,7 @@ export class TypeScriptPlugin } async onWatchFileChanges(onWatchFileChangesParas: OnWatchFileChangesPara[]): Promise { - const doneUpdateProjectFiles = new Set(); + const doneUpdateProjectFiles = new Set(); for (const { fileName, changeType } of onWatchFileChangesParas) { const pathParts = fileName.split(/\/|\\/); @@ -365,19 +366,19 @@ export class TypeScriptPlugin continue; } - const snapshotManager = await this.getSnapshotManager(fileName); + const tsService = await this.lsAndTsDocResolver.getTSService(fileName); if (changeType === FileChangeType.Created) { - if (!doneUpdateProjectFiles.has(snapshotManager)) { - snapshotManager.updateProjectFiles(); - doneUpdateProjectFiles.add(snapshotManager); + if (!doneUpdateProjectFiles.has(tsService)) { + tsService.updateProjectFiles(); + doneUpdateProjectFiles.add(tsService); } } else if (changeType === FileChangeType.Deleted) { - snapshotManager.delete(fileName); - } else if (snapshotManager.has(fileName)) { + tsService.deleteSnapshot(fileName); + } else if (tsService.hasFile(fileName)) { // Only allow existing files to be update // Otherwise, new files would still get loaded // into snapshot manager after update - snapshotManager.updateTsOrJsFile(fileName); + tsService.updateTsOrJsFile(fileName); } } } @@ -428,8 +429,7 @@ export class TypeScriptPlugin } /** - * - * @internal + * @internal Public for tests only */ public getSnapshotManager(fileName: string) { return this.lsAndTsDocResolver.getSnapshotManager(fileName); diff --git a/packages/language-server/src/plugins/typescript/module-loader.ts b/packages/language-server/src/plugins/typescript/module-loader.ts index 16053100a..2801d59f1 100644 --- a/packages/language-server/src/plugins/typescript/module-loader.ts +++ b/packages/language-server/src/plugins/typescript/module-loader.ts @@ -1,32 +1,38 @@ import ts from 'typescript'; +import { getLastPartOfPath } from '../../utils'; +import { DocumentSnapshot } from './DocumentSnapshot'; +import { createSvelteSys } from './svelte-sys'; import { - isVirtualSvelteFilePath, ensureRealSvelteFilePath, - getExtensionFromScriptKind + getExtensionFromScriptKind, + isVirtualSvelteFilePath } from './utils'; -import { DocumentSnapshot } from './DocumentSnapshot'; -import { createSvelteSys } from './svelte-sys'; /** * Caches resolved modules. */ class ModuleResolutionCache { - private cache = new Map(); + private cache = new Map(); /** * 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 { return this.cache.get(this.getKey(moduleName, containingFile)); } /** - * Caches resolved module, if it is not undefined. + * Checks if has cached module. + */ + has(moduleName: string, containingFile: string): boolean { + return this.cache.has(this.getKey(moduleName, containingFile)); + } + + /** + * Caches resolved module (or undefined). */ set(moduleName: string, containingFile: string, resolvedModule: ts.ResolvedModule | undefined) { - if (!resolvedModule) { - return; - } this.cache.set(this.getKey(moduleName, containingFile), resolvedModule); } @@ -36,7 +42,21 @@ class ModuleResolutionCache { */ delete(resolvedModuleName: string): void { this.cache.forEach((val, key) => { - if (val.resolvedFileName === resolvedModuleName) { + if (val?.resolvedFileName === resolvedModuleName) { + this.cache.delete(key); + } + }); + } + + /** + * Deletes everything from cache that resolved to `undefined` + * and which might match the path. + */ + deleteUnresolvedResolutionsFromCache(path: string): void { + const fileNameWithoutEnding = getLastPartOfPath(path).split('.').shift() || ''; + this.cache.forEach((val, key) => { + const moduleName = key.split(':::').pop() || ''; + if (!val && moduleName.includes(fileNameWithoutEnding)) { this.cache.delete(key); } }); @@ -71,6 +91,8 @@ export function createSvelteModuleLoader( readFile: svelteSys.readFile, readDirectory: svelteSys.readDirectory, deleteFromModuleCache: (path: string) => moduleCache.delete(path), + deleteUnresolvedResolutionsFromCache: (path: string) => + moduleCache.deleteUnresolvedResolutionsFromCache(path), resolveModuleNames }; @@ -79,9 +101,8 @@ export function createSvelteModuleLoader( containingFile: string ): Array { return moduleNames.map((moduleName) => { - const cachedModule = moduleCache.get(moduleName, containingFile); - if (cachedModule) { - return cachedModule; + if (moduleCache.has(moduleName, containingFile)) { + return moduleCache.get(moduleName, containingFile); } const resolvedModule = resolveModuleName(moduleName, containingFile); diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index a5d277278..117c1e03f 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -1,21 +1,32 @@ import { dirname, resolve } from 'path'; import ts from 'typescript'; +import { TextDocumentContentChangeEvent } from 'vscode-languageserver-protocol'; +import { getPackageInfo } from '../../importPackage'; import { Document } from '../../lib/documents'; +import { configLoader } from '../../lib/documents/configLoader'; import { Logger } from '../../logger'; -import { getPackageInfo } from '../../importPackage'; import { DocumentSnapshot } from './DocumentSnapshot'; import { createSvelteModuleLoader } from './module-loader'; import { ignoredBuildDirectories, SnapshotManager } from './SnapshotManager'; import { ensureRealSvelteFilePath, findTsConfigPath } from './utils'; -import { configLoader } from '../../lib/documents/configLoader'; export interface LanguageServiceContainer { readonly tsconfigPath: string; readonly compilerOptions: ts.CompilerOptions; + /** + * @internal Public for tests only + */ readonly snapshotManager: SnapshotManager; getService(): ts.LanguageService; updateSnapshot(documentOrFilePath: Document | string): DocumentSnapshot; deleteSnapshot(filePath: string): void; + updateProjectFiles(): void; + updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void; + /** + * Checks if a file is present in the project. + * Unlike `fileBelongsToProject`, this doesn't run a file search on disk. + */ + hasFile(filePath: string): boolean; /** * Careful, don't call often, or it will hurt performance. * Only works for TS versions that have ScriptKind.Deferred @@ -131,6 +142,9 @@ async function createLanguageService( getService: () => languageService, updateSnapshot, deleteSnapshot, + updateProjectFiles, + updateTsOrJsFile, + hasFile, fileBelongsToProject, snapshotManager }; @@ -153,6 +167,10 @@ async function createLanguageService( return prevSnapshot; } + if (!prevSnapshot) { + svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath); + } + const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig); snapshotManager.set(filePath, newSnapshot); @@ -171,6 +189,7 @@ async function createLanguageService( return prevSnapshot; } + svelteModuleLoader.deleteUnresolvedResolutionsFromCache(filePath); const newSnapshot = DocumentSnapshot.fromFilePath( filePath, docContext.createDocument, @@ -188,6 +207,7 @@ async function createLanguageService( return doc; } + svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName); doc = DocumentSnapshot.fromFilePath( fileName, docContext.createDocument, @@ -197,8 +217,23 @@ async function createLanguageService( return doc; } + function updateProjectFiles(): void { + snapshotManager.updateProjectFiles(); + } + + function hasFile(filePath: string): boolean { + return snapshotManager.has(filePath); + } + function fileBelongsToProject(filePath: string): boolean { - return snapshotManager.has(filePath) || getParsedConfig().fileNames.includes(filePath); + return hasFile(filePath) || getParsedConfig().fileNames.includes(filePath); + } + + function updateTsOrJsFile(fileName: string, changes?: TextDocumentContentChangeEvent[]): void { + if (!snapshotManager.has(fileName)) { + svelteModuleLoader.deleteUnresolvedResolutionsFromCache(fileName); + } + snapshotManager.updateTsOrJsFile(fileName, changes); } function getParsedConfig() { diff --git a/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts index a1a79e21a..52f16378b 100644 --- a/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts @@ -30,7 +30,7 @@ const fileNameToAbsoluteUri = (file: string) => { return pathToUrl(join(testFilesDir, file)); }; -describe.only('CompletionProviderImpl', () => { +describe('CompletionProviderImpl', () => { function setup(filename: string) { const docManager = new DocumentManager( (textDocument) => new Document(textDocument.uri, textDocument.text) 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 becbc1d16..10b738d57 100644 --- a/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/DiagnosticsProvider.test.ts @@ -1,11 +1,12 @@ import * as assert from 'assert'; +import { existsSync, unlinkSync, writeFileSync } from 'fs'; import * as path from 'path'; import ts from 'typescript'; import { Document, DocumentManager } from '../../../../src/lib/documents'; import { LSConfigManager } from '../../../../src/ls-config'; import { DiagnosticsProviderImpl } from '../../../../src/plugins/typescript/features/DiagnosticsProvider'; import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver'; -import { pathToUrl } from '../../../../src/utils'; +import { pathToUrl, urlToPath } from '../../../../src/utils'; const testDir = path.join(__dirname, '..', 'testfiles', 'diagnostics'); @@ -25,7 +26,7 @@ describe('DiagnosticsProvider', () => { uri: pathToUrl(filePath), text: ts.sys.readFile(filePath) || '' }); - return { plugin, document, docManager }; + return { plugin, document, docManager, lsAndTsDocResolver }; } it('provides diagnostics', async () => { @@ -936,4 +937,28 @@ describe('DiagnosticsProvider', () => { const diagnostics = await plugin.getDiagnostics(document); assert.deepStrictEqual(diagnostics, []); }); + + it('notices creation and deletion of imported module', async () => { + const { plugin, document, lsAndTsDocResolver } = setup('unresolvedimport.svelte'); + + const diagnostics1 = await plugin.getDiagnostics(document); + assert.deepStrictEqual(diagnostics1.length, 1); + + // back-and-forth-conversion normalizes slashes + const newFilePath = urlToPath(pathToUrl(path.join(testDir, 'doesntexistyet.js'))) || ''; + writeFileSync(newFilePath, 'export default function foo() {}'); + assert.ok(existsSync(newFilePath)); + await lsAndTsDocResolver.getSnapshot(newFilePath); + + try { + const diagnostics2 = await plugin.getDiagnostics(document); + assert.deepStrictEqual(diagnostics2.length, 0); + lsAndTsDocResolver.deleteSnapshot(newFilePath); + } finally { + unlinkSync(newFilePath); + } + + const diagnostics3 = await plugin.getDiagnostics(document); + assert.deepStrictEqual(diagnostics3.length, 1); + }).timeout(5000); }); diff --git a/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport.svelte b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport.svelte new file mode 100644 index 000000000..03cc6a4b0 --- /dev/null +++ b/packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport.svelte @@ -0,0 +1,4 @@ + \ No newline at end of file