Skip to content

Commit 7890f63

Browse files
vladimamhegazy
vladima
authored and
mhegazy
committed
use unresolved imports as a source of used typings (microsoft#11828)
1 parent d0170d1 commit 7890f63

16 files changed

Lines changed: 457 additions & 96 deletions

File tree

‎src/compiler/core.ts‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,44 @@ namespace ts {
456456
return result;
457457
}
458458

459+
export function arrayIsEqualTo<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>, equaler?: (a: T, b: T) => boolean): boolean {
460+
if (!array1 || !array2) {
461+
return array1 === array2;
462+
}
463+
464+
if (array1.length !== array2.length) {
465+
return false;
466+
}
467+
468+
for (let i = 0; i < array1.length; i++) {
469+
const equals = equaler ? equaler(array1[i], array2[i]) : array1[i] === array2[i];
470+
if (!equals) {
471+
return false;
472+
}
473+
}
474+
475+
return true;
476+
}
477+
478+
export function changesAffectModuleResolution(oldOptions: CompilerOptions, newOptions: CompilerOptions): boolean {
479+
return !oldOptions ||
480+
(oldOptions.module !== newOptions.module) ||
481+
(oldOptions.moduleResolution !== newOptions.moduleResolution) ||
482+
(oldOptions.noResolve !== newOptions.noResolve) ||
483+
(oldOptions.target !== newOptions.target) ||
484+
(oldOptions.noLib !== newOptions.noLib) ||
485+
(oldOptions.jsx !== newOptions.jsx) ||
486+
(oldOptions.allowJs !== newOptions.allowJs) ||
487+
(oldOptions.rootDir !== newOptions.rootDir) ||
488+
(oldOptions.configFilePath !== newOptions.configFilePath) ||
489+
(oldOptions.baseUrl !== newOptions.baseUrl) ||
490+
(oldOptions.maxNodeModuleJsDepth !== newOptions.maxNodeModuleJsDepth) ||
491+
!arrayIsEqualTo(oldOptions.lib, newOptions.lib) ||
492+
!arrayIsEqualTo(oldOptions.typeRoots, newOptions.typeRoots) ||
493+
!arrayIsEqualTo(oldOptions.rootDirs, newOptions.rootDirs) ||
494+
!equalOwnProperties(oldOptions.paths, newOptions.paths);
495+
}
496+
459497
/**
460498
* Compacts an array, removing any falsey elements.
461499
*/
@@ -821,7 +859,7 @@ namespace ts {
821859
return result;
822860
}
823861

