From a2cd10b5ff2c0c9903ea136191b72372e78230df Mon Sep 17 00:00:00 2001 From: Benjamin Lichtman Date: Fri, 31 May 2019 20:36:33 -0400 Subject: [PATCH] Merge pull request #31685 from uniqueiniquity/stopInvalidatingOnOpenFileSave Stop invalidating module resolution cache when saving an open file --- src/compiler/resolutionCache.ts | 6 ++++ src/compiler/watch.ts | 1 + src/server/editorServices.ts | 6 ++++ src/server/project.ts | 5 ++++ src/testRunner/unittests/tsserver/projects.ts | 20 +++++++++++++ .../unittests/tsserver/resolutionCache.ts | 29 +++++++++++++++++++ .../unittests/tsserver/syntaxOperations.ts | 5 ++-- 7 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index b82a5a78d7187..fb04d7b7a187e 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -54,6 +54,7 @@ namespace ts { writeLog(s: string): void; maxNumberOfFilesToIterateForInvalidation?: number; getCurrentProgram(): Program | undefined; + fileIsOpen(filePath: Path): boolean; } interface DirectoryWatchesOfFailedLookup { @@ -698,6 +699,11 @@ namespace ts { // If something to do with folder/file starting with "." in node_modules folder, skip it if (isPathIgnored(fileOrDirectoryPath)) return false; + // prevent saving an open file from over-eagerly triggering invalidation + if (resolutionHost.fileIsOpen(fileOrDirectoryPath)) { + return false; + } + // Some file or directory in the watching directory is created // Return early if it does not have any of the watching extension or not the custom failed lookup path const dirOfFileOrDirectory = getDirectoryPath(fileOrDirectoryPath); diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index f9bf2a8468db6..dbc78fa5d4e2e 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -691,6 +691,7 @@ namespace ts { hasChangedAutomaticTypeDirectiveNames = true; scheduleProgramUpdate(); }; + compilerHost.fileIsOpen = returnFalse; compilerHost.maxNumberOfFilesToIterateForInvalidation = host.maxNumberOfFilesToIterateForInvalidation; compilerHost.getCurrentProgram = getCurrentProgram; compilerHost.writeLog = writeLog; diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 6bfce902a049b..b868286ea51af 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1003,6 +1003,12 @@ namespace ts.server { fileOrDirectory => { const fileOrDirectoryPath = this.toPath(fileOrDirectory); project.getCachedDirectoryStructureHost().addOrDeleteFileOrDirectory(fileOrDirectory, fileOrDirectoryPath); + + // don't trigger callback on open, existing files + if (project.fileIsOpen(fileOrDirectoryPath)) { + return; + } + if (isPathIgnored(fileOrDirectoryPath)) return; const configFilename = project.getConfigFilePath(); diff --git a/src/server/project.ts b/src/server/project.ts index 20cd5667dd9db..e10a0988f7185 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -457,6 +457,11 @@ namespace ts.server { return this.getTypeAcquisition().enable ? this.projectService.typingsInstaller.globalTypingsCacheLocation : undefined; } + /*@internal*/ + fileIsOpen(filePath: Path) { + return this.projectService.openFiles.has(filePath); + } + /*@internal*/ writeLog(s: string) { this.projectService.logger.info(s); diff --git a/src/testRunner/unittests/tsserver/projects.ts b/src/testRunner/unittests/tsserver/projects.ts index ba1eae236a5ec..409ce5c525f74 100644 --- a/src/testRunner/unittests/tsserver/projects.ts +++ b/src/testRunner/unittests/tsserver/projects.ts @@ -1341,6 +1341,26 @@ var x = 10;` } }); + it("no project structure update on directory watch invoke on open file save", () => { + const projectRootPath = "/users/username/projects/project"; + const file1: File = { + path: `${projectRootPath}/a.ts`, + content: "export const a = 10;" + }; + const config: File = { + path: `${projectRootPath}/tsconfig.json`, + content: "{}" + }; + const files = [file1, config]; + const host = createServerHost(files); + const service = createProjectService(host); + service.openClientFile(file1.path); + checkNumberOfProjects(service, { configuredProjects: 1 }); + + host.modifyFile(file1.path, file1.content, { invokeFileDeleteCreateAsPartInsteadOfChange: true }); + host.checkTimeoutQueueLength(0); + }); + it("handles delayed directory watch invoke on file creation", () => { const projectRootPath = "/users/username/projects/project"; const fileB: File = { diff --git a/src/testRunner/unittests/tsserver/resolutionCache.ts b/src/testRunner/unittests/tsserver/resolutionCache.ts index 33b59e2faae3a..768111f3bfeb4 100644 --- a/src/testRunner/unittests/tsserver/resolutionCache.ts +++ b/src/testRunner/unittests/tsserver/resolutionCache.ts @@ -975,5 +975,34 @@ export const x = 10;` host.checkTimeoutQueueLength(0); }); }); + + describe("avoid unnecessary invalidation", () => { + it("unnecessary lookup invalidation on save", () => { + const expectedNonRelativeDirectories = [`${projectLocation}/node_modules`, `${projectLocation}/src`]; + const module1Name = "module1"; + const module2Name = "module2"; + const fileContent = `import { module1 } from "${module1Name}";import { module2 } from "${module2Name}";`; + const file1: File = { + path: `${projectLocation}/src/file1.ts`, + content: fileContent + }; + const { module1, module2 } = getModules(`${projectLocation}/src/node_modules/module1/index.ts`, `${projectLocation}/node_modules/module2/index.ts`); + const files = [module1, module2, file1, configFile, libFile]; + const host = createServerHost(files); + const resolutionTrace = createHostModuleResolutionTrace(host); + const service = createProjectService(host); + service.openClientFile(file1.path); + const project = service.configuredProjects.get(configFile.path)!; + (project as ResolutionCacheHost).maxNumberOfFilesToIterateForInvalidation = 1; + const expectedTrace = getExpectedNonRelativeModuleResolutionTrace(host, file1, module1, module1Name); + getExpectedNonRelativeModuleResolutionTrace(host, file1, module2, module2Name, expectedTrace); + verifyTrace(resolutionTrace, expectedTrace); + verifyWatchesWithConfigFile(host, files, file1, expectedNonRelativeDirectories); + + // invoke callback to simulate saving + host.modifyFile(file1.path, file1.content, { invokeFileDeleteCreateAsPartInsteadOfChange: true }); + host.checkTimeoutQueueLengthAndRun(0); + }); + }); }); } diff --git a/src/testRunner/unittests/tsserver/syntaxOperations.ts b/src/testRunner/unittests/tsserver/syntaxOperations.ts index 66a962d362507..d3127355191cf 100644 --- a/src/testRunner/unittests/tsserver/syntaxOperations.ts +++ b/src/testRunner/unittests/tsserver/syntaxOperations.ts @@ -60,13 +60,14 @@ describe("Test Suite 1", () => { const navBarResultUnitTest1 = navBarFull(session, unitTest1); host.deleteFile(unitTest1.path); - host.checkTimeoutQueueLengthAndRun(2); - checkProjectActualFiles(project, expectedFilesWithoutUnitTest1); + host.checkTimeoutQueueLengthAndRun(0); + checkProjectActualFiles(project, expectedFilesWithUnitTest1); session.executeCommandSeq({ command: protocol.CommandTypes.Close, arguments: { file: unitTest1.path } }); + host.checkTimeoutQueueLengthAndRun(2); checkProjectActualFiles(project, expectedFilesWithoutUnitTest1); const unitTest1WithChangedContent: File = {