(perf) also cache unresolved modules

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<SnapshotManager> {
         return (await this.getTSService(filePath)).snapshotManager;
     }

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<string, DocumentSnapshot> = new Map();
     private lastLogged = new Date(new Date().getTime() - 60_001);

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<void> {
-        const doneUpdateProjectFiles = new Set<SnapshotManager>();
+        const doneUpdateProjectFiles = new Set<LanguageServiceContainer>();
 
         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);

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<string, ts.ResolvedModule>();
+    private cache = new Map<string, ts.ResolvedModule | undefined>();
 
     /**
      * 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<ts.ResolvedModule | undefined> {
         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);

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() {

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)

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);
 });

packages/language-server/test/plugins/typescript/testfiles/diagnostics/unresolvedimport.svelte

@@ -0,0 +1,4 @@
+<script lang="ts">
+  import foo from './doesntexistyet';
+  foo;
+</script>