(fix) only allow client files and project files to be root (#2146)

#2132 This PR moved the tracking of "document is open by the client" to document so it can be copied over to ts snapshot and create a new documentManager.openClientDocument to replace openDoucment in existing places. This will mark the file as client files in one method call, so we don't have to add markAsOpenByClient to all existing tests. The reason that we need client files is that files might be opened by the client and not in the project files. One reason is that the user has an empty include in the tsconfig or has the default one. Also, we didn't watch Svelte files with File Watcher. So the new svelte file won't be in the project files. We probably should do it for something like git checkout, but that can be a separate PR. We have to use a TypeScript internal API and add a layer of synchronization because of a module resolution bug. The way getSnapshot prevents the problem before the amount of root files have changed so that TypeScript will check more thoroughly. The existing test is different from how it'll work in typical cases. It probably worked when it was introduced, but we later changed to not run getSnapshot in new ts/js files, so it's not actually preventing the error.
sveltejs · Sep 11, 2023 · c8ae56e · c8ae56e
1 parent 6c76ba8
commit c8ae56e
Show file tree

Hide file tree

Showing 41 changed files with 346 additions and 106 deletions.
diff --git a/packages/language-server/src/lib/documents/Document.ts b/packages/language-server/src/lib/documents/Document.ts
@@ -18,6 +18,7 @@ export class Document extends WritableDocument {
     configPromise: Promise<SvelteConfig | undefined>;
     config?: SvelteConfig;
     html!: HTMLDocument;
+    openedByClient = false;
     /**
      * Compute and cache directly because of performance reasons
      * and it will be called anyway.

diff --git a/packages/language-server/src/lib/documents/DocumentManager.ts b/packages/language-server/src/lib/documents/DocumentManager.ts
@@ -16,7 +16,6 @@ export type DocumentEvent = 'documentOpen' | 'documentChange' | 'documentClose';
  */
 export class DocumentManager {
     private emitter = new EventEmitter();
-    private openedInClient: FileSet;
     private documents: FileMap<Document>;
     private locked: FileSet;
     private deleteCandidates: FileSet;
@@ -27,13 +26,19 @@ export class DocumentManager {
             useCaseSensitiveFileNames: ts.sys.useCaseSensitiveFileNames
         }
     ) {
-        this.openedInClient = new FileSet(options.useCaseSensitiveFileNames);
         this.documents = new FileMap(options.useCaseSensitiveFileNames);
         this.locked = new FileSet(options.useCaseSensitiveFileNames);
         this.deleteCandidates = new FileSet(options.useCaseSensitiveFileNames);
     }
 
-    openDocument(textDocument: Pick<TextDocumentItem, 'text' | 'uri'>): Document {
+    openClientDocument(textDocument: Pick<TextDocumentItem, 'text' | 'uri'>): Document {
+        return this.openDocument(textDocument, /**openedByClient */ true);
+    }
+
+    openDocument(
+        textDocument: Pick<TextDocumentItem, 'text' | 'uri'>,
+        openedByClient: boolean
+    ): Document {
         textDocument = {
             ...textDocument,
             uri: normalizeUri(textDocument.uri)
@@ -50,6 +55,7 @@ export class DocumentManager {
         }
 
         this.notify('documentChange', document);
+        document.openedByClient = openedByClient;
 
         return document;
     }
@@ -59,24 +65,29 @@ export class DocumentManager {
     }
 
     markAsOpenedInClient(uri: string): void {
-        this.openedInClient.add(normalizeUri(uri));
+        const document = this.documents.get(normalizeUri(uri));
+        if (document) {
+            document.openedByClient = true;
+        }
     }
 
     getAllOpenedByClient() {
-        return Array.from(this.documents.entries()).filter((doc) =>
-            this.openedInClient.has(doc[0])
-        );
+        return Array.from(this.documents.entries()).filter((doc) => doc[1].openedByClient);
     }
 
     isOpenedInClient(uri: string) {
-        return this.openedInClient.has(normalizeUri(uri));
+        const document = this.documents.get(normalizeUri(uri));
+        return !!document?.openedByClient;
     }
 
     releaseDocument(uri: string): void {
         uri = normalizeUri(uri);
 
         this.locked.delete(uri);
-        this.openedInClient.delete(uri);
+        const document = this.documents.get(uri);
+        if (document) {
+            document.openedByClient = false;
+        }
         if (this.deleteCandidates.has(uri)) {
             this.deleteCandidates.delete(uri);
             this.closeDocument(uri);
@@ -100,7 +111,7 @@ export class DocumentManager {
             this.deleteCandidates.add(uri);
         }
 
-        this.openedInClient.delete(uri);
+        document.openedByClient = false;
     }
 
     updateDocument(

diff --git a/packages/language-server/src/lib/documents/fileCollection.ts b/packages/language-server/src/lib/documents/fileCollection.ts
@@ -91,6 +91,10 @@ export class FileSet implements Iterable<string> {
         return this.set.delete(this.getCanonicalFileName(filePath));
     }
 
+    clear() {
+        this.set.clear();
+    }
+
     [Symbol.iterator](): Iterator<string> {
         return this.set[Symbol.iterator]();
     }

diff --git a/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts b/packages/language-server/src/plugins/typescript/DocumentSnapshot.ts
@@ -61,6 +61,7 @@ export interface DocumentSnapshot extends ts.IScriptSnapshot, DocumentMapper {
      * Convenience function for getText(0, getLength())
      */
     getFullText(): string;
+    isOpenedInClient(): boolean;
 }
 
 /**
@@ -372,6 +373,10 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
         return this.url;
     }
 
+    isOpenedInClient() {
+        return this.parent.openedByClient;
+    }
+
     private getLineOffsets() {
         if (!this.lineOffsets) {
             this.lineOffsets = getLineOffsets(this.text);
@@ -428,6 +433,12 @@ export class JSOrTSDocumentSnapshot extends IdentityMapper implements DocumentSn
     private serverHooksPath = 'src/hooks.server';
     private clientHooksPath = 'src/hooks.client';
 
+    private openedByClient = false;
+
+    isOpenedInClient(): boolean {
+        return this.openedByClient;
+    }
+
     constructor(public version: number, public readonly filePath: string, private text: string) {
         super(pathToUrl(filePath));
         this.adjustText();
@@ -516,6 +527,8 @@ export class JSOrTSDocumentSnapshot extends IdentityMapper implements DocumentSn
         this.version++;
         this.lineOffsets = undefined;
         this.internalLineOffsets = undefined;
+        // only client can have incremental updates
+        this.openedByClient = true;
     }
 
     protected getLineOffsets() {

diff --git a/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts b/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts
@@ -78,10 +78,13 @@ export class LSAndTSDocResolver {
      */
     private createDocument = (fileName: string, content: string) => {
         const uri = pathToUrl(fileName);
-        const document = this.docManager.openDocument({
-            text: content,
-            uri
-        });
+        const document = this.docManager.openDocument(
+            {
+                text: content,
+                uri
+            },
+            /* openedByClient */ false
+        );
         this.docManager.lockDocument(uri);
         return document;
     };
@@ -116,11 +119,31 @@ export class LSAndTSDocResolver {
         lang: ts.LanguageService;
         userPreferences: ts.UserPreferences;
     }> {
-        const lang = await this.getLSForPath(document.getFilePath() || '');
+        const { tsDoc, lsContainer, userPreferences } = await this.getLSAndTSDocWorker(document);
+
+        return { tsDoc, lang: lsContainer.getService(), userPreferences };
+    }
+
+    /**
+     * Retrieves the LS for operations that don't need cross-files information.
+     * can save some time by not synchronizing languageService program
+     */
+    async getLsForSyntheticOperations(document: Document): Promise<{
+        tsDoc: SvelteDocumentSnapshot;
+        lang: ts.LanguageService;
+        userPreferences: ts.UserPreferences;
+    }> {
+        const { tsDoc, lsContainer, userPreferences } = await this.getLSAndTSDocWorker(document);
+
+        return { tsDoc, userPreferences, lang: lsContainer.getService(/* skipSynchronize */ true) };
+    }
+
+    private async getLSAndTSDocWorker(document: Document) {
+        const lsContainer = await this.getTSService(document.getFilePath() || '');
         const tsDoc = await this.getSnapshot(document);
         const userPreferences = this.getUserPreferences(tsDoc);
 
-        return { tsDoc, lang, userPreferences };
+        return { tsDoc, lsContainer, userPreferences };
     }
 
     /**
@@ -161,6 +184,10 @@ export class LSAndTSDocResolver {
         this.docManager.releaseDocument(uri);
     }
 
+    async invalidateModuleCache(filePath: string) {
+        await forAllServices((service) => service.invalidateModuleCache(filePath));
+    }
+
     /**
      * Updates project files in all existing ts services
      */

diff --git a/packages/language-server/src/plugins/typescript/SnapshotManager.ts b/packages/language-server/src/plugins/typescript/SnapshotManager.ts
@@ -225,8 +225,10 @@ export class SnapshotManager {
         this.globalSnapshotsManager.delete(fileName);
     }
 
-    getFileNames(): string[] {
-        return Array.from(this.documents.entries()).map(([_, doc]) => doc.filePath);
+    getClientFileNames(): string[] {
+        return Array.from(this.documents.values())
+            .filter((doc) => doc.isOpenedInClient())
+            .map((doc) => doc.filePath);
     }
 
     getProjectFileNames(): string[] {

diff --git a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts
@@ -510,14 +510,21 @@ export class TypeScriptPlugin
                 continue;
             }
 
-            if (changeType === FileChangeType.Created && !doneUpdateProjectFiles) {
-                doneUpdateProjectFiles = true;
-                await this.lsAndTsDocResolver.updateProjectFiles();
-            } else if (changeType === FileChangeType.Deleted) {
+            if (changeType === FileChangeType.Deleted) {
                 await this.lsAndTsDocResolver.deleteSnapshot(fileName);
-            } else {
-                await this.lsAndTsDocResolver.updateExistingTsOrJsFile(fileName);
+                continue;
+            }
+
+            if (changeType === FileChangeType.Created) {
+                if (!doneUpdateProjectFiles) {
+                    doneUpdateProjectFiles = true;
+                    await this.lsAndTsDocResolver.updateProjectFiles();
+                }
+                await this.lsAndTsDocResolver.invalidateModuleCache(fileName);
+                continue;
             }
+
+            await this.lsAndTsDocResolver.updateExistingTsOrJsFile(fileName);
         }
     }
 

diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts
@@ -312,6 +312,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
             : `${document.getText()}<script>${inserts}</script>`;
 
         const virtualDoc = new Document(virtualUri, newText);
+        virtualDoc.openedByClient = true;
         // let typescript know about the virtual document
         await this.lsAndTsDocResolver.getSnapshot(virtualDoc);
 

diff --git a/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts b/packages/language-server/src/plugins/typescript/features/CompletionProvider.ts
@@ -103,9 +103,11 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return null;
         }
 
-        const { lang, tsDoc, userPreferences } = await this.lsAndTsDocResolver.getLSAndTSDoc(
-            document
-        );
+        const {
+            lang: langForSyntheticOperations,
+            tsDoc,
+            userPreferences
+        } = await this.lsAndTsDocResolver.getLsForSyntheticOperations(document);
 
         const filePath = tsDoc.filePath;
         if (!filePath) {
@@ -156,7 +158,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
         const offset = tsDoc.offsetAt(tsDoc.getGeneratedPosition(position));
 
         if (isJsDocTriggerCharacter) {
-            return getJsDocTemplateCompletion(tsDoc, lang, filePath, offset);
+            return getJsDocTemplateCompletion(tsDoc, langForSyntheticOperations, filePath, offset);
         }
 
         const svelteNode = tsDoc.svelteNodeAt(originalOffset);
@@ -172,6 +174,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
             return null;
         }
 
+        const { lang } = await this.lsAndTsDocResolver.getLSAndTSDoc(document);
         if (cancellationToken?.isCancellationRequested) {
             return null;
         }

diff --git a/packages/language-server/src/plugins/typescript/features/InlayHintProvider.ts b/packages/language-server/src/plugins/typescript/features/InlayHintProvider.ts
@@ -22,18 +22,20 @@ export class InlayHintProviderImpl implements InlayHintProvider {
         range: Range,
         cancellationToken?: CancellationToken
     ): Promise<InlayHint[] | null> {
-        const { lang, tsDoc, userPreferences } = await this.lsAndTsDocResolver.getLSAndTSDoc(
+        // Don't sync yet so we can skip TypeScript's synchronizeHostData if inlay hints are disabled
+        const { userPreferences } = await this.lsAndTsDocResolver.getLsForSyntheticOperations(
             document
         );
 
         if (
             cancellationToken?.isCancellationRequested ||
-            // skip TypeScript's synchronizeHostData
             !this.areInlayHintsEnabled(userPreferences)
         ) {
             return null;
         }
 
+        const { tsDoc, lang } = await this.lsAndTsDocResolver.getLSAndTSDoc(document);
+
         const inlayHints = lang.provideInlayHints(
             tsDoc.filePath,
             this.convertToTargetTextSpan(range, tsDoc),