From 9f3e5eaaddce416f2e56b8b43a186ab8d1c393e2 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 26 Jun 2018 16:14:27 -0700 Subject: [PATCH] Use different cache for the ScriptInfoVersion --- src/server/editorServices.ts | 30 ++++++++++--------- .../reference/api/tsserverlibrary.d.ts | 1 + 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 5394d53079d..af3ddfe33ec 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -334,11 +334,6 @@ namespace ts.server { return project.dirty && project.updateGraph(); } - type ScriptInfoOrVersion = ScriptInfo | ScriptInfoVersion; - function isScriptInfoVersion(infoOrVersion: ScriptInfoOrVersion): infoOrVersion is ScriptInfoVersion { - return (infoOrVersion as ScriptInfoVersion).svc !== undefined; - } - export class ProjectService { /*@internal*/ @@ -350,7 +345,13 @@ namespace ts.server { /** * Container of all known scripts */ - private readonly filenameToScriptInfo = createMap(); + private readonly filenameToScriptInfo = createMap(); + /** + * Contains all the deleted script info's version information so that + * it does not reset when creating script info again + * (and could have potentially collided with version where contents mismatch) + */ + private readonly filenameToScriptInfoVersion = createMap(); // Set of all '.js' files ever opened. private readonly allJsFilesForOpenFileTelemetry = createMap(); @@ -889,7 +890,7 @@ namespace ts.server { project.close(); if (Debug.shouldAssert(AssertionLevel.Normal)) { - this.filenameToScriptInfo.forEach(info => Debug.assert(isScriptInfoVersion(info) || !info.isAttached(project))); + this.filenameToScriptInfo.forEach(info => Debug.assert(!info.isAttached(project))); } // Remove the project from pending project updates this.pendingProjectUpdates.delete(project.getProjectName()); @@ -1026,7 +1027,8 @@ namespace ts.server { } private deleteScriptInfo(info: ScriptInfo) { - this.filenameToScriptInfo.set(info.path, info.getVersion()); + this.filenameToScriptInfo.delete(info.path); + this.filenameToScriptInfoVersion.set(info.path, info.getVersion()); const realpath = info.getRealpathIfDifferent(); if (realpath) { this.realpathToScriptInfos!.remove(realpath, info); // TODO: GH#18217 @@ -1855,8 +1857,8 @@ namespace ts.server { private getOrCreateScriptInfoWorker(fileName: NormalizedPath, currentDirectory: string, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, hostToQueryFileExistsOn?: { fileExists(path: string): boolean; }) { Debug.assert(fileContent === undefined || openedByClient, "ScriptInfo needs to be opened by client to be able to set its user defined content"); const path = normalizedPathToPath(fileName, currentDirectory, this.toCanonicalFileName); - let info = this.filenameToScriptInfo.get(path); - if (!info || isScriptInfoVersion(info)) { + let info = this.getScriptInfoForPath(path); + if (!info) { const isDynamic = isDynamicFileName(fileName); Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nScript info with non-dynamic relative file name can only be open script info`); Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nOpen script files with non rooted disk path opened with current directory context cannot have same canonical names`); @@ -1865,8 +1867,9 @@ namespace ts.server { if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) { return; } - info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path, info); // TODO: GH#18217 + info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path, this.filenameToScriptInfoVersion.get(path)); // TODO: GH#18217 this.filenameToScriptInfo.set(info.path, info); + this.filenameToScriptInfoVersion.delete(info.path); if (!openedByClient) { this.watchClosedScriptInfo(info); } @@ -1899,8 +1902,7 @@ namespace ts.server { } getScriptInfoForPath(fileName: Path) { - const info = this.filenameToScriptInfo.get(fileName); - return info && isScriptInfoVersion(info) ? undefined : info; + return this.filenameToScriptInfo.get(fileName); } setHostConfiguration(args: protocol.ConfigureRequestArguments) { @@ -2151,7 +2153,7 @@ namespace ts.server { // It was then postponed to cleanup these script infos so that they can be reused if // the file from that old project is reopened because of opening file from here. this.filenameToScriptInfo.forEach(info => { - if (!isScriptInfoVersion(info) && !info.isScriptOpen() && info.isOrphan()) { + if (!info.isScriptOpen() && info.isOrphan()) { // if there are not projects that include this script info - delete it this.stopWatchingScriptInfo(info); this.deleteScriptInfo(info); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index d5f37c2aba2..3841b80b16b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -13784,6 +13784,7 @@ declare namespace ts.server { readonly typingsCache: TypingsCache; readonly documentRegistry: DocumentRegistry; private readonly filenameToScriptInfo; + private readonly filenameToScriptInfoVersion; private readonly allJsFilesForOpenFileTelemetry; readonly realpathToScriptInfos: MultiMap | undefined; private readonly externalProjectToConfiguredProjectMap;