Skip to content

Commit b0d6896

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request #23438 from Microsoft/typingsFiles
Better handling of typing installer events and consuming typing files in tsserver
2 parents a2f494a + 7c5f524 commit b0d6896

File tree

10 files changed

+199
-138
lines changed

10 files changed

+199
-138
lines changed

‎src/compiler/core.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2987,18 +2987,19 @@ namespace ts {
29872987
}
29882988

29892989
/** Remove the *first* occurrence of `item` from the array. */
2990-
export function unorderedRemoveItem<T>(array: T[], item: T): void {
2991-
unorderedRemoveFirstItemWhere(array, element => element === item);
2990+
export function unorderedRemoveItem<T>(array: T[], item: T) {
2991+
return unorderedRemoveFirstItemWhere(array, element => element === item);
29922992
}
29932993

29942994
/** Remove the *first* element satisfying `predicate`. */
2995-
function unorderedRemoveFirstItemWhere<T>(array: T[], predicate: (element: T) => boolean): void {
2995+
function unorderedRemoveFirstItemWhere<T>(array: T[], predicate: (element: T) => boolean) {
29962996
for (let i = 0; i < array.length; i++) {
29972997
if (predicate(array[i])) {
29982998
unorderedRemoveItemAt(array, i);
2999-
break;
2999+
return true;
30003000
}
30013001
}
3002+
return false;
30023003
}
30033004

30043005
export type GetCanonicalFileName = (fileName: string) => string;

‎src/compiler/resolutionCache.ts

Lines changed: 26 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<ReadonlyArray<string>>): 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<ReadonlyArray<string>> | 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,
@@ -165,6 +168,16 @@ namespace ts {
165168
return collected;
166169
}
167170

171+
function isFileWithInvalidatedNonRelativeUnresolvedImports(path: Path) {
172+
if (!filesWithInvalidatedNonRelativeUnresolvedImports) {
173+
return false;
174+
}
175+
176+
// Invalidated if file has unresolved imports
177+
const value = filesWithInvalidatedNonRelativeUnresolvedImports.get(path);
178+
return value && !!value.length;
179+
}
180+
168181
function createHasInvalidatedResolution(forceAllFilesAsInvalidated?: boolean): HasInvalidatedResolution {
169182
if (allFilesHaveInvalidatedResolution || forceAllFilesAsInvalidated) {
170183
// Any file asked would have invalidated resolution
@@ -173,7 +186,8 @@ namespace ts {
173186
}
174187
const collected = filesWithInvalidatedResolutions;
175188
filesWithInvalidatedResolutions = undefined;
176-
return path => collected && collected.has(path);
189+
return path => (collected && collected.has(path)) ||
190+
isFileWithInvalidatedNonRelativeUnresolvedImports(path);
177191
}
178192

179193
function clearPerDirectoryResolutions() {
@@ -184,6 +198,7 @@ namespace ts {
184198

185199
function finishCachingPerDirectoryResolution() {
186200
allFilesHaveInvalidatedResolution = false;
201+
filesWithInvalidatedNonRelativeUnresolvedImports = undefined;
187202
directoryWatchesOfFailedLookups.forEach((watcher, path) => {
188203
if (watcher.refCount === 0) {
189204
directoryWatchesOfFailedLookups.delete(path);
@@ -237,13 +252,15 @@ namespace ts {
237252

238253
const resolvedModules: R[] = [];
239254
const compilerOptions = resolutionHost.getCompilationSettings();
240-
255+
const hasInvalidatedNonRelativeUnresolvedImport = logChanges && isFileWithInvalidatedNonRelativeUnresolvedImports(path);
241256
const seenNamesInFile = createMap<true>();
242257
for (const name of names) {
243258
let resolution = resolutionsInFile.get(name);
244259
// Resolution is valid if it is present and not invalidated
245260
if (!seenNamesInFile.has(name) &&
246-
allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated) {
261+
allFilesHaveInvalidatedResolution || !resolution || resolution.isInvalidated ||
262+
// If the name is unresolved import that was invalidated, recalculate
263+
(hasInvalidatedNonRelativeUnresolvedImport && !isExternalModuleNameRelative(name) && !getResolutionWithResolvedFileName(resolution))) {
247264
const existingResolution = resolution;
248265
const resolutionInDirectory = perDirectoryResolution.get(name);
249266
if (resolutionInDirectory) {
@@ -284,7 +301,7 @@ namespace ts {
284301
if (oldResolution === newResolution) {
285302
return true;
286303
}
287-
if (!oldResolution || !newResolution || oldResolution.isInvalidated) {
304+
if (!oldResolution || !newResolution) {
288305
return false;
289306
}
290307
const oldResult = getResolutionWithResolvedFileName(oldResolution);
@@ -577,6 +594,11 @@ namespace ts {
577594
);
578595
}
579596

597+
function setFilesWithInvalidatedNonRelativeUnresolvedImports(filesMap: Map<ReadonlyArray<string>>) {
598+
Debug.assert(filesWithInvalidatedNonRelativeUnresolvedImports === filesMap || filesWithInvalidatedNonRelativeUnresolvedImports === undefined);
599+
filesWithInvalidatedNonRelativeUnresolvedImports = filesMap;
600+
}
601+
580602
function invalidateResolutionOfFailedLookupLocation(fileOrDirectoryPath: Path, isCreatingWatchedDirectory: boolean) {
581603
let isChangedFailedLookupLocation: (location: string) => boolean;
582604
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: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -999,14 +999,14 @@ namespace ts.projectSystem {
999999
proj.updateGraph();
10001000

10011001
assert.deepEqual(
1002-
proj.getCachedUnresolvedImportsPerFile_TestOnly().get(<Path>f1.path),
1002+
proj.cachedUnresolvedImportsPerFile.get(<Path>f1.path),
10031003
["foo", "foo", "foo", "@bar/router", "@bar/common", "@bar/common"]
10041004
);
10051005

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 = {
@@ -1029,7 +1029,7 @@ namespace ts.projectSystem {
10291029
const projectService = session.getProjectService();
10301030
checkNumberOfProjects(projectService, { inferredProjects: 1 });
10311031
const proj = projectService.inferredProjects[0];
1032-
const version1 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
1032+
const version1 = proj.lastCachedUnresolvedImportsList;
10331033

10341034
// make a change that should not affect the structure of the program
10351035
const changeRequest: server.protocol.ChangeRequest = {
@@ -1047,8 +1047,8 @@ namespace ts.projectSystem {
10471047
};
10481048
session.executeCommand(changeRequest);
10491049
host.checkTimeoutQueueLengthAndRun(2); // This enqueues the updategraph and refresh inferred projects
1050-
const version2 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
1051-
assert.notEqual(version1, version2, "set of unresolved imports should change");
1050+
const version2 = proj.lastCachedUnresolvedImportsList;
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: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,8 @@ namespace ts.server {
312312

313313
export class ProjectService {
314314

315-
public readonly typingsCache: TypingsCache;
315+
/*@internal*/
316+
readonly typingsCache: TypingsCache;
316317

317318
private readonly documentRegistry: DocumentRegistry;
318319

@@ -523,13 +524,13 @@ namespace ts.server {
523524
}
524525
switch (response.kind) {
525526
case ActionSet:
526-
project.resolutionCache.clear();
527-
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));
528529
break;
529530
case ActionInvalidate:
530-
project.resolutionCache.clear();
531-
this.typingsCache.deleteTypingsForProject(response.projectName);
532-
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;
533534
}
534535
this.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(project);
535536
}

0 commit comments

Comments
 (0)