Skip to content

Commit

Permalink
Schedule update graph only if typings change
Browse files Browse the repository at this point in the history
Fixes #39326
  • Loading branch information
sheetalkamat committed Jan 21, 2021
1 parent d606b70 commit 6d543ea
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 30 deletions.
1 change: 0 additions & 1 deletion src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,6 @@ namespace ts.server {
case ActionSet:
// Update the typing files and update the project
project.updateTypingFiles(this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typeAcquisition, response.unresolvedImports, response.typings));
this.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(project);
return;
case ActionInvalidate:
// Do not clear resolution cache, there was changes detected in typings, so enque typing request and let it get us correct results
Expand Down
13 changes: 8 additions & 5 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1057,13 +1057,16 @@ namespace ts.server {

/*@internal*/
updateTypingFiles(typingFiles: SortedReadonlyArray<string>) {
enumerateInsertsAndDeletes<string, string>(typingFiles, this.typingFiles, getStringComparer(!this.useCaseSensitiveFileNames()),
if (enumerateInsertsAndDeletes<string, string>(typingFiles, this.typingFiles, getStringComparer(!this.useCaseSensitiveFileNames()),
/*inserted*/ noop,
removed => this.detachScriptInfoFromProject(removed)
);
this.typingFiles = typingFiles;
// Invalidate files with unresolved imports
this.resolutionCache.setFilesWithInvalidatedNonRelativeUnresolvedImports(this.cachedUnresolvedImportsPerFile);
)) {
// If typing files changed, then only schedule project update
this.typingFiles = typingFiles;
// Invalidate files with unresolved imports
this.resolutionCache.setFilesWithInvalidatedNonRelativeUnresolvedImports(this.cachedUnresolvedImportsPerFile);
this.projectService.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(this);
}
}

/* @internal */
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tsserver/projectErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ namespace ts.projectSystem {
checkErrors([serverUtilities.path, app.path]);

function checkErrors(openFiles: [string, string]) {
verifyGetErrRequestNoErrors({ session, host, files: openFiles, existingTimeouts: 2 });
verifyGetErrRequestNoErrors({ session, host, files: openFiles });
}
});

Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/tsserver/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ namespace ts.projectSystem {
unresolvedImports: response.unresolvedImports,
});

host.checkTimeoutQueueLengthAndRun(1);
host.checkTimeoutQueueLength(0);
assert.isUndefined(request);
});

Expand Down
10 changes: 2 additions & 8 deletions src/testRunner/unittests/tsserver/resolutionCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,7 @@ namespace ts.projectSystem {

checkNumberOfProjects(service, { inferredProjects: 1 });
session.clearMessages();
host.checkTimeoutQueueLengthAndRun(2);

checkProjectUpdatedInBackgroundEvent(session, [file.path]);

host.checkTimeoutQueueLength(0);
verifyGetErrRequest({
session,
host,
Expand Down Expand Up @@ -240,10 +237,7 @@ namespace ts.projectSystem {

checkNumberOfProjects(service, { inferredProjects: 1 });
session.clearMessages();
host.checkTimeoutQueueLengthAndRun(2);

checkProjectUpdatedInBackgroundEvent(session, [file.path]);

host.checkTimeoutQueueLength(0);
verifyGetErrRequest({
session,
host,
Expand Down
26 changes: 12 additions & 14 deletions src/testRunner/unittests/tsserver/typingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ namespace ts.projectSystem {
checkProjectActualFiles(p, [jqueryJs.path]);

installer.installAll(/*expectedCount*/ 0);
host.checkTimeoutQueueLengthAndRun(2);
host.checkTimeoutQueueLength(0);
checkNumberOfProjects(projectService, { inferredProjects: 1 });
// files should not be removed from project if ATA is skipped
checkProjectActualFiles(p, [jqueryJs.path]);
Expand Down Expand Up @@ -1024,9 +1024,8 @@ namespace ts.projectSystem {
service.openClientFile(f.path);

installer.checkPendingCommands(/*expectedCount*/ 0);

host.writeFile(fixedPackageJson.path, fixedPackageJson.content);
host.checkTimeoutQueueLengthAndRun(2); // To refresh the project and refresh inferred projects
host.checkTimeoutQueueLength(0);
// expected install request
installer.installAll(/*expectedCount*/ 1);
host.checkTimeoutQueueLengthAndRun(2);
Expand Down Expand Up @@ -1212,7 +1211,8 @@ namespace ts.projectSystem {
}
};
session.executeCommand(changeRequest);
host.checkTimeoutQueueLengthAndRun(2); // This enqueues the updategraph and refresh inferred projects
host.checkTimeoutQueueLength(0);
proj.updateGraph();
const version2 = proj.lastCachedUnresolvedImportsList;
assert.strictEqual(version1, version2, "set of unresolved imports should change");
});
Expand Down Expand Up @@ -1884,8 +1884,8 @@ namespace ts.projectSystem {
}]));
host.runQueuedTimeoutCallbacks(); // Update the graph
// Update the typing
host.checkTimeoutQueueLength(2);
assert.isTrue(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(app.path as Path));
host.checkTimeoutQueueLength(0);
assert.isFalse(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(app.path as Path));
}

it("correctly invalidate the resolutions with typing names", () => {
Expand Down Expand Up @@ -1947,7 +1947,7 @@ declare module "stream" {
executeCommand(this, host, ["node"], [nodeTyping], cb);
}
})();
const projectService = createProjectService(host, { typingsInstaller: installer, logger: createLoggerWritingToConsole() });
const projectService = createProjectService(host, { typingsInstaller: installer });
projectService.openClientFile(file.path);
projectService.checkNumberOfProjects({ inferredProjects: 1 });

Expand Down Expand Up @@ -1975,11 +1975,8 @@ declare module "stream" {
host.checkTimeoutQueueLengthAndRun(2);
checkProjectActualFiles(proj, [file.path, libFile.path, nodeTyping.path]);

// Here, since typings doesnt contain node typings and resolution fails and
// node typings go out of project,
// but because we handle core node modules when resolving from typings cache
// node typings are included in the project
host.checkTimeoutQueueLengthAndRun(2);
// Here, since typings dont change, there is no timeout scheduled
host.checkTimeoutQueueLength(0);
checkProjectActualFiles(proj, [file.path, libFile.path, nodeTyping.path]);
projectService.applyChangesInOpenFiles(/*openFiles*/ undefined, arrayIterator([{
fileName: file.path,
Expand All @@ -1989,9 +1986,10 @@ declare module "stream" {
}])
}]));
proj.updateGraph(); // Update the graph
checkProjectActualFiles(proj, [file.path, libFile.path, nodeTyping.path]);
// Update the typing
host.checkTimeoutQueueLength(2);
assert.isTrue(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(file.path as Path));
host.checkTimeoutQueueLength(0);
assert.isFalse(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(file.path as Path));
});
});

Expand Down

0 comments on commit 6d543ea

Please sign in to comment.