Skip to content

use unresolved imports as a source of typings in auto acquisition #11828

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

Merged
merged 1 commit into from
Oct 25, 2016
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
48 changes: 43 additions & 5 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,44 @@ namespace ts {
return result;
}

export function arrayIsEqualTo<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>, equaler?: (a: T, b: T) => boolean): boolean {
if (!array1 || !array2) {
return array1 === array2;
}

if (array1.length !== array2.length) {
return false;
}

for (let i = 0; i < array1.length; i++) {
const equals = equaler ? equaler(array1[i], array2[i]) : array1[i] === array2[i];
if (!equals) {
return false;
}
}

return true;
}

export function changesAffectModuleResolution(oldOptions: CompilerOptions, newOptions: CompilerOptions): boolean {
return !oldOptions ||
(oldOptions.module !== newOptions.module) ||
(oldOptions.moduleResolution !== newOptions.moduleResolution) ||
(oldOptions.noResolve !== newOptions.noResolve) ||
(oldOptions.target !== newOptions.target) ||
(oldOptions.noLib !== newOptions.noLib) ||
(oldOptions.jsx !== newOptions.jsx) ||
(oldOptions.allowJs !== newOptions.allowJs) ||
(oldOptions.rootDir !== newOptions.rootDir) ||
(oldOptions.configFilePath !== newOptions.configFilePath) ||
(oldOptions.baseUrl !== newOptions.baseUrl) ||
(oldOptions.maxNodeModuleJsDepth !== newOptions.maxNodeModuleJsDepth) ||
!arrayIsEqualTo(oldOptions.lib, newOptions.lib) ||
!arrayIsEqualTo(oldOptions.typeRoots, newOptions.typeRoots) ||
!arrayIsEqualTo(oldOptions.rootDirs, newOptions.rootDirs) ||
!equalOwnProperties(oldOptions.paths, newOptions.paths);
}

/**
* Compacts an array, removing any falsey elements.
*/
Expand Down Expand Up @@ -821,7 +859,7 @@ namespace ts {
return result;
}

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

if (file) {
Debug.assert(start <= file.text.length, `start must be within the bounds of the file. ${ start } > ${ file.text.length }`);
Debug.assert(end <= file.text.length, `end must be the bounds of the file. ${ end } > ${ file.text.length }`);
Debug.assert(start <= file.text.length, `start must be within the bounds of the file. ${start} > ${file.text.length}`);
Debug.assert(end <= file.text.length, `end must be the bounds of the file. ${end} > ${file.text.length}`);
}

let text = getLocaleSpecificMessage(message);
Expand Down Expand Up @@ -1525,7 +1563,7 @@ namespace ts {
return undefined;
}

const replaceWildcardCharacter = usage === "files" ? replaceWildCardCharacterFiles : replaceWildCardCharacterOther;
const replaceWildcardCharacter = usage === "files" ? replaceWildCardCharacterFiles : replaceWildCardCharacterOther;
const singleAsteriskRegexFragment = usage === "files" ? singleAsteriskRegexFragmentFiles : singleAsteriskRegexFragmentOther;

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

