Skip to content

Commit 10ee85c

Browse files
sheetalkamatsheetalkamat
committed
Retain the configured project opened during opening client file even if opened file isnt included in that project
This helps not create and remove project on every open if tsconfig file isnt referenced by any open file
1 parent ee623c1 commit 10ee85c

File tree

4 files changed

+83
-30
lines changed

4 files changed

+83
-30
lines changed

‎src/server/editorServices.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,10 @@ namespace ts.server {
284284
configFileErrors?: ReadonlyArray<Diagnostic>;
285285
}
286286

287+
interface AssignProjectResult extends OpenConfiguredProjectResult {
288+
defaultConfigProject: ConfiguredProject | undefined;
289+
}
290+
287291
interface FilePropertyReader<T> {
288292
getFileName(f: T): string;
289293
getScriptKind(f: T, extraFileExtensions?: FileExtensionInfo[]): ScriptKind;
@@ -2635,10 +2639,11 @@ namespace ts.server {
26352639
return info;
26362640
}
26372641

2638-
private assignProjectToOpenedScriptInfo(info: ScriptInfo): OpenConfiguredProjectResult {
2642+
private assignProjectToOpenedScriptInfo(info: ScriptInfo): AssignProjectResult {
26392643
let configFileName: NormalizedPath | undefined;
26402644
let configFileErrors: ReadonlyArray<Diagnostic> | undefined;
26412645
let project: ConfiguredProject | ExternalProject | undefined = this.findExternalProjectContainingOpenScriptInfo(info);
2646+
let defaultConfigProject: ConfiguredProject | undefined;
26422647
if (!project && !this.syntaxOnly) { // Checking syntaxOnly is an optimization
26432648
configFileName = this.getConfigFileNameForFile(info);
26442649
if (configFileName) {
@@ -2659,6 +2664,7 @@ namespace ts.server {
26592664
// Ensure project is ready to check if it contains opened script info
26602665
updateProjectIfDirty(project);
26612666
}
2667+
defaultConfigProject = project;
26622668
}
26632669
}
26642670

@@ -2678,13 +2684,13 @@ namespace ts.server {
26782684
this.assignOrphanScriptInfoToInferredProject(info, this.openFiles.get(info.path));
26792685
}
26802686
Debug.assert(!info.isOrphan());
2681-
return { configFileName, configFileErrors };
2687+
return { configFileName, configFileErrors, defaultConfigProject };
26822688
}
26832689

2684-
private cleanupAfterOpeningFile() {
2690+
private cleanupAfterOpeningFile(toRetainConfigProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
26852691
// This was postponed from closeOpenFile to after opening next file,
26862692
// so that we can reuse the project if we need to right away
2687-
this.removeOrphanConfiguredProjects();
2693+
this.removeOrphanConfiguredProjects(toRetainConfigProjects);
26882694

26892695
// Remove orphan inferred projects now that we have reused projects
26902696
// We need to create a duplicate because we cant guarantee order after removal
@@ -2705,22 +2711,30 @@ namespace ts.server {
27052711

27062712
openClientFileWithNormalizedPath(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, projectRootPath?: NormalizedPath): OpenConfiguredProjectResult {
27072713
const info = this.getOrCreateOpenScriptInfo(fileName, fileContent, scriptKind, hasMixedContent, projectRootPath);
2708-
const result = this.assignProjectToOpenedScriptInfo(info);
2709-
this.cleanupAfterOpeningFile();
2714+
const { defaultConfigProject, ...result } = this.assignProjectToOpenedScriptInfo(info);
2715+
this.cleanupAfterOpeningFile(defaultConfigProject);
27102716
this.telemetryOnOpenFile(info);
27112717
return result;
27122718
}
27132719

2714-
private removeOrphanConfiguredProjects() {
2720+
private removeOrphanConfiguredProjects(toRetainConfiguredProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
27152721
const toRemoveConfiguredProjects = cloneMap(this.configuredProjects);
2722+
if (toRetainConfiguredProjects) {
2723+
if (isArray(toRetainConfiguredProjects)) {
2724+
toRetainConfiguredProjects.forEach(retainConfiguredProject);
2725+
}
2726+
else {
2727+
retainConfiguredProject(toRetainConfiguredProjects);
2728+
}
2729+
}
27162730

27172731
// Do not remove configured projects that are used as original projects of other
27182732
this.inferredProjects.forEach(markOriginalProjectsAsUsed);
27192733
this.externalProjects.forEach(markOriginalProjectsAsUsed);
27202734
this.configuredProjects.forEach(project => {
27212735
// 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
27222736
if (project.hasOpenRef()) {
2723-
toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
2737+
retainConfiguredProject(project);
27242738
markOriginalProjectsAsUsed(project);
27252739
}
27262740
else {
@@ -2729,7 +2743,7 @@ namespace ts.server {
27292743
if (ref) {
27302744
const refProject = this.configuredProjects.get(ref.sourceFile.path);
27312745
if (refProject && refProject.hasOpenRef()) {
2732-
toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
2746+
retainConfiguredProject(project);
27332747
}
27342748
}
27352749
});
@@ -2744,6 +2758,10 @@ namespace ts.server {
27442758
project.originalConfiguredProjects.forEach((_value, configuredProjectPath) => toRemoveConfiguredProjects.delete(configuredProjectPath));
27452759
}
27462760
}
2761+
2762+
function retainConfiguredProject(project: ConfiguredProject) {
2763+
toRemoveConfiguredProjects.delete(project.canonicalConfigFilePath);
2764+
}
27472765
}
27482766

27492767
private removeOrphanScriptInfos() {
@@ -2886,8 +2904,9 @@ namespace ts.server {
28862904
}
28872905

28882906
// All the script infos now exist, so ok to go update projects for open files
2907+
let defaultConfigProjects: ConfiguredProject[] | undefined;
28892908
if (openScriptInfos) {
2890-
openScriptInfos.forEach(info => this.assignProjectToOpenedScriptInfo(info));
2909+
defaultConfigProjects = mapDefined(openScriptInfos, info => this.assignProjectToOpenedScriptInfo(info).defaultConfigProject);
28912910
}
28922911

28932912
// While closing files there could be open files that needed assigning new inferred projects, do it now
@@ -2896,7 +2915,7 @@ namespace ts.server {
28962915
}
28972916

28982917
// Cleanup projects
2899-
this.cleanupAfterOpeningFile();
2918+
this.cleanupAfterOpeningFile(defaultConfigProjects);
29002919

29012920
// Telemetry
29022921
forEach(openScriptInfos, info => this.telemetryOnOpenFile(info));

‎src/testRunner/unittests/tsserver/configuredProjects.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -866,12 +866,12 @@ namespace ts.projectSystem {
866866
const projectService = createProjectService(host);
867867
projectService.openClientFile(file1.path);
868868
host.runQueuedTimeoutCallbacks();
869-
// Since there is no file open from configFile it would be closed
870-
checkNumberOfConfiguredProjects(projectService, 0);
871-
checkNumberOfInferredProjects(projectService, 1);
872869

870+
// Since file1 refers to config file as the default project, it needs to be kept alive
871+
checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
873872
const inferredProject = projectService.inferredProjects[0];
874873
assert.isTrue(inferredProject.containsFile(<server.NormalizedPath>file1.path));
874+
assert.isFalse(projectService.configuredProjects.get(configFile.path)!.containsFile(<server.NormalizedPath>file1.path));
875875
});
876876

877877
it("should be able to handle @types if input file list is empty", () => {
@@ -898,8 +898,8 @@ namespace ts.projectSystem {
898898
const projectService = createProjectService(host);
899899

900900
projectService.openClientFile(f.path);
901-
// Since no file from the configured project is open, it would be closed immediately
902-
projectService.checkNumberOfProjects({ configuredProjects: 0, inferredProjects: 1 });
901+
// Since f refers to config file as the default project, it needs to be kept alive
902+
projectService.checkNumberOfProjects({ configuredProjects: 1, inferredProjects: 1 });
903903
});
904904

905905
it("should tolerate invalid include files that start in subDirectory", () => {
@@ -924,8 +924,8 @@ namespace ts.projectSystem {
924924
const projectService = createProjectService(host);
925925

926926
projectService.openClientFile(f.path);
927-
// Since no file from the configured project is open, it would be closed immediately
928-
projectService.checkNumberOfProjects({ configuredProjects: 0, inferredProjects: 1 });
927+
// Since f refers to config file as the default project, it needs to be kept alive
928+
projectService.checkNumberOfProjects({ configuredProjects: 1, inferredProjects: 1 });
929929
});
930930

931931
it("Changed module resolution reflected when specifying files list", () => {

‎src/testRunner/unittests/tsserver/inferredProjects.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -381,30 +381,54 @@ namespace ts.projectSystem {
381381
return originalDelete.call(projectService.configuredProjects, key);
382382
};
383383

384+
// Do not remove config project when opening jsFile that is not present as part of config project
384385
projectService.openClientFile(jsFile1.path);
385-
checkNumberOfProjects(projectService, { inferredProjects: 1 });
386+
checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
386387
checkProjectActualFiles(projectService.inferredProjects[0], [jsFile1.path, libFile.path]);
387-
checkConfiguredProjectCreatedAndDeleted();
388+
const project = projectService.configuredProjects.get(config.path)!;
389+
checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
390+
checkConfiguredProjectCreatedAndNotDeleted();
388391

392+
// Do not remove config project when opening jsFile that is not present as part of config project
389393
projectService.closeClientFile(jsFile1.path);
390-
checkNumberOfProjects(projectService, { inferredProjects: 1 });
394+
checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
391395
projectService.openClientFile(jsFile2.path);
392-
checkNumberOfProjects(projectService, { inferredProjects: 1 });
396+
checkNumberOfProjects(projectService, { inferredProjects: 1, configuredProjects: 1 });
393397
checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
394-
checkConfiguredProjectCreatedAndDeleted();
398+
checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
399+
checkConfiguredProjectNotCreatedAndNotDeleted();
395400

401+
// Do not remove config project when opening jsFile that is not present as part of config project
396402
projectService.openClientFile(jsFile1.path);
403+
checkNumberOfProjects(projectService, { inferredProjects: 2, configuredProjects: 1 });
404+
checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
405+
checkProjectActualFiles(projectService.inferredProjects[1], [jsFile1.path, libFile.path]);
406+
checkProjectActualFiles(project, [appFile.path, config.path, libFile.path]);
407+
checkConfiguredProjectNotCreatedAndNotDeleted();
408+
409+
// When opening file that doesnt fall back to the config file, we remove the config project
410+
projectService.openClientFile(libFile.path);
397411
checkNumberOfProjects(projectService, { inferredProjects: 2 });
398412
checkProjectActualFiles(projectService.inferredProjects[0], [jsFile2.path, libFile.path]);
399413
checkProjectActualFiles(projectService.inferredProjects[1], [jsFile1.path, libFile.path]);
400-
checkConfiguredProjectCreatedAndDeleted();
414+
checkConfiguredProjectNotCreatedButDeleted();
401415

402-
function checkConfiguredProjectCreatedAndDeleted() {
416+
function checkConfiguredProjectCreatedAndNotDeleted() {
403417
assert.equal(configuredCreated.size, 1);
404418
assert.isTrue(configuredCreated.has(config.path));
419+
assert.equal(configuredRemoved.size, 0);
420+
configuredCreated.clear();
421+
}
422+
423+
function checkConfiguredProjectNotCreatedAndNotDeleted() {
424+
assert.equal(configuredCreated.size, 0);
425+
assert.equal(configuredRemoved.size, 0);
426+
}
427+
428+
function checkConfiguredProjectNotCreatedButDeleted() {
429+
assert.equal(configuredCreated.size, 0);
405430
assert.equal(configuredRemoved.size, 1);
406431
assert.isTrue(configuredRemoved.has(config.path));
407-
configuredCreated.clear();
408432
configuredRemoved.clear();
409433
}
410434
});

‎src/testRunner/unittests/tsserver/projects.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -634,13 +634,17 @@ namespace ts.projectSystem {
634634
path: "/a/main.js",
635635
content: "var y = 1"
636636
};
637+
const f3 = {
638+
path: "/main.js",
639+
content: "var y = 1"
640+
};
637641
const config = {
638642
path: "/a/tsconfig.json",
639643
content: JSON.stringify({
640644
compilerOptions: { allowJs: true }
641645
})
642646
};
643-
const host = createServerHost([f1, f2, config]);
647+
const host = createServerHost([f1, f2, f3, config]);
644648
const projectService = createProjectService(host);
645649
projectService.setHostConfiguration({
646650
extraFileExtensions: [
@@ -652,13 +656,19 @@ namespace ts.projectSystem {
652656
projectService.checkNumberOfProjects({ configuredProjects: 1 });
653657
checkProjectActualFiles(configuredProjectAt(projectService, 0), [f1.path, config.path]);
654658

655-
// Should close configured project with next file open
659+
// Since f2 refers to config file as the default project, it needs to be kept alive
656660
projectService.closeClientFile(f1.path);
657-
658661
projectService.openClientFile(f2.path);
662+
projectService.checkNumberOfProjects({ inferredProjects: 1, configuredProjects: 1 });
663+
assert.isDefined(projectService.configuredProjects.get(config.path));
664+
checkProjectActualFiles(projectService.inferredProjects[0], [f2.path]);
665+
666+
// Should close configured project with next file open
667+
projectService.closeClientFile(f2.path);
668+
projectService.openClientFile(f3.path);
659669
projectService.checkNumberOfProjects({ inferredProjects: 1 });
660670
assert.isUndefined(projectService.configuredProjects.get(config.path));
661-
checkProjectActualFiles(projectService.inferredProjects[0], [f2.path]);
671+
checkProjectActualFiles(projectService.inferredProjects[0], [f3.path]);
662672
});
663673

664674
it("tsconfig script block support", () => {

0 commit comments

Comments
 (0)