From b3599b23bce28be28a19694aed952ccba08ac9e9 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 27 Apr 2021 14:23:11 +0200 Subject: [PATCH] (fix) update document logic Due to the refactoring in #917, documents were reread from disk every time when getting a snapshot. This resulted in worse performance and introduced inconsistencies because "openDocument" of the DocumentManager was called over and over again with potentially outdated content, resulting in buggy, seemingly random behavior. This fix reverts that part of #917 which was accidentally introduced and only loads documents from disk if a snapshot doesn't already exist. --- .../src/lib/documents/Document.ts | 1 - .../plugins/typescript/LSAndTSDocResolver.ts | 4 +- .../src/plugins/typescript/service.ts | 52 +++++++++++-------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/packages/language-server/src/lib/documents/Document.ts b/packages/language-server/src/lib/documents/Document.ts index 3272530f3..c933b21df 100644 --- a/packages/language-server/src/lib/documents/Document.ts +++ b/packages/language-server/src/lib/documents/Document.ts @@ -51,7 +51,6 @@ export class Document extends WritableDocument { if (config && !config.loadConfigError) { update(config); } else { - this.configPromise = configLoader.awaitConfig(this.getFilePath() || ''); update(undefined); this.configPromise.then((c) => update(c)); } diff --git a/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts b/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts index 6772a95c7..702291920 100644 --- a/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts +++ b/packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts @@ -86,7 +86,7 @@ export class LSAndTSDocResolver { async getSnapshot(pathOrDoc: string | Document) { const filePath = typeof pathOrDoc === 'string' ? pathOrDoc : pathOrDoc.getFilePath() || ''; const tsService = await this.getTSService(filePath); - return tsService.updateDocument(pathOrDoc); + return tsService.updateSnapshot(pathOrDoc); } async updateSnapshotPath(oldPath: string, newPath: string): Promise { @@ -95,7 +95,7 @@ export class LSAndTSDocResolver { } async deleteSnapshot(filePath: string) { - (await this.getTSService(filePath)).deleteDocument(filePath); + (await this.getTSService(filePath)).deleteSnapshot(filePath); this.docManager.releaseDocument(pathToUrl(filePath)); } diff --git a/packages/language-server/src/plugins/typescript/service.ts b/packages/language-server/src/plugins/typescript/service.ts index ec753f396..9ae3a31d2 100644 --- a/packages/language-server/src/plugins/typescript/service.ts +++ b/packages/language-server/src/plugins/typescript/service.ts @@ -14,8 +14,8 @@ export interface LanguageServiceContainer { readonly compilerOptions: ts.CompilerOptions; readonly snapshotManager: SnapshotManager; getService(): ts.LanguageService; - updateDocument(documentOrFilePath: Document | string): DocumentSnapshot; - deleteDocument(filePath: string): void; + updateSnapshot(documentOrFilePath: Document | string): DocumentSnapshot; + deleteSnapshot(filePath: string): void; } const services = new Map>(); @@ -125,36 +125,31 @@ async function createLanguageService( tsconfigPath, compilerOptions, getService: () => languageService, - updateDocument, - deleteDocument, + updateSnapshot, + deleteSnapshot, snapshotManager }; - function deleteDocument(filePath: string): void { + function deleteSnapshot(filePath: string): void { svelteModuleLoader.deleteFromModuleCache(filePath); snapshotManager.delete(filePath); } - function updateDocument(documentOrFilePath: Document | string): DocumentSnapshot { - const filePath = - typeof documentOrFilePath === 'string' - ? documentOrFilePath - : documentOrFilePath.getFilePath() || ''; - const document = typeof documentOrFilePath === 'string' ? undefined : documentOrFilePath; - const prevSnapshot = snapshotManager.get(filePath); + function updateSnapshot(documentOrFilePath: Document | string): DocumentSnapshot { + return typeof documentOrFilePath === 'string' + ? updateSnapshotFromFilePath(documentOrFilePath) + : updateSnapshotFromDocument(documentOrFilePath); + } - // Don't reinitialize document if no update needed. - if (document && prevSnapshot?.version === document.version) { + function updateSnapshotFromDocument(document: Document): DocumentSnapshot { + const filePath = document.getFilePath() || ''; + const prevSnapshot = snapshotManager.get(filePath); + if (prevSnapshot?.version === document.version) { return prevSnapshot; } - const newSnapshot = document - ? DocumentSnapshot.fromDocument(document, transformationConfig) - : DocumentSnapshot.fromFilePath( - filePath, - docContext.createDocument, - transformationConfig - ); + const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig); + snapshotManager.set(filePath, newSnapshot); if (prevSnapshot && prevSnapshot.scriptKind !== newSnapshot.scriptKind) { // Restart language service as it doesn't handle script kind changes. @@ -165,6 +160,21 @@ async function createLanguageService( return newSnapshot; } + function updateSnapshotFromFilePath(filePath: string): DocumentSnapshot { + const prevSnapshot = snapshotManager.get(filePath); + if (prevSnapshot) { + return prevSnapshot; + } + + const newSnapshot = DocumentSnapshot.fromFilePath( + filePath, + docContext.createDocument, + transformationConfig + ); + snapshotManager.set(filePath, newSnapshot); + return newSnapshot; + } + function getSnapshot(fileName: string): DocumentSnapshot { fileName = ensureRealSvelteFilePath(fileName);