export function getSupportedExtensions(options?: CompilerOptions): string[] {
return options && options.allowJs ? allSupportedExtensions : supportedTypeScriptExtensions;
Expand Down
16 changes: 1 addition & 15 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,21 +462,7 @@ namespace ts {
// check properties that can affect structure of the program or module resolution strategy
// if any of these properties has changed - structure cannot be reused
const oldOptions = oldProgram.getCompilerOptions();
if ((oldOptions.module !== options.module) ||
(oldOptions.moduleResolution !== options.moduleResolution) ||
(oldOptions.noResolve !== options.noResolve) ||
(oldOptions.target !== options.target) ||
(oldOptions.noLib !== options.noLib) ||
(oldOptions.jsx !== options.jsx) ||
(oldOptions.allowJs !== options.allowJs) ||
(oldOptions.rootDir !== options.rootDir) ||
(oldOptions.configFilePath !== options.configFilePath) ||
(oldOptions.baseUrl !== options.baseUrl) ||
(oldOptions.maxNodeModuleJsDepth !== options.maxNodeModuleJsDepth) ||
!arrayIsEqualTo(oldOptions.lib, options.lib) ||
!arrayIsEqualTo(oldOptions.typeRoots, options.typeRoots) ||
!arrayIsEqualTo(oldOptions.rootDirs, options.rootDirs) ||
!equalOwnProperties(oldOptions.paths, options.paths)) {
if (changesAffectModuleResolution(oldOptions, options)) {
return false;
}

Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3034,6 +3034,7 @@ namespace ts {
packageNameToTypingLocation: Map<string>; // The map of package names to their cached typing locations
typingOptions: TypingOptions; // Used to customize the typing inference process
compilerOptions: CompilerOptions; // Used as a source for typing inference
unresolvedImports: ReadonlyArray<string>; // List of unresolved module ids from imports
}

export enum ModuleKind {
Expand Down
19 changes: 0 additions & 19 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,6 @@ namespace ts {
return node.end - node.pos;
}

export function arrayIsEqualTo<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>, equaler?: (a: T, b: T) => boolean): boolean {
if (!array1 || !array2) {
return array1 === array2;
}

if (array1.length !== array2.length) {
return false;
}

for (let i = 0; i < array1.length; i++) {
const equals = equaler ? equaler(array1[i], array2[i]) : array1[i] === array2[i];
if (!equals) {
return false;
}
}

return true;
}

export function hasResolvedModule(sourceFile: SourceFile, moduleNameText: string): boolean {
return !!(sourceFile && sourceFile.resolvedModules && sourceFile.resolvedModules[moduleNameText]);
}
Expand Down
4 changes: 2 additions & 2 deletions src/harness/unittests/tsserverProjectSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ namespace ts.projectSystem {
this.projectService.updateTypingsForProject(response);
}

enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
const request = server.createInstallTypingsRequest(project, typingOptions, this.globalTypingsCacheLocation);
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
const request = server.createInstallTypingsRequest(project, typingOptions, unresolvedImports, this.globalTypingsCacheLocation);
this.install(request);
}

Expand Down
168 changes: 163 additions & 5 deletions src/harness/unittests/typingsInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,9 @@ namespace ts.projectSystem {
constructor() {
super(host);
}
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
enqueueIsCalled = true;
super.enqueueInstallTypingsRequest(project, typingOptions);
super.enqueueInstallTypingsRequest(project, typingOptions, unresolvedImports);
}
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings = ["@types/jquery"];
Expand Down Expand Up @@ -319,9 +319,9 @@ namespace ts.projectSystem {
constructor() {
super(host);
}
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions) {
enqueueInstallTypingsRequest(project: server.Project, typingOptions: TypingOptions, unresolvedImports: server.SortedReadonlyArray<string>) {
enqueueIsCalled = true;
super.enqueueInstallTypingsRequest(project, typingOptions);
super.enqueueInstallTypingsRequest(project, typingOptions, unresolvedImports);
}
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: TI.RequestCompletedAction): void {
const installedTypings: string[] = [];
Expand Down Expand Up @@ -724,7 +724,7 @@ namespace ts.projectSystem {
it("Malformed package.json should be watched", () => {
const f = {
path: "/a/b/app.js",
content: "var x = require('commander')"
content: "var x = 1"
};
const brokenPackageJson = {
path: "/a/b/package.json",
Expand Down Expand Up @@ -763,6 +763,133 @@ namespace ts.projectSystem {
service.checkNumberOfProjects({ inferredProjects: 1 });
checkProjectActualFiles(service.inferredProjects[0], [f.path, commander.path]);
});

it("should install typings for unresolved imports", () => {
const file = {
path: "/a/b/app.js",
content: `
import * as fs from "fs";
import * as commander from "commander";`
};
const cachePath = "/a/cache";
const node = {
path: cachePath + "/node_modules/@types/node/index.d.ts",
content: "export let x: number"
};
const commander = {
path: cachePath + "/node_modules/@types/commander/index.d.ts",
content: "export let y: string"
};
const host = createServerHost([file]);
const installer = new (class extends Installer {
constructor() {
super(host, { globalTypingsCacheLocation: cachePath });
}
executeRequest(requestKind: TI.RequestKind, _requestId: number, _args: string[], _cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
const installedTypings = ["@types/node", "@types/commander"];
const typingFiles = [node, commander];
executeCommand(this, host, installedTypings, typingFiles, requestKind, cb);
}
})();
const service = createProjectService(host, { typingsInstaller: installer });
service.openClientFile(file.path);

service.checkNumberOfProjects({ inferredProjects: 1 });
checkProjectActualFiles(service.inferredProjects[0], [file.path]);

installer.installAll([TI.NpmViewRequest, TI.NpmViewRequest], [TI.NpmInstallRequest]);

assert.isTrue(host.fileExists(node.path), "typings for 'node' should be created");
assert.isTrue(host.fileExists(commander.path), "typings for 'commander' should be created");

checkProjectActualFiles(service.inferredProjects[0], [file.path, node.path, commander.path]);
});

it("should pick typing names from non-relative unresolved imports", () => {
const f1 = {
path: "/a/b/app.js",
content: `
import * as a from "foo/a/a";
import * as b from "foo/a/b";
import * as c from "foo/a/c";
import * as d from "@bar/router/";
import * as e from "@bar/common/shared";
import * as e from "@bar/common/apps";
import * as f from "./lib"
`
};

const host = createServerHost([f1]);
const installer = new (class extends Installer {
constructor() {
super(host, { globalTypingsCacheLocation: "/tmp" });
}
executeRequest(requestKind: TI.RequestKind, _requestId: number, args: string[], _cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
if (requestKind === TI.NpmViewRequest) {
// args should have only non-scoped packages - scoped packages are not yet supported
assert.deepEqual(args, ["foo"]);
}
executeCommand(this, host, ["foo"], [], requestKind, cb);
}
})();
const projectService = createProjectService(host, { typingsInstaller: installer });
projectService.openClientFile(f1.path);
projectService.checkNumberOfProjects({ inferredProjects: 1 });

const proj = projectService.inferredProjects[0];
proj.updateGraph();

assert.deepEqual(
proj.getCachedUnresolvedImportsPerFile_TestOnly().get(<Path>f1.path),
["foo", "foo", "foo", "@bar/router", "@bar/common", "@bar/common"]
);

installer.installAll([TI.NpmViewRequest], [TI.NpmInstallRequest]);
});

it("cached unresolved typings are not recomputed if program structure did not change", () => {
const host = createServerHost([]);
const session = createSession(host);
const f = {
path: "/a/app.js",
content: `
import * as fs from "fs";
import * as cmd from "commander
`
};
session.executeCommand(<server.protocol.OpenRequest>{
seq: 1,
type: "request",
command: "open",
arguments: {
file: f.path,
fileContent: f.content
}
});
const projectService = session.getProjectService();
checkNumberOfProjects(projectService, { inferredProjects: 1 });
const proj = projectService.inferredProjects[0];
const version1 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();

// make a change that should not affect the structure of the program
session.executeCommand(<server.protocol.ChangeRequest>{
seq: 2,
type: "request",
command: "change",
arguments: {
file: f.path,
insertString: "\nlet x = 1;",
line: 2,
offset: 0,
endLine: 2,
endOffset: 0
}
});
host.checkTimeoutQueueLength(1);
host.runQueuedTimeoutCallbacks();
const version2 = proj.getCachedUnresolvedImportsPerFile_TestOnly().getVersion();
assert.equal(version1, version2, "set of unresolved imports should not change");
});
});

describe("Validate package name:", () => {
Expand Down Expand Up @@ -820,4 +947,35 @@ namespace ts.projectSystem {
assert.isTrue(messages.indexOf("Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
});
});

describe("discover typings", () => {
it("should return node for core modules", () => {
const f = {
path: "/a/b/app.js",
content: ""
};
const host = createServerHost([f]);
const cache = createMap<string>();
for (const name of JsTyping.nodeCoreModuleList) {
const result = JsTyping.discoverTypings(host, [f.path], getDirectoryPath(<Path>f.path), /*safeListPath*/ undefined, cache, { enableAutoDiscovery: true }, [name, "somename"]);
assert.deepEqual(result.newTypingNames.sort(), ["node", "somename"]);
}
});

it("should use cached locaitons", () => {
const f = {
path: "/a/b/app.js",
content: ""
};
const node = {
path: "/a/b/node.d.ts",
content: ""
};
const host = createServerHost([f, node]);
const cache = createMap<string>({ "node": node.path });
const result = JsTyping.discoverTypings(host, [f.path], getDirectoryPath(<Path>f.path), /*safeListPath*/ undefined, cache, { enableAutoDiscovery: true }, ["fs", "bar"]);
assert.deepEqual(result.cachedTypingPaths, [node.path]);
assert.deepEqual(result.newTypingNames, ["bar"]);
});
});
}
6 changes: 3 additions & 3 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,13 +287,13 @@ namespace ts.server {
}
switch (response.kind) {
case "set":
this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.typings);
project.updateGraph();
this.typingsCache.updateTypingsForProject(response.projectName, response.compilerOptions, response.typingOptions, response.unresolvedImports, response.typings);
break;
case "invalidate":
this.typingsCache.invalidateCachedTypingsForProject(project);
this.typingsCache.deleteTypingsForProject(response.projectName);
break;
}
project.updateGraph();
}

setCompilerOptionsForInferredProjects(projectCompilerOptions: protocol.ExternalProjectCompilerOptions): void {
Expand Down
Loading