Skip to content

Commit

Permalink
Only when typings file change for the project, schedule the update fo…
Browse files Browse the repository at this point in the history 
…r the project (#42428)

* Update and add test when typings dont change because of import name

* Update project scheduling only when typings are set

* Schedule update graph only if typings change
Fixes #39326
  • Loading branch information
@sheetalkamat
sheetalkamat authored Jan 22, 2021
1 parent 4c62f6e commit 80dfc6a
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 27 deletions.
3 changes: 3 additions & 0 deletions src/compiler/resolutionCache.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace ts {
setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: ESMap<Path, readonly string[]>): void;
createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution;
hasChangedAutomaticTypeDirectiveNames(): boolean;
isFileWithInvalidatedNonRelativeUnresolvedImports(path: Path): boolean;


startCachingPerDirectoryResolution(): void;
finishCachingPerDirectoryResolution(): void;
Expand Down Expand Up @@ -208,6 +210,7 @@ namespace ts {
invalidateResolutionsOfFailedLookupLocations,
setFilesWithInvalidatedNonRelativeUnresolvedImports,
createHasInvalidatedResolution,
isFileWithInvalidatedNonRelativeUnresolvedImports,
updateTypeRootsWatch,
closeTypeRootsWatch,
clear
Expand Down
3 changes: 1 addition & 2 deletions src/server/editorServices.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -922,13 +922,12 @@ 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));
break;
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
this.typingsCache.enqueueInstallTypingsForProject(project, project.lastCachedUnresolvedImportsList, /*forceRefresh*/ true);
return;
}
this.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(project);
}

/*@internal*/
Expand Down
13 changes: 8 additions & 5 deletions src/server/project.ts
View file
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
View file
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
View file
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
View file
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
46 changes: 36 additions & 10 deletions src/testRunner/unittests/tsserver/typingsInstaller.ts
View file
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 @@ -1837,6 +1837,7 @@ namespace ts.projectSystem {
const appPath = "/a/b/app.js" as Path;
const foooPath = "/a/b/node_modules/fooo/index.d.ts";
function verifyResolvedModuleOfFooo(project: server.Project) {
server.updateProjectIfDirty(project);
const foooResolution = project.getLanguageService().getProgram()!.getSourceFileByPath(appPath)!.resolvedModules!.get("fooo")!;
assert.equal(foooResolution.resolvedFileName, foooPath);
return foooResolution;
Expand All @@ -1851,6 +1852,7 @@ namespace ts.projectSystem {
path: foooPath,
content: `export var x: string;`
};

const host = createServerHost([app, fooo]);
const installer = new (class extends Installer {
constructor() {
Expand All @@ -1873,6 +1875,17 @@ namespace ts.projectSystem {
checkProjectActualFiles(proj, typingFiles.map(f => f.path).concat(app.path, fooo.path));
const foooResolution2 = verifyResolvedModuleOfFooo(proj);
assert.strictEqual(foooResolution1, foooResolution2);
projectService.applyChangesInOpenFiles(/*openFiles*/ undefined, arrayIterator([{
fileName: app.path,
changes: arrayIterator([{
span: { start: 0, length: 0 },
newText: `import * as bar from "bar";`
}])
}]));
host.runQueuedTimeoutCallbacks(); // Update the graph
// Update the typing
host.checkTimeoutQueueLength(0);
assert.isFalse(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(app.path as Path));
}

it("correctly invalidate the resolutions with typing names", () => {
Expand All @@ -1883,6 +1896,10 @@ namespace ts.projectSystem {
});

it("correctly invalidate the resolutions with typing names that are trimmed", () => {
const fooIndex: File = {
path: `${globalTypingsCacheLocation}/node_modules/foo/index.d.ts`,
content: "export function aa(): void;"
};
const fooAA: File = {
path: `${globalTypingsCacheLocation}/node_modules/foo/a/a.d.ts`,
content: "export function a (): void;"
Expand All @@ -1899,7 +1916,7 @@ namespace ts.projectSystem {
import * as a from "foo/a/a";
import * as b from "foo/a/b";
import * as c from "foo/a/c";
`, ["foo"], [fooAA, fooAB, fooAC]);
`, ["foo"], [fooIndex, fooAA, fooAB, fooAC]);
});

it("should handle node core modules", () => {
Expand Down Expand Up @@ -1958,12 +1975,21 @@ 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,
changes: arrayIterator([{
span: { start: file.content.indexOf("const"), length: 0 },
newText: `const bar = require("bar");`
}])
}]));
proj.updateGraph(); // Update the graph
checkProjectActualFiles(proj, [file.path, libFile.path, nodeTyping.path]);
// Update the typing
host.checkTimeoutQueueLength(0);
assert.isFalse(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(file.path as Path));
});
});

Expand Down

0 comments on commit 80dfc6a

Please sign in to comment.