Merge pull request #32561 from microsoft/retainFreshlyCreatedProject

  Retain the configured project opened during opening client file even if opened file isnt included in that project

Author: sheetalkamat

src/server/editorServices.ts

@@ -284,6 +284,10 @@ namespace ts.server {
         configFileErrors?: ReadonlyArray<Diagnostic>;
     }
 
+    interface AssignProjectResult extends OpenConfiguredProjectResult {
+        defaultConfigProject: ConfiguredProject | undefined;
+    }
+
     interface FilePropertyReader<T> {
         getFileName(f: T): string;
         getScriptKind(f: T, extraFileExtensions?: FileExtensionInfo[]): ScriptKind;
@@ -2635,10 +2639,11 @@ namespace ts.server {
             return info;
         }
 
-        private assignProjectToOpenedScriptInfo(info: ScriptInfo): OpenConfiguredProjectResult {
+        private assignProjectToOpenedScriptInfo(info: ScriptInfo): AssignProjectResult {
             let configFileName: NormalizedPath | undefined;
             let configFileErrors: ReadonlyArray<Diagnostic> | undefined;
             let project: ConfiguredProject | ExternalProject | undefined = this.findExternalProjectContainingOpenScriptInfo(info);
+            let defaultConfigProject: ConfiguredProject | undefined;
             if (!project && !this.syntaxOnly) { // Checking syntaxOnly is an optimization
                 configFileName = this.getConfigFileNameForFile(info);
                 if (configFileName) {
@@ -2659,6 +2664,7 @@ namespace ts.server {
                         // Ensure project is ready to check if it contains opened script info
                         updateProjectIfDirty(project);
                     }
+                    defaultConfigProject = project;
                 }
             }
 
@@ -2678,13 +2684,13 @@ namespace ts.server {
                 this.assignOrphanScriptInfoToInferredProject(info, this.openFiles.get(info.path));
             }
             Debug.assert(!info.isOrphan());
-            return { configFileName, configFileErrors };
+            return { configFileName, configFileErrors, defaultConfigProject };
         }
 
-        private cleanupAfterOpeningFile() {
+        private cleanupAfterOpeningFile(toRetainConfigProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
             // This was postponed from closeOpenFile to after opening next file,
             // so that we can reuse the project if we need to right away
-            this.removeOrphanConfiguredProjects();
+            this.removeOrphanConfiguredProjects(toRetainConfigProjects);
 
             // Remove orphan inferred projects now that we have reused projects
             // We need to create a duplicate because we cant guarantee order after removal
@@ -2705,22 +2711,30 @@ namespace ts.server {
 
         openClientFileWithNormalizedPath(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, projectRootPath?: NormalizedPath): OpenConfiguredProjectResult {
             const info = this.getOrCreateOpenScriptInfo(fileName, fileContent, scriptKind, hasMixedContent, projectRootPath);
-            const result = this.assignProjectToOpenedScriptInfo(info);
-            this.cleanupAfterOpeningFile();
+            const { defaultConfigProject, ...result } = this.assignProjectToOpenedScriptInfo(info);
+            this.cleanupAfterOpeningFile(defaultConfigProject);
             this.telemetryOnOpenFile(info);
             return result;
         }
 
-        private removeOrphanConfiguredProjects() {
+        private removeOrphanConfiguredProjects(toRetainConfiguredProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
             const toRemoveConfiguredProjects = cloneMap(this.configuredProjects);
+            if (toRetainConfiguredProjects) {
+                if (isArray(toRetainConfiguredProjects)) {
+                    toRetainConfiguredProjects.forEach(retainConfiguredProject);
+                }
+                else {
+                    retainConfiguredProject(toRetainConfiguredProjects);
+                }
+            }
 
             // Do not remove configured projects that are used as original projects of other
             this.inferredProjects.forEach(markOriginalProjectsAsUsed);
             this.externalProjects.forEach(markOriginalProjectsAsUsed);
             this.configuredProjects.forEach(project => {
                 // If project has open ref (there are more than zero references from external project/open file), keep it alive as well as any project it references
                 if (project.hasOpenRef()) {
-                    toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
+                    retainConfiguredProject(project);
                     markOriginalProjectsAsUsed(project);
                 }
                 else {
@@ -2729,7 +2743,7 @@ namespace ts.server {
                         if (ref) {
                             const refProject = this.configuredProjects.get(ref.sourceFile.path);
                             if (refProject && refProject.hasOpenRef()) {
-                                toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
+                                retainConfiguredProject(project);
                             }
                         }
                     });
@@ -2744,6 +2758,10 @@ namespace ts.server {
                     project.originalConfiguredProjects.forEach((_value, configuredProjectPath) => toRemoveConfiguredProjects.delete(configuredProjectPath));
                 }
             }
+
+            function retainConfiguredProject(project: ConfiguredProject) {
+                toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
+            }
         }
 
         private removeOrphanScriptInfos() {
@@ -2886,8 +2904,9 @@ namespace ts.server {
             }
 
             // All the script infos now exist, so ok to go update projects for open files
+            let defaultConfigProjects: ConfiguredProject[] | undefined;
             if (openScriptInfos) {
-                openScriptInfos.forEach(info => this.assignProjectToOpenedScriptInfo(info));
+                defaultConfigProjects = mapDefined(openScriptInfos, info => this.assignProjectToOpenedScriptInfo(info).defaultConfigProject);
             }
 
             // While closing files there could be open files that needed assigning new inferred projects, do it now
@@ -2896,7 +2915,7 @@ namespace ts.server {
             }
 
             // Cleanup projects
-            this.cleanupAfterOpeningFile();
+            this.cleanupAfterOpeningFile(defaultConfigProjects);
 
             // Telemetry
             forEach(openScriptInfos, info => this.telemetryOnOpenFile(info));

src/testRunner/unittests/tsserver/configuredProjects.ts

@@ -866,12 +866,12 @@ namespace ts.projectSystem {
             const projectService = createProjectService(host);
             projectService.openClientFile(file1.path);
             host.runQueuedTimeoutCallbacks();
-            // Since there is no file open from configFile it would be closed
-            checkNumberOfConfiguredProjects(projectService, 0);
-            checkNumberOfInferredProjects(projectService, 1);
 
+            // Since file1 refers to config file as the default project, it needs to be kept alive
+            checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
             const inferredProject = projectService.inferredProjects[0];
             assert.isTrue(inferredProject.containsFile(<server.NormalizedPath>file1.path));
+            assert.isFalse(projectService.configuredProjects.get(configFile.path)!.containsFile(<server.NormalizedPath>file1.path));
         });
 
         it("should be able to handle @types if input file list is empty", () => {
@@ -898,8 +898,8 @@ namespace ts.projectSystem {
             const projectService = createProjectService(host);
 
             projectService.openClientFile(f.path);
-            // Since no file from the configured project is open, it would be closed immediately
-            projectService.checkNumberOfProjects({ configuredProjects: 0, inferredProjects: 1 });
+            // Since f refers to config file as the default project, it needs to be kept alive
+            projectService.checkNumberOfProjects({ configuredProjects: 1, inferredProjects: 1 });
         });
 
         it("should tolerate invalid include files that start in subDirectory", () => {
@@ -924,8 +924,8 @@ namespace ts.projectSystem {
             const projectService = createProjectService(host);
 
             projectService.openClientFile(f.path);
-            // Since no file from the configured project is open, it would be closed immediately
-            projectService.checkNumberOfProjects({ configuredProjects: 0, inferredProjects: 1 });
+            // Since f refers to config file as the default project, it needs to be kept alive
+            projectService.checkNumberOfProjects({ configuredProjects: 1, inferredProjects: 1 });
         });
 
         it("Changed module resolution reflected when specifying files list", () => {

src/testRunner/unittests/tsserver/inferredProjects.ts

@@ -345,5 +345,92 @@ namespace ts.projectSystem {
         it("inferred projects per project root with case insensitive system", () => {
             verifyProjectRootWithCaseSensitivity(/*useCaseSensitiveFileNames*/ false);
         });
+
+        it("should still retain configured project created while opening the file", () => {
+            const projectRoot = "/user/username/projects/project";
+            const appFile: File = {
+                path: `${projectRoot}/app.ts`,
+                content: `const app = 20;`
+            };
+            const config: File = {
+                path: `${projectRoot}/tsconfig.json`,
+                content: "{}"
+            };
+            const jsFile1: File = {
+                path: `${projectRoot}/jsFile1.js`,
+                content: `const jsFile1 = 10;`
+            };
+            const jsFile2: File = {
+                path: `${projectRoot}/jsFile2.js`,
+                content: `const jsFile2 = 10;`
+            };
+            const host = createServerHost([appFile, libFile, config, jsFile1, jsFile2]);
+            const projectService = createProjectService(host);
+            const originalSet = projectService.configuredProjects.set;
+            const originalDelete = projectService.configuredProjects.delete;
+            const configuredCreated = createMap<true>();
+            const configuredRemoved = createMap<true>();
+            projectService.configuredProjects.set = (key, value) => {
+                assert.isFalse(configuredCreated.has(key));
+                configuredCreated.set(key, true);
+                return originalSet.call(projectService.configuredProjects, key, value);
+            };
+            projectService.configuredProjects.delete = key => {
+                assert.isFalse(configuredRemoved.has(key));
+                configuredRemoved.set(key, true);
+                return originalDelete.call(projectService.configuredProjects, key);
+            };
+
+            // Do not remove config project when opening jsFile that is not present as part of config project
+            projectService.openClientFile(jsFile1.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
+            checkProjectActualFiles(projectService.inferredProjects[0], [jsFile1.path, libFile.path]);
+            const project = projectService.configuredProjects.get(config.path)!;
+            checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
+            checkConfiguredProjectCreatedAndNotDeleted();
+
+            // Do not remove config project when opening jsFile that is not present as part of config project
+            projectService.closeClientFile(jsFile1.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
+            projectService.openClientFile(jsFile2.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
+            checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
+            checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
+            checkConfiguredProjectNotCreatedAndNotDeleted();
+
+            // Do not remove config project when opening jsFile that is not present as part of config project
+            projectService.openClientFile(jsFile1.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 2, configuredProjects: 1 });
+            checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
+            checkProjectActualFiles(projectService.inferredProjects[1], [jsFile1.path, libFile.path]);
+            checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
+            checkConfiguredProjectNotCreatedAndNotDeleted();
+
+            // When opening file that doesnt fall back to the config file, we remove the config project
+            projectService.openClientFile(libFile.path);
+            checkNumberOfProjects(projectService, { inferredProjects: 2 });
+            checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
+            checkProjectActualFiles(projectService.inferredProjects[1], [jsFile1.path, libFile.path]);
+            checkConfiguredProjectNotCreatedButDeleted();
+
+            function checkConfiguredProjectCreatedAndNotDeleted() {
+                assert.equal(configuredCreated.size, 1);
+                assert.isTrue(configuredCreated.has(config.path));
+                assert.equal(configuredRemoved.size, 0);
+                configuredCreated.clear();
+            }
+
+            function checkConfiguredProjectNotCreatedAndNotDeleted() {
+                assert.equal(configuredCreated.size, 0);
+                assert.equal(configuredRemoved.size, 0);
+            }
+
+            function checkConfiguredProjectNotCreatedButDeleted() {
+                assert.equal(configuredCreated.size, 0);
+                assert.equal(configuredRemoved.size, 1);
+                assert.isTrue(configuredRemoved.has(config.path));
+                configuredRemoved.clear();
+            }
+        });
     });
 }

src/testRunner/unittests/tsserver/projects.ts

@@ -634,13 +634,17 @@ namespace ts.projectSystem {
                 path: "/a/main.js",
                 content: "var y = 1"
             };
+            const f3 = {
+                path: "/main.js",
+                content: "var y = 1"
+            };
             const config = {
                 path: "/a/tsconfig.json",
                 content: JSON.stringify({
                     compilerOptions: { allowJs: true }
                 })
             };
-            const host = createServerHost([f1, f2, config]);
+            const host = createServerHost([f1, f2, f3, config]);
             const projectService = createProjectService(host);
             projectService.setHostConfiguration({
                 extraFileExtensions: [
@@ -652,13 +656,19 @@ namespace ts.projectSystem {
             projectService.checkNumberOfProjects({ configuredProjects: 1 });
             checkProjectActualFiles(configuredProjectAt(projectService, 0), [f1.path, config.path]);
 
-            // Should close configured project with next file open
+            // Since f2 refers to config file as the default project, it needs to be kept alive
             projectService.closeClientFile(f1.path);
-
             projectService.openClientFile(f2.path);
+            projectService.checkNumberOfProjects({ inferredProjects: 1, configuredProjects: 1 });
+            assert.isDefined(projectService.configuredProjects.get(config.path));
+            checkProjectActualFiles(projectService.inferredProjects[0], [f2.path]);
+
+            // Should close configured project with next file open
+            projectService.closeClientFile(f2.path);
+            projectService.openClientFile(f3.path);
             projectService.checkNumberOfProjects({ inferredProjects: 1 });
             assert.isUndefined(projectService.configuredProjects.get(config.path));
-            checkProjectActualFiles(projectService.inferredProjects[0], [f2.path]);
+            checkProjectActualFiles(projectService.inferredProjects[0], [f3.path]);
         });
 
         it("tsconfig script block support", () => {