Skip to content

Commit 823b69a

Browse files
authored
Fixes slow nav to issue by keeping project with result. (#39721)
Also cache the result of config file name for open files Fixes #38491
1 parent 73b268e commit 823b69a

File tree

4 files changed

+136
-48
lines changed

4 files changed

+136
-48
lines changed

Diff for: src/server/editorServices.ts

+17-5
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,8 @@ namespace ts.server {
629629
* Open files: with value being project root path, and key being Path of the file that is open
630630
*/
631631
readonly openFiles: Map<NormalizedPath | undefined> = new Map<Path, NormalizedPath | undefined>();
632+
/* @internal */
633+
readonly configFileForOpenFiles: ESMap<Path, NormalizedPath | false> = new Map();
632634
/**
633635
* Map of open files that are opened without complete path but have projectRoot as current directory
634636
*/
@@ -1461,6 +1463,7 @@ namespace ts.server {
14611463
}
14621464

14631465
this.openFiles.delete(info.path);
1466+
this.configFileForOpenFiles.delete(info.path);
14641467

14651468
if (!skipAssignOrphanScriptInfosToInferredProject && ensureProjectsForOpenFiles) {
14661469
this.assignOrphanScriptInfosToInferredProject();
@@ -1791,7 +1794,11 @@ namespace ts.server {
17911794
* otherwise just file name
17921795
*/
17931796
private getConfigFileNameForFile(info: OpenScriptInfoOrClosedOrConfigFileInfo) {
1794-
if (isOpenScriptInfo(info)) Debug.assert(info.isScriptOpen());
1797+
if (isOpenScriptInfo(info)) {
1798+
Debug.assert(info.isScriptOpen());
1799+
const result = this.configFileForOpenFiles.get(info.path);
1800+
if (result !== undefined) return result || undefined;
1801+
}
17951802
this.logger.info(`Search path: ${getDirectoryPath(info.fileName)}`);
17961803
const configFileName = this.forEachConfigFileLocation(info, (configFileName, canonicalConfigFilePath) =>
17971804
this.configFileExists(configFileName, canonicalConfigFilePath, info));
@@ -1801,6 +1808,9 @@ namespace ts.server {
18011808
else {
18021809
this.logger.info(`For info: ${info.fileName} :: No config files found.`);
18031810
}
1811+
if (isOpenScriptInfo(info)) {
1812+
this.configFileForOpenFiles.set(info.path, configFileName || false);
1813+
}
18041814
return configFileName;
18051815
}
18061816

@@ -2171,14 +2181,14 @@ namespace ts.server {
21712181
* Read the config file of the project again by clearing the cache and update the project graph
21722182
*/
21732183
/* @internal */
2174-
reloadConfiguredProject(project: ConfiguredProject, reason: string) {
2184+
reloadConfiguredProject(project: ConfiguredProject, reason: string, isInitialLoad: boolean) {
21752185
// At this point, there is no reason to not have configFile in the host
21762186
const host = project.getCachedDirectoryStructureHost();
21772187

21782188
// Clear the cache since we are reloading the project from disk
21792189
host.clearCache();
21802190
const configFileName = project.getConfigFilePath();
2181-
this.logger.info(`Reloading configured project ${configFileName}`);
2191+
this.logger.info(`${isInitialLoad ? "Loading" : "Reloading"} configured project ${configFileName}`);
21822192

21832193
// Load project from the disk
21842194
this.loadConfiguredProject(project, reason);
@@ -2795,11 +2805,13 @@ namespace ts.server {
27952805
const reloadChildProject = (child: ConfiguredProject) => {
27962806
if (!updatedProjects.has(child.canonicalConfigFilePath)) {
27972807
updatedProjects.set(child.canonicalConfigFilePath, true);
2798-
this.reloadConfiguredProject(child, reason);
2808+
this.reloadConfiguredProject(child, reason, /*isInitialLoad*/ false);
27992809
}
28002810
};
28012811
// try to reload config file for all open files
28022812
openFiles.forEach((openFileValue, path) => {
2813+
// Invalidate default config file name for open file
2814+
this.configFileForOpenFiles.delete(path);
28032815
// Filter out the files that need to be ignored
28042816
if (!shouldReloadProjectFor(openFileValue)) {
28052817
return;
@@ -2823,7 +2835,7 @@ namespace ts.server {
28232835
}
28242836
else {
28252837
// reload from the disk
2826-
this.reloadConfiguredProject(project, reason);
2838+
this.reloadConfiguredProject(project, reason, /*isInitialLoad*/ false);
28272839
// If this project does not contain this file directly, reload the project till the reloaded project contains the script info directly
28282840
if (!projectContainsInfoDirectly(project, info)) {
28292841
const referencedProject = forEachResolvedProjectReferenceProject(

Diff for: src/server/project.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -2102,6 +2102,7 @@ namespace ts.server {
21022102
* @returns: true if set of files in the project stays the same and false - otherwise.
21032103
*/
21042104
updateGraph(): boolean {
2105+
const isInitialLoad = this.isInitialLoadPending();
21052106
this.isInitialLoadPending = returnFalse;
21062107
const reloadLevel = this.pendingReload;
21072108
this.pendingReload = ConfigFileProgramReloadLevel.None;
@@ -2115,7 +2116,7 @@ namespace ts.server {
21152116
this.openFileWatchTriggered.clear();
21162117
const reason = Debug.checkDefined(this.pendingReloadReason);
21172118
this.pendingReloadReason = undefined;
2118-
this.projectService.reloadConfiguredProject(this, reason);
2119+
this.projectService.reloadConfiguredProject(this, reason, isInitialLoad);
21192120
result = true;
21202121
break;
21212122
default:

Diff for: src/server/session.ts

+56-41
Original file line numberDiff line numberDiff line change
@@ -284,34 +284,43 @@ namespace ts.server {
284284
return deduplicate(outputs, equateValues);
285285
}
286286

287-
function combineProjectOutputFromEveryProject<T>(projectService: ProjectService, action: (project: Project) => readonly T[], areEqual: (a: T, b: T) => boolean) {
288-
const outputs: T[] = [];
287+
type CombineOutputResult<T> = { project: Project; result: readonly T[]; }[];
288+
function combineOutputResultContains<T>(outputs: CombineOutputResult<T>, output: T, areEqual: (a: T, b: T) => boolean) {
289+
return outputs.some(({ result }) => contains(result, output, areEqual));
290+
}
291+
function addToCombineOutputResult<T>(outputs: CombineOutputResult<T>, project: Project, result: readonly T[]) {
292+
if (result.length) outputs.push({ project, result });
293+
}
294+
295+
function combineProjectOutputFromEveryProject<T>(projectService: ProjectService, action: (project: Project) => readonly T[], areEqual: (a: T, b: T) => boolean): CombineOutputResult<T> {
296+
const outputs: CombineOutputResult<T> = [];
289297
projectService.loadAncestorProjectTree();
290298
projectService.forEachEnabledProject(project => {
291299
const theseOutputs = action(project);
292-
outputs.push(...theseOutputs.filter(output => !outputs.some(o => areEqual(o, output))));
300+
addToCombineOutputResult(outputs, project, filter(theseOutputs, output => !combineOutputResultContains(outputs, output, areEqual)));
293301
});
294302
return outputs;
295303
}
296304

305+
function flattenCombineOutputResult<T>(outputs: CombineOutputResult<T>): readonly T[] {
306+
return flatMap(outputs, ({ result }) => result);
307+
}
308+
297309
function combineProjectOutputWhileOpeningReferencedProjects<T>(
298310
projects: Projects,
299311
defaultProject: Project,
300312
action: (project: Project) => readonly T[],
301313
getLocation: (t: T) => DocumentPosition,
302314
resultsEqual: (a: T, b: T) => boolean,
303-
): T[] {
304-
const outputs: T[] = [];
315+
): CombineOutputResult<T> {
316+
const outputs: CombineOutputResult<T> = [];
305317
combineProjectOutputWorker(
306318
projects,
307319
defaultProject,
308320
/*initialLocation*/ undefined,
309321
(project, _, tryAddToTodo) => {
310-
for (const output of action(project)) {
311-
if (!contains(outputs, output, resultsEqual) && !tryAddToTodo(project, getLocation(output))) {
312-
outputs.push(output);
313-
}
314-
}
322+
const theseOutputs = action(project);
323+
addToCombineOutputResult(outputs, project, filter(theseOutputs, output => !combineOutputResultContains(outputs, output, resultsEqual) && !tryAddToTodo(project, getLocation(output))));
315324
},
316325
);
317326
return outputs;
@@ -326,7 +335,6 @@ namespace ts.server {
326335
hostPreferences: UserPreferences
327336
): readonly RenameLocation[] {
328337
const outputs: RenameLocation[] = [];
329-
330338
combineProjectOutputWorker(
331339
projects,
332340
defaultProject,
@@ -1930,38 +1938,42 @@ namespace ts.server {
19301938

19311939
private getNavigateToItems(args: protocol.NavtoRequestArgs, simplifiedResult: boolean): readonly protocol.NavtoItem[] | readonly NavigateToItem[] {
19321940
const full = this.getFullNavigateToItems(args);
1933-
return !simplifiedResult ? full : full.map((navItem) => {
1934-
const { file, project } = this.getFileAndProject({ file: navItem.fileName });
1935-
const scriptInfo = project.getScriptInfo(file)!;
1936-
const bakedItem: protocol.NavtoItem = {
1937-
name: navItem.name,
1938-
kind: navItem.kind,
1939-
kindModifiers: navItem.kindModifiers,
1940-
isCaseSensitive: navItem.isCaseSensitive,
1941-
matchKind: navItem.matchKind,
1942-
file: navItem.fileName,
1943-
start: scriptInfo.positionToLineOffset(navItem.textSpan.start),
1944-
end: scriptInfo.positionToLineOffset(textSpanEnd(navItem.textSpan))
1945-
};
1946-
if (navItem.kindModifiers && (navItem.kindModifiers !== "")) {
1947-
bakedItem.kindModifiers = navItem.kindModifiers;
1948-
}
1949-
if (navItem.containerName && (navItem.containerName.length > 0)) {
1950-
bakedItem.containerName = navItem.containerName;
1951-
}
1952-
if (navItem.containerKind && (navItem.containerKind.length > 0)) {
1953-
bakedItem.containerKind = navItem.containerKind;
1954-
}
1955-
return bakedItem;
1956-
});
1941+
return !simplifiedResult ?
1942+
flattenCombineOutputResult(full) :
1943+
flatMap(
1944+
full,
1945+
({ project, result }) => result.map(navItem => {
1946+
const scriptInfo = project.getScriptInfo(navItem.fileName)!;
1947+
const bakedItem: protocol.NavtoItem = {
1948+
name: navItem.name,
1949+
kind: navItem.kind,
1950+
kindModifiers: navItem.kindModifiers,
1951+
isCaseSensitive: navItem.isCaseSensitive,
1952+
matchKind: navItem.matchKind,
1953+
file: navItem.fileName,
1954+
start: scriptInfo.positionToLineOffset(navItem.textSpan.start),
1955+
end: scriptInfo.positionToLineOffset(textSpanEnd(navItem.textSpan))
1956+
};
1957+
if (navItem.kindModifiers && (navItem.kindModifiers !== "")) {
1958+
bakedItem.kindModifiers = navItem.kindModifiers;
1959+
}
1960+
if (navItem.containerName && (navItem.containerName.length > 0)) {
1961+
bakedItem.containerName = navItem.containerName;
1962+
}
1963+
if (navItem.containerKind && (navItem.containerKind.length > 0)) {
1964+
bakedItem.containerKind = navItem.containerKind;
1965+
}
1966+
return bakedItem;
1967+
})
1968+
);
19571969
}
19581970

1959-
private getFullNavigateToItems(args: protocol.NavtoRequestArgs): readonly NavigateToItem[] {
1971+
private getFullNavigateToItems(args: protocol.NavtoRequestArgs): CombineOutputResult<NavigateToItem> {
19601972
const { currentFileOnly, searchValue, maxResultCount, projectFileName } = args;
19611973
if (currentFileOnly) {
19621974
Debug.assertDefined(args.file);
19631975
const { file, project } = this.getFileAndProject(args as protocol.FileRequestArgs);
1964-
return project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file);
1976+
return [{ project, result: project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file) }];
19651977
}
19661978
else if (!args.file && !projectFileName) {
19671979
return combineProjectOutputFromEveryProject(
@@ -2082,10 +2094,13 @@ namespace ts.server {
20822094
const newPath = toNormalizedPath(args.newFilePath);
20832095
const formatOptions = this.getHostFormatOptions();
20842096
const preferences = this.getHostPreferences();
2085-
const changes = combineProjectOutputFromEveryProject(
2086-
this.projectService,
2087-
project => project.getLanguageService().getEditsForFileRename(oldPath, newPath, formatOptions, preferences),
2088-
(a, b) => a.fileName === b.fileName);
2097+
const changes = flattenCombineOutputResult(
2098+
combineProjectOutputFromEveryProject(
2099+
this.projectService,
2100+
project => project.getLanguageService().getEditsForFileRename(oldPath, newPath, formatOptions, preferences),
2101+
(a, b) => a.fileName === b.fileName
2102+
)
2103+
);
20892104
return simplifiedResult ? changes.map(c => this.mapTextChangeToCodeEdit(c)) : changes;
20902105
}
20912106

Diff for: src/testRunner/unittests/tsserver/navTo.ts

+61-1
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,75 @@ namespace ts.projectSystem {
6161
export const ghijkl = a.abcdef;`
6262
};
6363
const host = createServerHost([configFile1, file1, configFile2, file2]);
64-
const session = createSession(host);
64+
const logger = createHasErrorMessageLogger().logger;
65+
const logs: string[] = [];
66+
logger.info = s => logs.push(s);
67+
const session = createSession(host, { logger });
6568
openFilesForSession([file1, file2], session);
69+
logs.length = 0;
6670

6771
const request = makeSessionRequest<protocol.NavtoRequestArgs>(CommandNames.Navto, { searchValue: "abcdef", file: file1.path });
6872
const items = session.executeCommand(request).response as protocol.NavtoItem[];
6973
assert.strictEqual(items.length, 1);
7074
const item = items[0];
7175
assert.strictEqual(item.name, "abcdef");
7276
assert.strictEqual(item.file, file1.path);
77+
assert.deepEqual(logs, []);
78+
});
79+
80+
it("should de-duplicate symbols when searching all projects", () => {
81+
const solutionConfig: File = {
82+
path: "/tsconfig.json",
83+
content: JSON.stringify({
84+
references: [{ path: "./a" }, { path: "./b" }],
85+
files: [],
86+
})
87+
};
88+
const configFile1: File = {
89+
path: "/a/tsconfig.json",
90+
content: `{
91+
"compilerOptions": {
92+
"composite": true
93+
}
94+
}`
95+
};
96+
const file1: File = {
97+
path: "/a/index.ts",
98+
content: "export const abcdef = 1;"
99+
};
100+
const configFile2: File = {
101+
path: "/b/tsconfig.json",
102+
content: `{
103+
"compilerOptions": {
104+
"composite": true
105+
},
106+
"references": [
107+
{ "path": "../a" }
108+
]
109+
}`
110+
};
111+
const file2: File = {
112+
path: "/b/index.ts",
113+
content: `import a = require("../a");
114+
export const ghijkl = a.abcdef;`
115+
};
116+
const host = createServerHost([configFile1, file1, configFile2, file2, solutionConfig]);
117+
const logger = createHasErrorMessageLogger().logger;
118+
const logs: string[] = [];
119+
logger.info = s => logs.push(s);
120+
const session = createSession(host, { logger });
121+
openFilesForSession([file1], session);
122+
logs.length = 0;
123+
124+
const request = makeSessionRequest<protocol.NavtoRequestArgs>(CommandNames.Navto, { searchValue: "abcdef" });
125+
const items = session.executeCommand(request).response as protocol.NavtoItem[];
126+
assert.strictEqual(items.length, 1);
127+
const item = items[0];
128+
assert.strictEqual(item.name, "abcdef");
129+
assert.strictEqual(item.file, file1.path);
130+
// Cannt check logs as empty since it loads projects and writes information for those in the log
131+
assert.isFalse(contains(logs, "Search path: /a"));
132+
assert.isFalse(contains(logs, "For info: /a/index.ts :: Config file name: /a/tsconfig.json"));
73133
});
74134

75135
it("should work with Deprecated", () => {

0 commit comments

Comments
 (0)