From 051edacf260d0505f3abd3a34de480d8e3770eba Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 21 Dec 2016 15:03:37 -0800 Subject: [PATCH 1/5] Look for package.json::main when resolving JS targets --- src/compiler/moduleNameResolver.ts | 9 ++- src/harness/unittests/moduleResolution.ts | 68 +++++++++++++------ ...lutionWithExtensions_unexpected.trace.json | 2 + .../untypedModuleImport_MainInPackageJson.js | 22 ++++++ ...ypedModuleImport_MainInPackageJson.symbols | 7 ++ ...ntypedModuleImport_MainInPackageJson.types | 10 +++ ...ypedModuleImport_noImplicitAny2.errors.txt | 18 +++++ .../untypedModuleImport_noImplicitAny2.js | 18 +++++ .../untypedModuleImport_MainInPackageJson.ts | 16 +++++ .../untypedModuleImport_noImplicitAny2.ts | 15 ++++ 10 files changed, 163 insertions(+), 22 deletions(-) create mode 100644 tests/baselines/reference/untypedModuleImport_MainInPackageJson.js create mode 100644 tests/baselines/reference/untypedModuleImport_MainInPackageJson.symbols create mode 100644 tests/baselines/reference/untypedModuleImport_MainInPackageJson.types create mode 100644 tests/baselines/reference/untypedModuleImport_noImplicitAny2.errors.txt create mode 100644 tests/baselines/reference/untypedModuleImport_noImplicitAny2.js create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_MainInPackageJson.ts create mode 100644 tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny2.ts diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index c2fcbddaa61dc..391aca96a04ac 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1,4 +1,4 @@ -/// +/// /// namespace ts { @@ -726,6 +726,13 @@ namespace ts { if (resolved) { return resolved; } + + if (extensions === Extensions.JavaScript) { + const resolved = tryAddingExtensions(mainOrTypesFile, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); + if (resolved) { + return resolved; + } + } } else { if (state.traceEnabled) { diff --git a/src/harness/unittests/moduleResolution.ts b/src/harness/unittests/moduleResolution.ts index 35313e153082c..88594dc6f0594 100644 --- a/src/harness/unittests/moduleResolution.ts +++ b/src/harness/unittests/moduleResolution.ts @@ -1,4 +1,4 @@ -/// +/// namespace ts { export function checkResolvedModule(expected: ResolvedModuleFull, actual: ResolvedModuleFull): boolean { @@ -408,7 +408,7 @@ export = C; "/a/b/c.ts": `/// `, "/a/b/d.ts": "var x" }); - test(files, { module: ts.ModuleKind.AMD }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "/a/b/d.ts"], []); + test(files, { module: ts.ModuleKind.AMD }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "/a/b/d.ts"], []); }); it("should fail when two files used in program differ only in casing (tripleslash references)", () => { @@ -416,7 +416,7 @@ export = C; "/a/b/c.ts": `/// `, "/a/b/d.ts": "var x" }); - test(files, { module: ts.ModuleKind.AMD, forceConsistentCasingInFileNames: true }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "d.ts"], [1149]); + test(files, { module: ts.ModuleKind.AMD, forceConsistentCasingInFileNames: true }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "d.ts"], [1149]); }); it("should fail when two files used in program differ only in casing (imports)", () => { @@ -424,7 +424,7 @@ export = C; "/a/b/c.ts": `import {x} from "D"`, "/a/b/d.ts": "export var x" }); - test(files, { module: ts.ModuleKind.AMD, forceConsistentCasingInFileNames: true }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "d.ts"], [1149]); + test(files, { module: ts.ModuleKind.AMD, forceConsistentCasingInFileNames: true }, "/a/b", /*useCaseSensitiveFileNames*/ false, ["c.ts", "d.ts"], [1149]); }); it("should fail when two files used in program differ only in casing (imports, relative module names)", () => { @@ -432,7 +432,7 @@ export = C; "moduleA.ts": `import {x} from "./ModuleB"`, "moduleB.ts": "export var x" }); - test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "", /*useCaseSensitiveFileNames*/ false, ["moduleA.ts", "moduleB.ts"], [1149]); + test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "", /*useCaseSensitiveFileNames*/ false, ["moduleA.ts", "moduleB.ts"], [1149]); }); it("should fail when two files exist on disk that differs only in casing", () => { @@ -441,7 +441,7 @@ export = C; "/a/b/D.ts": "export var x", "/a/b/d.ts": "export var y" }); - test(files, { module: ts.ModuleKind.AMD }, "/a/b", /*useCaseSensitiveFileNames*/ true, ["c.ts", "d.ts"], [1149]); + test(files, { module: ts.ModuleKind.AMD }, "/a/b", /*useCaseSensitiveFileNames*/ true, ["c.ts", "d.ts"], [1149]); }); it("should fail when module name in 'require' calls has inconsistent casing", () => { @@ -450,7 +450,7 @@ export = C; "moduleB.ts": `import a = require("./moduleC")`, "moduleC.ts": "export var x" }); - test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "", /*useCaseSensitiveFileNames*/ false, ["moduleA.ts", "moduleB.ts", "moduleC.ts"], [1149, 1149]); + test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "", /*useCaseSensitiveFileNames*/ false, ["moduleA.ts", "moduleB.ts", "moduleC.ts"], [1149, 1149]); }); it("should fail when module names in 'require' calls has inconsistent casing and current directory has uppercase chars", () => { @@ -463,7 +463,7 @@ import a = require("./moduleA"); import b = require("./moduleB"); ` }); - test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], [1149]); + test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], [1149]); }); it("should not fail when module names in 'require' calls has consistent casing and current directory has uppercase chars", () => { const files = createMap({ @@ -475,7 +475,7 @@ import a = require("./moduleA"); import b = require("./moduleB"); ` }); - test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], []); + test(files, { module: ts.ModuleKind.CommonJS, forceConsistentCasingInFileNames: true }, "/a/B/c", /*useCaseSensitiveFileNames*/ false, ["moduleD.ts"], []); }); }); @@ -490,7 +490,7 @@ import b = require("./moduleB"); const file2: File = { name: "/root/folder2/file2.ts" }; const file3: File = { name: "/root/folder2/file3.ts" }; const host = createModuleResolutionHost(hasDirectoryExists, file1, file2, file3); - for (const moduleResolution of [ ModuleResolutionKind.NodeJs, ModuleResolutionKind.Classic ]) { + for (const moduleResolution of [ModuleResolutionKind.NodeJs, ModuleResolutionKind.Classic]) { const options: CompilerOptions = { moduleResolution, baseUrl: "/root" }; { const result = resolveModuleName("folder2/file2", file1.name, options, host); @@ -703,7 +703,7 @@ import b = require("./moduleB"); } }); - it ("classic + baseUrl + path mappings", () => { + it("classic + baseUrl + path mappings", () => { // classic mode does not use directoryExists test(/*hasDirectoryExists*/ false); @@ -762,7 +762,7 @@ import b = require("./moduleB"); } }); - it ("node + rootDirs", () => { + it("node + rootDirs", () => { test(/*hasDirectoryExists*/ false); test(/*hasDirectoryExists*/ true); @@ -835,7 +835,7 @@ import b = require("./moduleB"); } }); - it ("classic + rootDirs", () => { + it("classic + rootDirs", () => { test(/*hasDirectoryExists*/ false); function test(hasDirectoryExists: boolean) { @@ -889,7 +889,7 @@ import b = require("./moduleB"); } }); - it ("nested node module", () => { + it("nested node module", () => { test(/*hasDirectoryExists*/ false); test(/*hasDirectoryExists*/ true); @@ -902,9 +902,9 @@ import b = require("./moduleB"); moduleResolution: ModuleResolutionKind.NodeJs, baseUrl: "/root", paths: { - "libs/guid": [ "src/libs/guid" ] + "libs/guid": ["src/libs/guid"] } - }; + }; const result = resolveModuleName("libs/guid", app.name, options, host); checkResolvedModuleWithFailedLookupLocations(result, createResolvedModule(libsTypings.name), [ // first try to load module as file @@ -1020,7 +1020,7 @@ import b = require("./moduleB"); const names = map(files, f => f.name); const sourceFiles = arrayToMap(map(files, f => createSourceFile(f.name, f.content, ScriptTarget.ES2015)), f => f.fileName); const compilerHost: CompilerHost = { - fileExists : fileName => fileName in sourceFiles, + fileExists: fileName => fileName in sourceFiles, getSourceFile: fileName => sourceFiles[fileName], getDefaultLibFileName: () => "lib.d.ts", writeFile: notImplemented, @@ -1042,7 +1042,7 @@ import b = require("./moduleB"); assert.equal(diagnostics1[0].messageText, diagnostics2[0].messageText, "expected one diagnostic"); }); - it ("Modules in the same .d.ts file are preferred to external files", () => { + it("Modules in the same .d.ts file are preferred to external files", () => { const f = { name: "/a/b/c/c/app.d.ts", content: ` @@ -1056,7 +1056,7 @@ import b = require("./moduleB"); }; const file = createSourceFile(f.name, f.content, ScriptTarget.ES2015); const compilerHost: CompilerHost = { - fileExists : fileName => fileName === file.fileName, + fileExists: fileName => fileName === file.fileName, getSourceFile: fileName => fileName === file.fileName ? file : undefined, getDefaultLibFileName: () => "lib.d.ts", writeFile: notImplemented, @@ -1074,7 +1074,7 @@ import b = require("./moduleB"); createProgram([f.name], {}, compilerHost); }); - it ("Modules in .ts file are not checked in the same file", () => { + it("Modules in .ts file are not checked in the same file", () => { const f = { name: "/a/b/c/c/app.ts", content: ` @@ -1088,7 +1088,7 @@ import b = require("./moduleB"); }; const file = createSourceFile(f.name, f.content, ScriptTarget.ES2015); const compilerHost: CompilerHost = { - fileExists : fileName => fileName === file.fileName, + fileExists: fileName => fileName === file.fileName, getSourceFile: fileName => fileName === file.fileName ? file : undefined, getDefaultLibFileName: () => "lib.d.ts", writeFile: notImplemented, @@ -1105,5 +1105,31 @@ import b = require("./moduleB"); }; createProgram([f.name], {}, compilerHost); }); + + it("Module name as directory - load js from 'main'", () => { + test(/*hasDirectoryExists*/ false); + test(/*hasDirectoryExists*/ true); + + function test(hasDirectoryExists: boolean) { + const containingFile = { name: "/a/b.ts" }; + const packageJson = { name: "/a/c/package.json", content: JSON.stringify({ main: "d/e" }) }; + const indexFile = { name: "/a/c/d/e.js" }; + const resolution = nodeModuleNameResolver("./c", containingFile.name, {}, createModuleResolutionHost(hasDirectoryExists, containingFile, packageJson, indexFile)); + checkResolvedModuleWithFailedLookupLocations(resolution, createResolvedModule(indexFile.name), [ + "/a/c.ts", + "/a/c.tsx", + "/a/c.d.ts", + "/a/c/index.ts", + "/a/c/index.tsx", + "/a/c/index.d.ts", + "/a/c.js", + "/a/c.jsx", + "/a/c/d/e", + "/a/c/d/e.ts", + "/a/c/d/e.tsx", + "/a/c/d/e.d.ts", + ]); + } + }); }); } diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json index 84532e8db7b22..8ee33a5286a3a 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json @@ -23,6 +23,8 @@ "File '/node_modules/normalize.css/normalize.css.ts' does not exist.", "File '/node_modules/normalize.css/normalize.css.tsx' does not exist.", "File '/node_modules/normalize.css/normalize.css.d.ts' does not exist.", + "File '/node_modules/normalize.css/normalize.css.js' does not exist.", + "File '/node_modules/normalize.css/normalize.css.jsx' does not exist.", "File '/node_modules/normalize.css/index.js' does not exist.", "File '/node_modules/normalize.css/index.jsx' does not exist.", "======== Module name 'normalize.css' was not resolved. ========" diff --git a/tests/baselines/reference/untypedModuleImport_MainInPackageJson.js b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.js new file mode 100644 index 0000000000000..3b4c84a7b4372 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.js @@ -0,0 +1,22 @@ +//// [tests/cases/conformance/moduleResolution/untypedModuleImport_MainInPackageJson.ts] //// + +//// [foo.js] +// This tests that importing from a JS file globally works in an untyped way. +// (Assuming we don't have `--noImplicitAny` or `--allowJs`.) + +This file is not processed. + +//// [package.json] +{ + "main": "lib/foo" +} + +//// [a.ts] +import * as foo from "foo"; +foo.bar(); + + +//// [a.js] +"use strict"; +var foo = require("foo"); +foo.bar(); diff --git a/tests/baselines/reference/untypedModuleImport_MainInPackageJson.symbols b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.symbols new file mode 100644 index 0000000000000..f5703f555ab0d --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.symbols @@ -0,0 +1,7 @@ +=== /a.ts === +import * as foo from "foo"; +>foo : Symbol(foo, Decl(a.ts, 0, 6)) + +foo.bar(); +>foo : Symbol(foo, Decl(a.ts, 0, 6)) + diff --git a/tests/baselines/reference/untypedModuleImport_MainInPackageJson.types b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.types new file mode 100644 index 0000000000000..9d4e2e909c90c --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_MainInPackageJson.types @@ -0,0 +1,10 @@ +=== /a.ts === +import * as foo from "foo"; +>foo : any + +foo.bar(); +>foo.bar() : any +>foo.bar : any +>foo : any +>bar : any + diff --git a/tests/baselines/reference/untypedModuleImport_noImplicitAny2.errors.txt b/tests/baselines/reference/untypedModuleImport_noImplicitAny2.errors.txt new file mode 100644 index 0000000000000..f912decd6e8d0 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noImplicitAny2.errors.txt @@ -0,0 +1,18 @@ +/a.ts(1,22): error TS7016: Could not find a declaration file for module 'foo'. '/node_modules/foo/lib/foo.js' implicitly has an 'any' type. + + +==== /a.ts (1 errors) ==== + import * as foo from "foo"; + ~~~~~ +!!! error TS7016: Could not find a declaration file for module 'foo'. '/node_modules/foo/lib/foo.js' implicitly has an 'any' type. + +==== /node_modules/foo/lib/foo.js (0 errors) ==== + // This tests that `--noImplicitAny` disables untyped modules. + + This file is not processed. + +==== /node_modules/foo/package.json (0 errors) ==== + { + "main": "lib/foo" + } + \ No newline at end of file diff --git a/tests/baselines/reference/untypedModuleImport_noImplicitAny2.js b/tests/baselines/reference/untypedModuleImport_noImplicitAny2.js new file mode 100644 index 0000000000000..76f33d33132a2 --- /dev/null +++ b/tests/baselines/reference/untypedModuleImport_noImplicitAny2.js @@ -0,0 +1,18 @@ +//// [tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny2.ts] //// + +//// [foo.js] +// This tests that `--noImplicitAny` disables untyped modules. + +This file is not processed. + +//// [package.json] +{ + "main": "lib/foo" +} + +//// [a.ts] +import * as foo from "foo"; + + +//// [a.js] +"use strict"; diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_MainInPackageJson.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_MainInPackageJson.ts new file mode 100644 index 0000000000000..4d6fcc0249b9e --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_MainInPackageJson.ts @@ -0,0 +1,16 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// This tests that importing from a JS file globally works in an untyped way. +// (Assuming we don't have `--noImplicitAny` or `--allowJs`.) + +// @filename: /node_modules/foo/lib/foo.js +This file is not processed. + +// @filename: /node_modules/foo/package.json +{ + "main": "lib/foo" +} + +// @filename: /a.ts +import * as foo from "foo"; +foo.bar(); diff --git a/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny2.ts b/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny2.ts new file mode 100644 index 0000000000000..1293a8812257b --- /dev/null +++ b/tests/cases/conformance/moduleResolution/untypedModuleImport_noImplicitAny2.ts @@ -0,0 +1,15 @@ +// @noImplicitReferences: true +// @currentDirectory: / +// @noImplicitAny: true +// This tests that `--noImplicitAny` disables untyped modules. + +// @filename: /node_modules/foo/lib/foo.js +This file is not processed. + +// @filename: /node_modules/foo/package.json +{ + "main": "lib/foo" +} + +// @filename: /a.ts +import * as foo from "foo"; From 0e16657468d94e151927a3a23406a25d0b9c75f7 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 21 Dec 2016 15:04:08 -0800 Subject: [PATCH 2/5] Refactor the package.json property lookup --- src/compiler/moduleNameResolver.ts | 42 +++++++++++-------- ...lutionWithExtensions_unexpected.trace.json | 4 +- ...utionWithExtensions_unexpected2.trace.json | 1 + 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 391aca96a04ac..dd4b5434c2bc1 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -67,23 +67,13 @@ namespace ts { } /** Reads from "main" or "types"/"typings" depending on `extensions`. */ - function tryReadPackageJsonMainOrTypes(extensions: Extensions, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string { + function tryReadPackageJsonMainAndTypes(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): {types: string, main: string} { const jsonContent = readJson(packageJsonPath, state.host); - switch (extensions) { - case Extensions.DtsOnly: - case Extensions.TypeScript: - return tryReadFromField("typings") || tryReadFromField("types"); - - case Extensions.JavaScript: - if (typeof jsonContent.main === "string") { - if (state.traceEnabled) { - trace(state.host, Diagnostics.No_types_specified_in_package_json_so_returning_main_value_of_0, jsonContent.main); - } - return normalizePath(combinePaths(baseDirectory, jsonContent.main)); - } - return undefined; - } + return { + types: tryReadFromField("typings") || tryReadFromField("types"), + main: tryReadFromField("main") + }; function tryReadFromField(fieldName: string) { if (hasProperty(jsonContent, fieldName)) { @@ -710,7 +700,21 @@ namespace ts { if (state.traceEnabled) { trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath); } - const mainOrTypesFile = tryReadPackageJsonMainOrTypes(extensions, packageJsonPath, candidate, state); + + const {types, main} = tryReadPackageJsonMainAndTypes(packageJsonPath, candidate, state); + let mainOrTypesFile; + if (extensions === Extensions.DtsOnly || extensions === Extensions.TypeScript) { + mainOrTypesFile = types; + } + else if (extensions === Extensions.JavaScript) { + mainOrTypesFile = main; + if (main) { + if (state.traceEnabled) { + trace(state.host, Diagnostics.No_types_specified_in_package_json_so_returning_main_value_of_0, main); + } + } + } + if (mainOrTypesFile) { const onlyRecordFailures = !directoryProbablyExists(getDirectoryPath(mainOrTypesFile), state.host); // A package.json "typings" may specify an exact filename, or may choose to omit an extension. @@ -727,8 +731,10 @@ namespace ts { return resolved; } - if (extensions === Extensions.JavaScript) { - const resolved = tryAddingExtensions(mainOrTypesFile, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); + // A package.json "main" may specify an exact filename, or may choose to omit an extension. + // We tried the ts extensions erlier, now try the js extensions + if (main && extensions === Extensions.JavaScript) { + const resolved = tryAddingExtensions(main, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); if (resolved) { return resolved; } diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json index 8ee33a5286a3a..f25c36542334d 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json @@ -6,6 +6,7 @@ "File '/node_modules/normalize.css.tsx' does not exist.", "File '/node_modules/normalize.css.d.ts' does not exist.", "Found 'package.json' at '/node_modules/normalize.css/package.json'.", + "'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.", "'package.json' does not have a 'types' or 'main' field.", "File '/node_modules/normalize.css/index.ts' does not exist.", "File '/node_modules/normalize.css/index.tsx' does not exist.", @@ -17,7 +18,8 @@ "File '/node_modules/normalize.css.js' does not exist.", "File '/node_modules/normalize.css.jsx' does not exist.", "Found 'package.json' at '/node_modules/normalize.css/package.json'.", - "No types specified in 'package.json', so returning 'main' value of 'normalize.css'", + "'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.", + "No types specified in 'package.json', so returning 'main' value of '/node_modules/normalize.css/normalize.css'", "File '/node_modules/normalize.css/normalize.css' exist - use it as a name resolution result.", "File '/node_modules/normalize.css/normalize.css' has an unsupported extension, so skipping it.", "File '/node_modules/normalize.css/normalize.css.ts' does not exist.", diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json index 27c9d2243e5f2..57219250f0c1f 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json @@ -22,6 +22,7 @@ "File '/node_modules/foo.js' does not exist.", "File '/node_modules/foo.jsx' does not exist.", "Found 'package.json' at '/node_modules/foo/package.json'.", + "'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.", "'package.json' does not have a 'types' or 'main' field.", "File '/node_modules/foo/index.js' does not exist.", "File '/node_modules/foo/index.jsx' does not exist.", From 12f1d19fa4fb030f914ca067ebe8553f3562fb74 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 21 Dec 2016 15:45:37 -0800 Subject: [PATCH 3/5] Revert "Refactor the package.json property lookup" This reverts commit 0e16657468d94e151927a3a23406a25d0b9c75f7. --- src/compiler/moduleNameResolver.ts | 42 ++++++++----------- ...lutionWithExtensions_unexpected.trace.json | 4 +- ...utionWithExtensions_unexpected2.trace.json | 1 - 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index dd4b5434c2bc1..391aca96a04ac 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -67,13 +67,23 @@ namespace ts { } /** Reads from "main" or "types"/"typings" depending on `extensions`. */ - function tryReadPackageJsonMainAndTypes(packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): {types: string, main: string} { + function tryReadPackageJsonMainOrTypes(extensions: Extensions, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string { const jsonContent = readJson(packageJsonPath, state.host); - return { - types: tryReadFromField("typings") || tryReadFromField("types"), - main: tryReadFromField("main") - }; + switch (extensions) { + case Extensions.DtsOnly: + case Extensions.TypeScript: + return tryReadFromField("typings") || tryReadFromField("types"); + + case Extensions.JavaScript: + if (typeof jsonContent.main === "string") { + if (state.traceEnabled) { + trace(state.host, Diagnostics.No_types_specified_in_package_json_so_returning_main_value_of_0, jsonContent.main); + } + return normalizePath(combinePaths(baseDirectory, jsonContent.main)); + } + return undefined; + } function tryReadFromField(fieldName: string) { if (hasProperty(jsonContent, fieldName)) { @@ -700,21 +710,7 @@ namespace ts { if (state.traceEnabled) { trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath); } - - const {types, main} = tryReadPackageJsonMainAndTypes(packageJsonPath, candidate, state); - let mainOrTypesFile; - if (extensions === Extensions.DtsOnly || extensions === Extensions.TypeScript) { - mainOrTypesFile = types; - } - else if (extensions === Extensions.JavaScript) { - mainOrTypesFile = main; - if (main) { - if (state.traceEnabled) { - trace(state.host, Diagnostics.No_types_specified_in_package_json_so_returning_main_value_of_0, main); - } - } - } - + const mainOrTypesFile = tryReadPackageJsonMainOrTypes(extensions, packageJsonPath, candidate, state); if (mainOrTypesFile) { const onlyRecordFailures = !directoryProbablyExists(getDirectoryPath(mainOrTypesFile), state.host); // A package.json "typings" may specify an exact filename, or may choose to omit an extension. @@ -731,10 +727,8 @@ namespace ts { return resolved; } - // A package.json "main" may specify an exact filename, or may choose to omit an extension. - // We tried the ts extensions erlier, now try the js extensions - if (main && extensions === Extensions.JavaScript) { - const resolved = tryAddingExtensions(main, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); + if (extensions === Extensions.JavaScript) { + const resolved = tryAddingExtensions(mainOrTypesFile, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); if (resolved) { return resolved; } diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json index f25c36542334d..8ee33a5286a3a 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json @@ -6,7 +6,6 @@ "File '/node_modules/normalize.css.tsx' does not exist.", "File '/node_modules/normalize.css.d.ts' does not exist.", "Found 'package.json' at '/node_modules/normalize.css/package.json'.", - "'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.", "'package.json' does not have a 'types' or 'main' field.", "File '/node_modules/normalize.css/index.ts' does not exist.", "File '/node_modules/normalize.css/index.tsx' does not exist.", @@ -18,8 +17,7 @@ "File '/node_modules/normalize.css.js' does not exist.", "File '/node_modules/normalize.css.jsx' does not exist.", "Found 'package.json' at '/node_modules/normalize.css/package.json'.", - "'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.", - "No types specified in 'package.json', so returning 'main' value of '/node_modules/normalize.css/normalize.css'", + "No types specified in 'package.json', so returning 'main' value of 'normalize.css'", "File '/node_modules/normalize.css/normalize.css' exist - use it as a name resolution result.", "File '/node_modules/normalize.css/normalize.css' has an unsupported extension, so skipping it.", "File '/node_modules/normalize.css/normalize.css.ts' does not exist.", diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json index 57219250f0c1f..27c9d2243e5f2 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json @@ -22,7 +22,6 @@ "File '/node_modules/foo.js' does not exist.", "File '/node_modules/foo.jsx' does not exist.", "Found 'package.json' at '/node_modules/foo/package.json'.", - "'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.", "'package.json' does not have a 'types' or 'main' field.", "File '/node_modules/foo/index.js' does not exist.", "File '/node_modules/foo/index.jsx' does not exist.", From e096e308d6ecb1d9439d2bdb8e700a14cbe6a2e4 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 21 Dec 2016 15:47:19 -0800 Subject: [PATCH 4/5] Add comment --- src/compiler/moduleNameResolver.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 391aca96a04ac..e158a4b798004 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -728,6 +728,9 @@ namespace ts { } if (extensions === Extensions.JavaScript) { + // A package.json "main" may specify an exact filename, or may choose to omit an extension. + // We tried the ts extensions erlier, now try the js extensions. + // tryReadPackageJsonMainOrTypes returns main iff extensions is Extensions.JavaScript. const resolved = tryAddingExtensions(mainOrTypesFile, Extensions.JavaScript, failedLookupLocations, onlyRecordFailures, state); if (resolved) { return resolved; From 12e0275f1ad549ee4e62de87b3ef6aed08ecc194 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 21 Dec 2016 16:40:30 -0800 Subject: [PATCH 5/5] Remove invalid trace --- src/compiler/moduleNameResolver.ts | 8 +------- .../moduleResolutionWithExtensions_unexpected.trace.json | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index e158a4b798004..0d3db5362c1dd 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -76,13 +76,7 @@ namespace ts { return tryReadFromField("typings") || tryReadFromField("types"); case Extensions.JavaScript: - if (typeof jsonContent.main === "string") { - if (state.traceEnabled) { - trace(state.host, Diagnostics.No_types_specified_in_package_json_so_returning_main_value_of_0, jsonContent.main); - } - return normalizePath(combinePaths(baseDirectory, jsonContent.main)); - } - return undefined; + return tryReadFromField("main"); } function tryReadFromField(fieldName: string) { diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json index 8ee33a5286a3a..c4489ac057daf 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json @@ -17,7 +17,7 @@ "File '/node_modules/normalize.css.js' does not exist.", "File '/node_modules/normalize.css.jsx' does not exist.", "Found 'package.json' at '/node_modules/normalize.css/package.json'.", - "No types specified in 'package.json', so returning 'main' value of 'normalize.css'", + "'package.json' has 'main' field 'normalize.css' that references '/node_modules/normalize.css/normalize.css'.", "File '/node_modules/normalize.css/normalize.css' exist - use it as a name resolution result.", "File '/node_modules/normalize.css/normalize.css' has an unsupported extension, so skipping it.", "File '/node_modules/normalize.css/normalize.css.ts' does not exist.",