Merge pull request #31685 from uniqueiniquity/stopInvalidatingOnOpenF…

…ileSave Stop invalidating module resolution cache when saving an open file
microsoft · Jun 12, 2019 · a2cd10b · a2cd10b
1 parent a1a2bd6
commit a2cd10b
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<protocol.CloseRequest>({
                 command: protocol.CommandTypes.Close,
                 arguments: { file: unitTest1.path }
             });
+            host.checkTimeoutQueueLengthAndRun(2);
             checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
 
             const unitTest1WithChangedContent: File = {