824-
export function extend<T1, T2>(first: T1 , second: T2): T1 & T2 {
862+
export function extend<T1, T2>(first: T1, second: T2): T1 & T2 {
825863
const result: T1 & T2 = <any>{};
826864
for (const id in second) if (hasOwnProperty.call(second, id)) {
827865
(result as any)[id] = (second as any)[id];
@@ -974,8 +1012,8 @@ namespace ts {
9741012
Debug.assert(length >= 0, "length must be non-negative, is " + length);
9751013

9761014
if (file) {
977-
Debug.assert(start <= file.text.length, `start must be within the bounds of the file. ${ start } > ${ file.text.length }`);
978-
Debug.assert(end <= file.text.length, `end must be the bounds of the file. ${ end } > ${ file.text.length }`);
1015+
Debug.assert(start <= file.text.length, `start must be within the bounds of the file. ${start} > ${file.text.length}`);
1016+
Debug.assert(end <= file.text.length, `end must be the bounds of the file. ${end} > ${file.text.length}`);
9791017
}
9801018

9811019
let text = getLocaleSpecificMessage(message);
@@ -1525,7 +1563,7 @@ namespace ts {
15251563
return undefined;
15261564
}
15271565

1528-
const replaceWildcardCharacter = usage === "files" ? replaceWildCardCharacterFiles : replaceWildCardCharacterOther;
1566+
const replaceWildcardCharacter = usage === "files" ? replaceWildCardCharacterFiles : replaceWildCardCharacterOther;
15291567
const singleAsteriskRegexFragment = usage === "files" ? singleAsteriskRegexFragmentFiles : singleAsteriskRegexFragmentOther;
15301568

15311569
/**
@@ -1767,7 +1805,7 @@ namespace ts {
17671805
/** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */
17681806
export const supportedTypescriptExtensionsForExtractExtension = [".d.ts", ".ts", ".tsx"];
17691807
export const supportedJavascriptExtensions = [".js", ".jsx"];
1770-
const allSupportedExtensions = supportedTypeScriptExtensions.concat(supportedJavascriptExtensions);
1808+
const allSupportedExtensions = supportedTypeScriptExtensions.concat(supportedJavascriptExtensions);
17711809

17721810
export function getSupportedExtensions(options?: CompilerOptions): string[] {
17731811
return options && options.allowJs ? allSupportedExtensions : supportedTypeScriptExtensions;

‎src/compiler/program.ts‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -462,21 +462,7 @@ namespace ts {
462462
// check properties that can affect structure of the program or module resolution strategy
463463
// if any of these properties has changed - structure cannot be reused
464464
const oldOptions = oldProgram.getCompilerOptions();
465-
if ((oldOptions.module !== options.module) ||
466-
(oldOptions.moduleResolution !== options.moduleResolution) ||
467-
(oldOptions.noResolve !== options.noResolve) ||
468-
(oldOptions.target !== options.target) ||
469-
(oldOptions.noLib !== options.noLib) ||
470-
(oldOptions.jsx !== options.jsx) ||
471-
(oldOptions.allowJs !== options.allowJs) ||
472-
(oldOptions.rootDir !== options.rootDir) ||
473-
(oldOptions.configFilePath !== options.configFilePath) ||
474-
(oldOptions.baseUrl !== options.baseUrl) ||
475-
(oldOptions.maxNodeModuleJsDepth !== options.maxNodeModuleJsDepth) ||
476-
!arrayIsEqualTo(oldOptions.lib, options.lib) ||
477-
!arrayIsEqualTo(oldOptions.typeRoots, options.typeRoots) ||
478-
!arrayIsEqualTo(oldOptions.rootDirs, options.rootDirs) ||
479-
!equalOwnProperties(oldOptions.paths, options.paths)) {
465+
if (changesAffectModuleResolution(oldOptions, options)) {
480466
return false;
481467
}
482468

‎src/compiler/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3041,6 +3041,7 @@ namespace ts {
30413041
packageNameToTypingLocation: Map<string>; // The map of package names to their cached typing locations
30423042
typingOptions: TypingOptions; // Used to customize the typing inference process
30433043
compilerOptions: CompilerOptions; // Used as a source for typing inference
3044+
unresolvedImports: ReadonlyArray<string>; // List of unresolved module ids from imports
30443045
}
30453046

30463047
export enum ModuleKind {

‎src/compiler/utilities.ts‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,25 +83,6 @@ namespace ts {
8383
return node.end - node.pos;
8484
}
8585

86-
export function arrayIsEqualTo<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>, equaler?: (a: T, b: T) => boolean): boolean {
87-
if (!array1 || !array2) {
88-
return array1 === array2;
89-
}
90-
91-
if (array1.length !== array2.length) {
92-
return false;
93-
}
94-
95-
for (let i = 0; i < array1.length; i++) {
96-
const equals = equaler ? equaler(array1[i], array2[i]) : array1[i] === array2[i];
97-
if (!equals) {
98-
return false;
99-
}
100-
}
101-
102-
return true;
103-
}
104-
10586
export function hasResolvedModule(sourceFile: SourceFile, moduleNameText: string): boolean {
10687
return !!(sourceFile && sourceFile.resolvedModules && sourceFile.resolvedModules[moduleNameText]);
10788
}

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ namespace ts.projectSystem {
9494
this.projectService.updateTypingsForProject(response);
9595
}
9696

97-
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
98-
const request = server.createInstallTypingsRequest(project, typingOptions, this.globalTypingsCacheLocation);
97+
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
98+
const request = server.createInstallTypingsRequest(project, typingOptions, unresolvedImports, this.globalTypingsCacheLocation);
9999
this.install(request);
100100
}
101101

‎src/harness/unittests/typingsInstaller.ts‎

Lines changed: 163 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,9 @@ namespace ts.projectSystem {
217217
constructor() {
218218
super(host);
219219
}
220-
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
220+
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
221221
enqueueIsCalled = true;
222-
super.enqueueInstallTypingsRequest(project, typingOptions);
222+
super.enqueueInstallTypingsRequest(project, typingOptions, unresolvedImports);
223223
}
224224
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
225225
const installedTypings = ["@types/jquery"];
@@ -319,9 +319,9 @@ namespace ts.projectSystem {
319319
constructor() {
320320
super(host);
321321
}
322-
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
322+
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
323323
enqueueIsCalled = true;
324-
super.enqueueInstallTypingsRequest(project, typingOptions);
324+
super.enqueueInstallTypingsRequest(project, typingOptions, unresolvedImports);
325325
}
326326
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
327327
const installedTypings: string[] = [];
@@ -724,7 +724,7 @@ namespace ts.projectSystem {
724724
it("Malformed package.json should be watched", () => {
725725
const f = {
726726
path: "/a/b/app.js",
727-
content: "var x = require('commander')"
727+
content: "var x = 1"
728728
};
729729
const brokenPackageJson = {
730730
path: "/a/b/package.json",
@@ -763,6 +763,133 @@ namespace ts.projectSystem {
763763
service.checkNumberOfProjects({ inferredProjects: 1 });
764764
checkProjectActualFiles(service.inferredProjects[0], [f.path, commander.path]);
765765
});
766+
767+
it("should install typings for unresolved imports", () => {
768+
const file = {
769+
path: "/a/b/app.js",
770+
content: `
771+
import * as fs from "fs";
772+
import * as commander from "commander";`
773+
};
774+
const cachePath = "/a/cache";
775+
const node = {
776+
path: cachePath + "/node_modules/@types/node/index.d.ts",
777+
content: "export let x: number"
778+
};
779+
const commander = {
780+
path: cachePath + "/node_modules/@types/commander/index.d.ts",
781+
content: "export let y: string"
782+
};
783+
const host = createServerHost([file]);
784+
const installer = new (class extends Installer {
785+
constructor() {
786+
super(host, { globalTypingsCacheLocation: cachePath });
787+
}
788+
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
789+
const installedTypings = ["@types/node", "@types/commander"];
790+
const typingFiles = [node, commander];
791+
executeCommand(this, host, installedTypings, typingFiles, requestKind, cb);
792+
}
793+
})();
794+
const service = createProjectService(host, { typingsInstaller: installer });
795+
service.openClientFile(file.path);
796+
797+
service.checkNumberOfProjects({ inferredProjects: 1 });
798+
checkProjectActualFiles(service.inferredProjects[0], [file.path]);
799+
800+
installer.installAll([TI.NpmViewRequest, TI.NpmViewRequest], [TI.NpmInstallRequest]);
801+
802+
assert.isTrue(host.fileExists(node.path), "typings for 'node' should be created");
803+
assert.isTrue(host.fileExists(commander.path), "typings for 'commander' should be created");
804+
805+
checkProjectActualFiles(service.inferredProjects[0], [file.path, node.path, commander.path]);
806+
});
807+
808+
it("should pick typing names from non-relative unresolved imports", () => {
809+
const f1 = {
810+
path: "/a/b/app.js",
811+
content: `
812+
import * as a from "foo/a/a";
813+
import * as b from "foo/a/b";
814+
import * as c from "foo/a/c";
815+
import * as d from "@bar/router/";
816+
import * as e from "@bar/common/shared";
817+
import * as e from "@bar/common/apps";
818+
import * as f from "./lib"
819+
`
820+
};
821+
822+
const host = createServerHost([f1]);
823+
const installer = new (class extends Installer {
824+
constructor() {
825+
super(host, { globalTypingsCacheLocation: "/tmp" });
826+
}
827+
executeRequest(requestKind: TI.RequestKind, _requestId: number, args: string[], _cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
828+
if (requestKind === TI.NpmViewRequest) {
829+
// args should have only non-scoped packages - scoped packages are not yet supported
830+
assert.deepEqual(args, ["foo"]);
831+
}
832+
executeCommand(this, host, ["foo"], [], requestKind, cb);
833+
}
834+
})();
835+
const projectService = createProjectService(host, { typingsInstaller: installer });
836+
projectService.openClientFile(f1.path);
837+
projectService.checkNumberOfProjects({ inferredProjects: 1 });
838+
839+
const proj = projectService.inferredProjects[0];
840+
proj.updateGraph();
841+
842+
assert.deepEqual(
843+
proj.getCachedUnresolvedImportsPerFile_TestOnly().get(<Path>f1.path),
844+
["foo", "foo", "foo", "@bar/router", "@bar/common", "@bar/common"]
845+
);
846+
847+
installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]);
848+
});
849+
850+
it("cached unresolved typings are not recomputed if program structure did not change", () => {
851+
const host = createServerHost([]);
852+
const session = createSession(host);
853+
const f = {
854+
path: "/a/app.js",
855+
content: `
856+
import * as fs from "fs";
857+
import * as cmd from "commander
858+
`
859+
};
860+
session.executeCommand(<server.protocol.OpenRequest>{
861+
seq: 1,
862+
type: "request",
863+
command: "open",
864+
arguments: {
865+
file: f.path,
866+
fileContent: f.content
867+
}
868+
});
869+
const projectService = session.getProjectService();
870+
checkNumberOfProjects(projectService, { inferredProjects: 1 });
871+
const proj = projectService.inferredProjects[0];
872+
const version1 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
873+
874+
// make a change that should not affect the structure of the program
875+
session.executeCommand(<server.protocol.ChangeRequest>{
876+
seq: 2,
877+
type: "request",
878+
command: "change",
879+
arguments: {
880+
file: f.path,
881+
insertString: "\nlet x = 1;",
882+
line: 2,
883+
offset: 0,
884+
endLine: 2,
885+
endOffset: 0
886+
}
887+
});
888+
host.checkTimeoutQueueLength(1);
889+
host.runQueuedTimeoutCallbacks();
890+
const version2 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
891+
assert.equal(version1, version2, "set of unresolved imports should not change");
892+
});
766893
});
767894

768895
describe("Validate package name:", () => {
@@ -820,4 +947,35 @@ namespace ts.projectSystem {
820947
assert.isTrue(messages.indexOf("Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
821948
});
822949
});
950+
951+
describe("discover typings", () => {
952+
it("should return node for core modules", () => {
953+
const f = {
954+
path: "/a/b/app.js",
955+
content: ""
956+
};
957+
const host = createServerHost([f]);
958+
const cache = createMap<string>();
959+
for (const name of JsTyping.nodeCoreModuleList) {
960+
const result = JsTyping.discoverTypings(host, [f.path], getDirectoryPath(<Path>f.path), /*safeListPath*/ undefined, cache, { enableAutoDiscovery: true }, [name, "somename"]);
961+
assert.deepEqual(result.newTypingNames.sort(), ["node", "somename"]);
962+
}
963+
});
964+
965+
it("should use cached locaitons", () => {
966+
const f = {
967+
path: "/a/b/app.js",
968+
content: ""
969+
};
970+
const node = {
971+
path: "/a/b/node.d.ts",
972+
content: ""
973+
};
974+
const host = createServerHost([f, node]);
975+
const cache = createMap<string>({ "node": node.path });
976+
const result = JsTyping.discoverTypings(host, [f.path], getDirectoryPath(<Path>f.path), /*safeListPath*/ undefined, cache, { enableAutoDiscovery: true }, ["fs", "bar"]);
977+
assert.deepEqual(result.cachedTypingPaths, [node.path]);
978+
assert.deepEqual(result.newTypingNames, ["bar"]);
979+
});
980+
});
823981
}

‎src/server/editorServices.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -287,13 +287,13 @@ namespace ts.server {
287287
}
288288
switch (response.kind) {
289289
case "set":
290-
this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings);
291-
project.updateGraph();
290+
this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.unresolvedImports, response.typings);
292291
break;
293292
case "invalidate":
294-
this.typingsCache.invalidateCachedTypingsForProject(project);
293+
this.typingsCache.deleteTypingsForProject(response.projectName);
295294
break;
296295
}
296+
project.updateGraph();
297297
}
298298

299299
setCompilerOptionsForInferredProjects(projectCompilerOptions: protocol.ExternalProjectCompilerOptions): void {

0 commit comments

Comments
 (0)