Skip to content

Commit 60b19f5

Browse files
sheetalkamatsheetalkamat
committed
Invalidate the unresolved import resolutions when typing files are set
This has 3 changes: 1. In updateGraph when enqueue the typing installation request (depending on unresolved imports) 2. When ActionSet event is received, invalidate only files with unresolved imports and resolve those. 3. When ActionInvalidate event is received, typing installer has detected some change in global typing cache location, so just enqueue a new typing installation request. This will repeat the cycle of setting correct typings and pickiing unresolved imports
1 parent 35abe26 commit 60b19f5

7 files changed

Lines changed: 153 additions & 45 deletions

File tree

‎src/compiler/resolutionCache.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace ts {
1010

1111
invalidateResolutionOfFile(filePath: Path): void;
1212
removeResolutionsOfFile(filePath: Path): void;
13+
setFilesWithInvalidatedNonRelativeUnresolvedImports(filesWithUnresolvedImports: Map<any>): void;
1314
createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution;
1415

1516
startCachingPerDirectoryResolution(): void;
@@ -74,6 +75,7 @@ namespace ts {
7475
export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootDirForResolution: string, logChangesWhenResolvingModule: boolean): ResolutionCache {
7576
let filesWithChangedSetOfUnresolvedImports: Path[] | undefined;
7677
let filesWithInvalidatedResolutions: Map<true> | undefined;
78+
let filesWithInvalidatedNonRelativeUnresolvedImports: Map<any> | undefined;
7779
let allFilesHaveInvalidatedResolution = false;
7880

7981
const getCurrentDirectory = memoize(() => resolutionHost.getCurrentDirectory());
@@ -122,6 +124,7 @@ namespace ts {
122124
resolveTypeReferenceDirectives,
123125
removeResolutionsOfFile,
124126
invalidateResolutionOfFile,
127+
setFilesWithInvalidatedNonRelativeUnresolvedImports,
125128
createHasInvalidatedResolution,
126129
updateTypeRootsWatch,
127130
closeTypeRootsWatch,
@@ -173,7 +176,8 @@ namespace ts {
173176
}
174177
const collected = filesWithInvalidatedResolutions;
175178
filesWithInvalidatedResolutions = undefined;
176-
return path => collected && collected.has(path);
179+
return path => (collected && collected.has(path)) ||
180+
(filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path));
177181
}
178182

179183
function clearPerDirectoryResolutions() {
@@ -184,6 +188,7 @@ namespace ts {
184188

185189
function finishCachingPerDirectoryResolution() {
186190
allFilesHaveInvalidatedResolution = false;
191+
filesWithInvalidatedNonRelativeUnresolvedImports = undefined;
187192
directoryWatchesOfFailedLookups.forEach((watcher, path) => {
188193
if (watcher.refCount === 0) {
189194
directoryWatchesOfFailedLookups.delete(path);
@@ -237,13 +242,15 @@ namespace ts {
237242

238243
const resolvedModules: R[] = [];
239244
const compilerOptions = resolutionHost.getCompilationSettings();
240-
245+
const hasInvalidatedNonRelativeUnresolvedImport = logChanges && filesWithInvalidatedNonRelativeUnresolvedImports && filesWithInvalidatedNonRelativeUnresolvedImports.has(path);
241246
const seenNamesInFile = createMap<true>();
242247
for (const name of names) {
243248
let resolution = resolutionsInFile.get(name);
244249
// Resolution is valid if it is present and not invalidated
245250
if (!seenNamesInFile.has(name) &&
246-
allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated) {
251+
allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated ||
252+
// If the name is unresolved import that was invalidated, recalculate
253+
(hasInvalidatedNonRelativeUnresolvedImport && !isExternalModuleNameRelative(name) && !getResolutionWithResolvedFileName(resolution))) {
247254
const existingResolution = resolution;
248255
const resolutionInDirectory = perDirectoryResolution.get(name);
249256
if (resolutionInDirectory) {
@@ -284,7 +291,7 @@ namespace ts {
284291
if (oldResolution === newResolution) {
285292
return true;
286293
}
287-
if (!oldResolution || !newResolution || oldResolution.isInvalidated) {
294+
if (!oldResolution || !newResolution) {
288295
return false;
289296
}
290297
const oldResult = getResolutionWithResolvedFileName(oldResolution);
@@ -577,6 +584,11 @@ namespace ts {
577584
);
578585
}
579586

587+
function setFilesWithInvalidatedNonRelativeUnresolvedImports(filesMap: Map<any>) {
588+
Debug.assert(filesWithInvalidatedNonRelativeUnresolvedImports === filesMap || filesWithInvalidatedNonRelativeUnresolvedImports === undefined);
589+
filesWithInvalidatedNonRelativeUnresolvedImports = filesMap;
590+
}
591+
580592
function invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath: Path, isCreatingWatchedDirectory: boolean) {
581593
let isChangedFailedLookupLocation: (location: string) => boolean;
582594
if (isCreatingWatchedDirectory) {

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7294,7 +7294,6 @@ namespace ts.projectSystem {
72947294
const host = createServerHost(files);
72957295
const session = createSession(host);
72967296
const projectService = session.getProjectService();
7297-
debugger;
72987297
session.executeCommandSeq<protocol.OpenRequest>({
72997298
command: protocol.CommandTypes.Open,
73007299
arguments: {

‎src/harness/unittests/typingsInstaller.ts‎

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,7 @@ namespace ts.projectSystem {
10061006
installer.installAll(/*expectedCount*/ 1);
10071007
});
10081008

1009-
it("should recompute resolutions after typings are installed", () => {
1009+
it("cached unresolved typings are not recomputed if program structure did not change", () => {
10101010
const host = createServerHost([]);
10111011
const session = createSession(host);
10121012
const f = {
@@ -1048,7 +1048,7 @@ namespace ts.projectSystem {
10481048
session.executeCommand(changeRequest);
10491049
host.checkTimeoutQueueLengthAndRun(2); // This enqueues the updategraph and refresh inferred projects
10501050
const version2 = proj.lastCachedUnresolvedImportsList;
1051-
assert.notEqual(version1, version2, "set of unresolved imports should change");
1051+
assert.strictEqual(version1, version2, "set of unresolved imports should change");
10521052
});
10531053

10541054
it("expired cache entry (inferred project, should install typings)", () => {
@@ -1621,4 +1621,75 @@ namespace ts.projectSystem {
16211621
assert.deepEqual(commands, expectedCommands, "commands");
16221622
});
16231623
});
1624+
1625+
describe("recomputing resolutions of unresolved imports", () => {
1626+
const globalTypingsCacheLocation = "/tmp";
1627+
const appPath = "/a/b/app.js" as Path;
1628+
const foooPath = "/a/b/node_modules/fooo/index.d.ts";
1629+
function verifyResolvedModuleOfFooo(project: server.Project) {
1630+
const foooResolution = project.getLanguageService().getProgram().getSourceFileByPath(appPath).resolvedModules.get("fooo");
1631+
assert.equal(foooResolution.resolvedFileName, foooPath);
1632+
return foooResolution;
1633+
}
1634+
1635+
function verifyUnresolvedImportResolutions(appContents: string, typingNames: string[], typingFiles: FileOrFolder[]) {
1636+
const app: FileOrFolder = {
1637+
path: appPath,
1638+
content: `${appContents}import * as x from "fooo";`
1639+
};
1640+
const fooo: FileOrFolder = {
1641+
path: foooPath,
1642+
content: `export var x: string;`
1643+
};
1644+
const host = createServerHost([app, fooo]);
1645+
const installer = new (class extends Installer {
1646+
constructor() {
1647+
super(host, { globalTypingsCacheLocation, typesRegistry: createTypesRegistry("foo") });
1648+
}
1649+
installWorker(_requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction) {
1650+
executeCommand(this, host, typingNames, typingFiles, cb);
1651+
}
1652+
})();
1653+
const projectService = createProjectService(host, { typingsInstaller: installer });
1654+
projectService.openClientFile(app.path);
1655+
projectService.checkNumberOfProjects({ inferredProjects: 1 });
1656+
1657+
const proj = projectService.inferredProjects[0];
1658+
checkProjectActualFiles(proj, [app.path, fooo.path]);
1659+
const foooResolution1 = verifyResolvedModuleOfFooo(proj);
1660+
1661+
installer.installAll(/*expectedCount*/ 1);
1662+
host.checkTimeoutQueueLengthAndRun(2);
1663+
checkProjectActualFiles(proj, typingFiles.map(f => f.path).concat(app.path, fooo.path));
1664+
const foooResolution2 = verifyResolvedModuleOfFooo(proj);
1665+
assert.strictEqual(foooResolution1, foooResolution2);
1666+
}
1667+
1668+
it("correctly invalidate the resolutions with typing names", () => {
1669+
verifyUnresolvedImportResolutions('import * as a from "foo";', ["foo"], [{
1670+
path: `${globalTypingsCacheLocation}/node_modules/foo/index.d.ts`,
1671+
content: "export function a(): void;"
1672+
}]);
1673+
});
1674+
1675+
it("correctly invalidate the resolutions with typing names that are trimmed", () => {
1676+
const fooAA: FileOrFolder = {
1677+
path: `${globalTypingsCacheLocation}/node_modules/foo/a/a.d.ts`,
1678+
content: "export function a (): void;"
1679+
};
1680+
const fooAB: FileOrFolder = {
1681+
path: `${globalTypingsCacheLocation}/node_modules/foo/a/b.d.ts`,
1682+
content: "export function b (): void;"
1683+
};
1684+
const fooAC: FileOrFolder = {
1685+
path: `${globalTypingsCacheLocation}/node_modules/foo/a/c.d.ts`,
1686+
content: "export function c (): void;"
1687+
};
1688+
verifyUnresolvedImportResolutions(`
1689+
import * as a from "foo/a/a";
1690+
import * as b from "foo/a/b";
1691+
import * as c from "foo/a/c";
1692+
`, ["foo"], [fooAA, fooAB, fooAC]);
1693+
});
1694+
});
16241695
}

‎src/server/editorServices.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -524,13 +524,13 @@ namespace ts.server {
524524
}
525525
switch (response.kind) {
526526
case ActionSet:
527-
project.resolutionCache.clear();
528-
this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typeAcquisition, response.unresolvedImports, response.typings);
527+
// Update the typing files and update the project
528+
project.updateTypingFiles(this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typeAcquisition, response.unresolvedImports, response.typings));
529529
break;
530530
case ActionInvalidate:
531-
project.resolutionCache.clear();
532-
this.typingsCache.deleteTypingsForProject(response.projectName);
533-
break;
531+
// Do not clear resolution cache, there was changes detected in typings, so enque typing request and let it get us correct results
532+
this.typingsCache.enqueueInstallTypingsForProject(project, project.lastCachedUnresolvedImportsList, /*forceRefresh*/ true);
533+
return;
534534
}
535535
this.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(project);
536536
}

‎src/server/project.ts‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,18 @@ namespace ts.server {
8989
private plugins: PluginModule[] = [];
9090

9191
/*@internal*/
92+
/**
93+
* This is map from files to unresolved imports in it
94+
* Maop does not contain entries for files that do not have unresolved imports
95+
* This helps in containing the set of files to invalidate
96+
*/
9297
cachedUnresolvedImportsPerFile = createMap<ReadonlyArray<string>>();
98+
99+
/**
100+
* This is the set that has entry to true if file doesnt contain any unresolved import
101+
*/
102+
private filesWithNoUnresolvedImports = createMap<true>();
103+
93104
/*@internal*/
94105
lastCachedUnresolvedImportsList: SortedReadonlyArray<string>;
95106
/*@internal*/
@@ -143,7 +154,8 @@ namespace ts.server {
143154
/*@internal*/
144155
hasChangedAutomaticTypeDirectiveNames = false;
145156

146-
private typingFiles: SortedReadonlyArray<string>;
157+
/*@internal*/
158+
typingFiles: SortedReadonlyArray<string> = emptyArray;
147159

148160
private readonly cancellationToken: ThrottledCancellationToken;
149161

@@ -554,6 +566,7 @@ namespace ts.server {
554566
this.resolutionCache.clear();
555567
this.resolutionCache = undefined;
556568
this.cachedUnresolvedImportsPerFile = undefined;
569+
this.filesWithNoUnresolvedImports = undefined;
557570
this.directoryStructureHost = undefined;
558571

559572
// Clean up file watchers waiting for missing files
@@ -714,6 +727,7 @@ namespace ts.server {
714727
else {
715728
this.resolutionCache.invalidateResolutionOfFile(info.path);
716729
}
730+
this.filesWithNoUnresolvedImports.delete(info.path);
717731
this.cachedUnresolvedImportsPerFile.delete(info.path);
718732

719733
if (detachFromProject) {
@@ -735,16 +749,21 @@ namespace ts.server {
735749
}
736750

737751
/* @internal */
738-
private extractUnresolvedImportsFromSourceFile(file: SourceFile, result: Push<string>, ambientModules: string[]) {
752+
private extractUnresolvedImportsFromSourceFile(file: SourceFile, result: string[] | undefined, ambientModules: string[]): string[] | undefined {
753+
// No unresolve imports in this file
754+
if (this.filesWithNoUnresolvedImports.has(file.path)) {
755+
return result;
756+
}
757+
739758
const cached = this.cachedUnresolvedImportsPerFile.get(file.path);
740759
if (cached) {
741760
// found cached result - use it and return
742761
for (const f of cached) {
743-
result.push(f);
762+
(result || (result = [])).push(f);
744763
}
745-
return;
764+
return result;
746765
}
747-
let unresolvedImports: string[];
766+
let unresolvedImports: string[] | undefined;
748767
if (file.resolvedModules) {
749768
file.resolvedModules.forEach((resolvedModule, name) => {
750769
// pick unresolved non-relative names
@@ -760,11 +779,17 @@ namespace ts.server {
760779
trimmed = trimmed.substr(0, i);
761780
}
762781
(unresolvedImports || (unresolvedImports = [])).push(trimmed);
763-
result.push(trimmed);
782+
(result || (result = [])).push(trimmed);
764783
}
765784
});
766785
}
767-
this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports || emptyArray);
786+
if (unresolvedImports) {
787+
this.cachedUnresolvedImportsPerFile.set(file.path, unresolvedImports);
788+
}
789+
else {
790+
this.filesWithNoUnresolvedImports.set(file.path, true);
791+
}
792+
return result;
768793

769794
function isAmbientlyDeclaredModule(name: string) {
770795
return ambientModules.some(m => m === name);
@@ -778,7 +803,7 @@ namespace ts.server {
778803
updateGraph(): boolean {
779804
this.resolutionCache.startRecordingFilesWithChangedResolutions();
780805

781-
let hasChanges = this.updateGraphWorker();
806+
const hasChanges = this.updateGraphWorker();
782807
const hasMoreOrLessScriptInfos = this.hasMoreOrLessScriptInfos;
783808
this.hasMoreOrLessScriptInfos = false;
784809

@@ -787,6 +812,7 @@ namespace ts.server {
787812
for (const file of changedFiles) {
788813
// delete cached information for changed files
789814
this.cachedUnresolvedImportsPerFile.delete(file);
815+
this.filesWithNoUnresolvedImports.delete(file);
790816
}
791817

792818
// update builder only if language service is enabled
@@ -799,20 +825,15 @@ namespace ts.server {
799825
// (can reuse cached imports for files that were not changed)
800826
// 4. compilation settings were changed in the way that might affect module resolution - drop all caches and collect all data from the scratch
801827
if (hasChanges || changedFiles.length) {
802-
const result: string[] = [];
828+
let result: string[] | undefined;
803829
const ambientModules = this.program.getTypeChecker().getAmbientModules().map(mod => stripQuotes(mod.getName()));
804830
for (const sourceFile of this.program.getSourceFiles()) {
805-
this.extractUnresolvedImportsFromSourceFile(sourceFile, result, ambientModules);
831+
result = this.extractUnresolvedImportsFromSourceFile(sourceFile, result, ambientModules);
806832
}
807-
this.lastCachedUnresolvedImportsList = toDeduplicatedSortedArray(result);
833+
this.lastCachedUnresolvedImportsList = result ? toDeduplicatedSortedArray(result) : emptyArray;
808834
}
809835

810-
const cachedTypings = this.projectService.typingsCache.getTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessScriptInfos);
811-
if (!arrayIsEqualTo(this.typingFiles, cachedTypings)) {
812-
this.typingFiles = cachedTypings;
813-
this.markAsDirty();
814-
hasChanges = this.updateGraphWorker() || hasChanges;
815-
}
836+
this.projectService.typingsCache.enqueueInstallTypingsForProject(this, this.lastCachedUnresolvedImportsList, hasMoreOrLessScriptInfos);
816837
}
817838
else {
818839
this.lastCachedUnresolvedImportsList = undefined;
@@ -824,6 +845,13 @@ namespace ts.server {
824845
return !hasChanges;
825846
}
826847

848+
/*@internal*/
849+
updateTypingFiles(typingFiles: SortedReadonlyArray<string>) {
850+
this.typingFiles = typingFiles;
851+
// Invalidate files with unresolved imports
852+
this.resolutionCache.setFilesWithInvalidatedNonRelativeUnresolvedImports(this.cachedUnresolvedImportsPerFile);
853+
}
854+
827855
/* @internal */
828856
getCurrentProgram() {
829857
return this.program;
@@ -959,15 +987,14 @@ namespace ts.server {
959987
setCompilerOptions(compilerOptions: CompilerOptions) {
960988
if (compilerOptions) {
961989
compilerOptions.allowNonTsExtensions = true;
962-
if (changesAffectModuleResolution(this.compilerOptions, compilerOptions)) {
963-
// reset cached unresolved imports if changes in compiler options affected module resolution
964-
this.cachedUnresolvedImportsPerFile.clear();
965-
this.lastCachedUnresolvedImportsList = undefined;
966-
}
967990
const oldOptions = this.compilerOptions;
968991
this.compilerOptions = compilerOptions;
969992
this.setInternalCompilerOptionsForEmittingJsFiles();
970993
if (changesAffectModuleResolution(oldOptions, compilerOptions)) {
994+
// reset cached unresolved imports if changes in compiler options affected module resolution
995+
this.cachedUnresolvedImportsPerFile.clear();
996+
this.filesWithNoUnresolvedImports.clear();
997+
this.lastCachedUnresolvedImportsList = undefined;
971998
this.resolutionCache.clear();
972999
}
9731000
this.markAsDirty();

0 commit comments

Comments
 (0)