address PR feedback

This commit is contained in:
Vladimir Matveev
2015-08-20 16:13:49 -07:00
parent f415097d0d
commit dde7545d34
20 changed files with 216 additions and 78 deletions
+10 -1
View File
@@ -218,7 +218,16 @@ namespace ts {
type: "boolean",
experimental: true,
description: Diagnostics.Enables_experimental_support_for_emitting_type_metadata_for_decorators
}
},
{
name: "moduleResolution",
type: {
"node": ModuleResolutionKind.NodeJs,
"classic": ModuleResolutionKind.Classic
},
experimental: true,
description: Diagnostics.Specifies_module_resolution_strategy_Colon_node_Node_or_classic_TypeScript_pre_1_6
}
];
export function parseCommandLine(commandLine: string[]): ParsedCommandLine {
@@ -574,6 +574,7 @@ namespace ts {
Enables_experimental_support_for_emitting_type_metadata_for_decorators: { code: 6066, category: DiagnosticCategory.Message, key: "Enables experimental support for emitting type metadata for decorators." },
Option_experimentalAsyncFunctions_cannot_be_specified_when_targeting_ES5_or_lower: { code: 6067, category: DiagnosticCategory.Message, key: "Option 'experimentalAsyncFunctions' cannot be specified when targeting ES5 or lower." },
Enables_experimental_support_for_ES7_async_functions: { code: 6068, category: DiagnosticCategory.Message, key: "Enables experimental support for ES7 async functions." },
Specifies_module_resolution_strategy_Colon_node_Node_or_classic_TypeScript_pre_1_6: { code: 6069, category: DiagnosticCategory.Message, key: "Specifies module resolution strategy: 'node' (Node) or 'classic' (TypeScript pre 1.6) ." },
Variable_0_implicitly_has_an_1_type: { code: 7005, category: DiagnosticCategory.Error, key: "Variable '{0}' implicitly has an '{1}' type." },
Parameter_0_implicitly_has_an_1_type: { code: 7006, category: DiagnosticCategory.Error, key: "Parameter '{0}' implicitly has an '{1}' type." },
Member_0_implicitly_has_an_1_type: { code: 7008, category: DiagnosticCategory.Error, key: "Member '{0}' implicitly has an '{1}' type." },
+4 -1
View File
@@ -2286,7 +2286,10 @@
"category": "Message",
"code": 6068
},
"Specifies module resolution strategy: 'node' (Node) or 'classic' (TypeScript pre 1.6) .": {
"category": "Message",
"code": 6069
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
+46 -32
View File
@@ -36,10 +36,13 @@ namespace ts {
}
export function resolveModuleName(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModule {
switch (compilerOptions.moduleResolution) {
let moduleResolution = compilerOptions.moduleResolution !== undefined
? compilerOptions.moduleResolution
: compilerOptions.module === ModuleKind.CommonJS ? ModuleResolutionKind.NodeJs : ModuleResolutionKind.Classic;
switch (moduleResolution) {
case ModuleResolutionKind.NodeJs: return nodeModuleNameResolver(moduleName, containingFile, host);
case ModuleResolutionKind.BaseUrl: return baseUrlModuleNameResolver(moduleName, containingFile, compilerOptions.baseUrl, host);
default: return legacyNameResolver(moduleName, containingFile, compilerOptions, host);
case ModuleResolutionKind.Classic: return classicNameResolver(moduleName, containingFile, compilerOptions, host);
}
}
@@ -49,46 +52,57 @@ namespace ts {
if (getRootLength(moduleName) !== 0 || nameStartsWithDotSlashOrDotDotSlash(moduleName)) {
let failedLookupLocations: string[] = [];
let candidate = normalizePath(combinePaths(containingDirectory, moduleName));
let result = loadNodeModuleFromFile(candidate, failedLookupLocations, host);
let resolvedFileName = loadNodeModuleFromFile(candidate, /* loadOnlyDts */ false, failedLookupLocations, host);
if (result) {
return { resolvedFileName: result, failedLookupLocations };
if (resolvedFileName) {
return { resolvedFileName, failedLookupLocations };
}
result = loadNodeModuleFromDirectory(candidate, failedLookupLocations, host);
return { resolvedFileName: result, failedLookupLocations };
resolvedFileName = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ false, failedLookupLocations, host);
return { resolvedFileName, failedLookupLocations };
}
else {
return loadModuleModuleFromNodeModules(moduleName, containingDirectory, host);
return loadModuleFromNodeModules(moduleName, containingDirectory, host);
}
}
function loadNodeModuleFromFile(candidate: string, failedLookupLocation: string[], host: ModuleResolutionHost): string {
// load only .d.ts files
let fileName = fileExtensionIs(candidate, ".d.ts") ? candidate : candidate + ".d.ts";
if (!host.fileExists(fileName)) {
failedLookupLocation.push(fileName);
return undefined;
function loadNodeModuleFromFile(candidate: string, loadOnlyDts: boolean, failedLookupLocation: string[], host: ModuleResolutionHost): string {
if (loadOnlyDts) {
return tryLoad(".d.ts");
}
else {
return forEach(supportedExtensions, tryLoad);
}
return fileName;
function tryLoad(ext: string): string {
let fileName = fileExtensionIs(candidate, ext) ? candidate : candidate + ext;
if (host.fileExists(fileName)) {
return fileName;
}
else {
failedLookupLocation.push(fileName);
return undefined;
}
}
}
function loadNodeModuleFromDirectory(candidate: string, failedLookupLocation: string[], host: ModuleResolutionHost): string {
function loadNodeModuleFromDirectory(candidate: string, loadOnlyDts: boolean, failedLookupLocation: string[], host: ModuleResolutionHost): string {
let packageJsonPath = combinePaths(candidate, "package.json");
if (host.fileExists(packageJsonPath)) {
let jsonText = host.readFile(packageJsonPath);
let jsonContent = jsonText ? <{ typings?: string, main?: string }>JSON.parse(jsonText) : { typings: undefined, main:undefined };
if (jsonContent.typings) {
let result = loadNodeModuleFromFile(normalizePath(combinePaths(candidate, jsonContent.typings)), failedLookupLocation, host);
if (result) {
return result;
}
let jsonContent: { typings?: string };
try {
let jsonText = host.readFile(packageJsonPath);
jsonContent = jsonText ? <{ typings?: string }>JSON.parse(jsonText) : { typings: undefined };
}
catch (e) {
// gracefully handle if readFile fails or returns not JSON
jsonContent = { typings: undefined };
}
if (jsonContent.main) {
let mainFile = removeFileExtension(jsonContent.main);
let result = loadNodeModuleFromFile(normalizePath(combinePaths(candidate, mainFile)), failedLookupLocation, host);
if (jsonContent.typings) {
let result = loadNodeModuleFromFile(normalizePath(combinePaths(candidate, jsonContent.typings)), loadOnlyDts, failedLookupLocation, host);
if (result) {
return result;
}
@@ -99,10 +113,10 @@ namespace ts {
failedLookupLocation.push(packageJsonPath);
}
return loadNodeModuleFromFile(combinePaths(candidate, "index"), failedLookupLocation, host);
return loadNodeModuleFromFile(combinePaths(candidate, "index"), loadOnlyDts, failedLookupLocation, host);
}
function loadModuleModuleFromNodeModules(moduleName: string, directory: string, host: ModuleResolutionHost): ResolvedModule {
function loadModuleFromNodeModules(moduleName: string, directory: string, host: ModuleResolutionHost): ResolvedModule {
let failedLookupLocations: string[] = [];
directory = normalizeSlashes(directory);
while (true) {
@@ -110,12 +124,12 @@ namespace ts {
if (baseName !== "node_modules") {
let nodeModulesFolder = combinePaths(directory, "node_modules");
let candidate = normalizePath(combinePaths(nodeModulesFolder, moduleName));
let result = loadNodeModuleFromFile(candidate, failedLookupLocations, host);
let result = loadNodeModuleFromFile(candidate, /* loadOnlyDts */ true, failedLookupLocations, host);
if (result) {
return { resolvedFileName: result, failedLookupLocations };
}
result = loadNodeModuleFromDirectory(candidate, failedLookupLocations, host);
result = loadNodeModuleFromDirectory(candidate, /* loadOnlyDts */ true, failedLookupLocations, host);
if (result) {
return { resolvedFileName: result, failedLookupLocations };
}
@@ -165,7 +179,7 @@ namespace ts {
return getRootLength(moduleName) === 0 && !nameStartsWithDotSlashOrDotDotSlash(moduleName);
}
export function legacyNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModule {
export function classicNameResolver(moduleName: string, containingFile: string, compilerOptions: CompilerOptions, host: ModuleResolutionHost): ResolvedModule {
// module names that contain '!' are used to reference resources and are not resolved to actual files on disk
if (moduleName.indexOf('!') != -1) {
+2 -4
View File
@@ -2010,9 +2010,8 @@ namespace ts {
}
export const enum ModuleResolutionKind {
Legacy = 1,
NodeJs = 2,
BaseUrl = 3
Classic = 1,
NodeJs = 2
}
export interface CompilerOptions {
@@ -2054,7 +2053,6 @@ namespace ts {
experimentalAsyncFunctions?: boolean;
emitDecoratorMetadata?: boolean;
moduleResolution?: ModuleResolutionKind
baseUrl?: string;
/* @internal */ stripInternal?: boolean;
// Skip checking lib.d.ts to help speed up tests.
+13 -1
View File
@@ -1025,6 +1025,9 @@ module Harness {
options.module = ts.ModuleKind.UMD;
} else if (setting.value.toLowerCase() === "commonjs") {
options.module = ts.ModuleKind.CommonJS;
if (options.moduleResolution === undefined) {
options.moduleResolution = ts.ModuleResolutionKind.Classic;
}
} else if (setting.value.toLowerCase() === "system") {
options.module = ts.ModuleKind.System;
} else if (setting.value.toLowerCase() === "unspecified") {
@@ -1036,7 +1039,16 @@ module Harness {
options.module = <any>setting.value;
}
break;
case "moduleresolution":
switch((setting.value || "").toLowerCase()) {
case "classic":
options.moduleResolution = ts.ModuleResolutionKind.Classic;
break;
case "node":
options.moduleResolution = ts.ModuleResolutionKind.NodeJs;
break;
}
break;
case "target":
case "codegentarget":
if (typeof setting.value === "string") {
+1
View File
@@ -163,6 +163,7 @@ class ProjectRunner extends RunnerBase {
mapRoot: testCase.resolveMapRoot && testCase.mapRoot ? ts.sys.resolvePath(testCase.mapRoot) : testCase.mapRoot,
sourceRoot: testCase.resolveSourceRoot && testCase.sourceRoot ? ts.sys.resolvePath(testCase.sourceRoot) : testCase.sourceRoot,
module: moduleKind,
moduleResolution: ts.ModuleResolutionKind.Classic, // currently all tests use classic module resolution kind, this will change in the future
noResolve: testCase.noResolve,
rootDir: testCase.rootDir
};
@@ -0,0 +1,12 @@
//// [tests/cases/compiler/nodeResolution1.ts] ////
//// [a.ts]
export var x = 1;
//// [b.ts]
import y = require("./a");
//// [a.js]
exports.x = 1;
//// [b.js]
@@ -0,0 +1,9 @@
=== tests/cases/compiler/b.ts ===
import y = require("./a");
>y : Symbol(y, Decl(b.ts, 0, 0))
=== tests/cases/compiler/a.ts ===
export var x = 1;
>x : Symbol(x, Decl(a.ts, 1, 10))
@@ -0,0 +1,10 @@
=== tests/cases/compiler/b.ts ===
import y = require("./a");
>y : typeof y
=== tests/cases/compiler/a.ts ===
export var x = 1;
>x : number
>1 : number
@@ -0,0 +1,10 @@
//// [tests/cases/compiler/nodeResolution2.ts] ////
//// [a.d.ts]
export var x: number;
//// [b.ts]
import y = require("a");
//// [b.js]
@@ -0,0 +1,9 @@
=== tests/cases/compiler/b.ts ===
import y = require("a");
>y : Symbol(y, Decl(b.ts, 0, 0))
=== tests/cases/compiler/node_modules/a.d.ts ===
export var x: number;
>x : Symbol(x, Decl(a.d.ts, 1, 10))
@@ -0,0 +1,9 @@
=== tests/cases/compiler/b.ts ===
import y = require("a");
>y : typeof y
=== tests/cases/compiler/node_modules/a.d.ts ===
export var x: number;
>x : number
@@ -0,0 +1,10 @@
//// [tests/cases/compiler/nodeResolution3.ts] ////
//// [index.d.ts]
export var x: number;
//// [a.ts]
import y = require("b");
//// [a.js]
@@ -0,0 +1,9 @@
=== tests/cases/compiler/a.ts ===
import y = require("b");
>y : Symbol(y, Decl(a.ts, 0, 0))
=== tests/cases/compiler/node_modules/b/index.d.ts ===
export var x: number;
>x : Symbol(x, Decl(index.d.ts, 1, 10))
@@ -0,0 +1,9 @@
=== tests/cases/compiler/a.ts ===
import y = require("b");
>y : typeof y
=== tests/cases/compiler/node_modules/b/index.d.ts ===
export var x: number;
>x : number
+8
View File
@@ -0,0 +1,8 @@
// @module: commonjs
// @moduleResolution: node
// @filename: a.ts
export var x = 1;
// @filename: b.ts
import y = require("./a");
+8
View File
@@ -0,0 +1,8 @@
// @module: commonjs
// @moduleResolution: node
// @filename: node_modules/a.d.ts
export var x: number;
// @filename: b.ts
import y = require("a");
+8
View File
@@ -0,0 +1,8 @@
// @module: commonjs
// @moduleResolution: node
// @filename: node_modules/b/index.d.ts
export var x: number;
// @filename: a.ts
import y = require("b");
+28 -39
View File
@@ -86,30 +86,24 @@ module ts {
describe("Node module resolution - relative paths", () => {
function testLoadAsFile(containingFileName: string, moduleFileNameNoExt: string, moduleName: string): void {
{
// loading only .d.ts files
let containingFile = { name: containingFileName, content: ""}
let moduleFile = { name: moduleFileNameNoExt + ".d.ts", content: "var x;"}
let resolution = nodeModuleNameResolver(moduleName, containingFile.name, createModuleResolutionHost(containingFile, moduleFile));
for (let ext of supportedExtensions) {
let containingFile = { name: containingFileName }
let moduleFile = { name: moduleFileNameNoExt + ext }
let resolution = nodeModuleNameResolver(moduleName, containingFile.name, createModuleResolutionHost(containingFile, moduleFile));
assert.equal(resolution.resolvedFileName, moduleFile.name);
assert.isTrue(resolution.failedLookupLocations.length === 0);
}
{
// does not try to load .ts files
let containingFile = { name: containingFileName, content: ""}
let moduleFile = { name: moduleFileNameNoExt + ".ts", content: "var x;"}
let resolution = nodeModuleNameResolver(moduleName, containingFile.name, createModuleResolutionHost(containingFile, moduleFile));
let failedLookupLocations: string[] = [];
let dir = getDirectoryPath(containingFileName);
for (let e of supportedExtensions) {
if (e === ext) {
break;
}
else {
failedLookupLocations.push(normalizePath(getRootLength(moduleName) === 0 ? combinePaths(dir, moduleName) : moduleName) + e);
}
}
assert.equal(resolution.resolvedFileName, undefined);
assert.equal(resolution.failedLookupLocations.length, 3);
assert.deepEqual(resolution.failedLookupLocations, [
moduleFileNameNoExt + ".d.ts",
moduleFileNameNoExt + "/package.json",
moduleFileNameNoExt + "/index.d.ts"
])
assert.deepEqual(resolution.failedLookupLocations, failedLookupLocations);
}
}
@@ -129,28 +123,21 @@ module ts {
testLoadAsFile("c:/foo/bar/baz.ts", "c:/foo", "c:/foo");
});
function testLoadingFromPackageJson(containingFileName: string, packageJsonFileName: string, fieldName: string, fieldRef: string, moduleFileName: string, moduleName: string): void {
function testLoadingFromPackageJson(containingFileName: string, packageJsonFileName: string, fieldRef: string, moduleFileName: string, moduleName: string): void {
let containingFile = { name: containingFileName };
let packageJson = { name: packageJsonFileName, content: JSON.stringify({ [fieldName]: fieldRef }) };
let packageJson = { name: packageJsonFileName, content: JSON.stringify({ "typings": fieldRef }) };
let moduleFile = { name: moduleFileName };
let resolution = nodeModuleNameResolver(moduleName, containingFile.name, createModuleResolutionHost(containingFile, packageJson, moduleFile));
assert.equal(resolution.resolvedFileName, moduleFile.name);
// expect one failed lookup location - attempt to load module as file
assert.equal(resolution.failedLookupLocations.length, 1);
// expect three failed lookup location - attempt to load module as file with all supported extensions
assert.equal(resolution.failedLookupLocations.length, 3);
}
it("module name as directory - load from typings", () => {
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/b/c/bar/package.json", "typings", "c/d/e.d.ts", "/a/b/c/bar/c/d/e.d.ts", "./bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/bar/package.json", "typings", "e.d.ts", "/a/bar/e.d.ts", "../../bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/bar/package.json", "typings", "e.d.ts", "/bar/e.d.ts", "/bar");
testLoadingFromPackageJson("c:/a/b/c/d.ts", "c:/bar/package.json", "typings", "e.d.ts", "c:/bar/e.d.ts", "c:/bar");
});
it("module name as directory - load from main", () => {
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/b/c/bar/package.json", "main", "c/d/e.d.ts", "/a/b/c/bar/c/d/e.d.ts", "./bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/bar/package.json", "main", "e.d.ts", "/a/bar/e.d.ts", "../../bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/bar/package.json", "main", "e.d.ts", "/bar/e.d.ts", "/bar");
testLoadingFromPackageJson("c:/a/b/c/d.ts", "c:/bar/package.json", "main", "e.d.ts", "c:/bar/e.d.ts", "c:/bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/b/c/bar/package.json", "c/d/e.d.ts", "/a/b/c/bar/c/d/e.d.ts", "./bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/a/bar/package.json", "e.d.ts", "/a/bar/e.d.ts", "../../bar");
testLoadingFromPackageJson("/a/b/c/d.ts", "/bar/package.json", "e.d.ts", "/bar/e.d.ts", "/bar");
testLoadingFromPackageJson("c:/a/b/c/d.ts", "c:/bar/package.json", "e.d.ts", "c:/bar/e.d.ts", "c:/bar");
});
it ("module name as directory - load index.d.ts", () => {
@@ -159,10 +146,12 @@ module ts {
let indexFile = { name: "/a/b/foo/index.d.ts" };
let resolution = nodeModuleNameResolver("./foo", containingFile.name, createModuleResolutionHost(containingFile, packageJson, indexFile));
assert.equal(resolution.resolvedFileName, indexFile.name);
// expect 2 failed lookup locations:
assert.deepEqual(resolution.failedLookupLocations, [
"/a/b/foo.d.ts",
"/c/d.d.ts"
"/a/b/foo.ts",
"/a/b/foo.tsx",
"/a/b/foo.d.ts",
"/a/b/foo/index.ts",
"/a/b/foo/index.tsx",
]);
});
});