Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only when typings file change for the project, schedule the update for the project #42428

Merged
merged 3 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/compiler/resolutionCache.ts
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
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
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
46 changes: 36 additions & 10 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 @@ -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