Skip to content

Commit

Permalink
Merge pull request #13105 from Microsoft/Fix12921
Browse files Browse the repository at this point in the history
Check for `.js` targets for `package.json`::`main`
  • Loading branch information
mhegazy committed Dec 22, 2016
2 parents c5904d3 + 12e0275 commit 2cf9979
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 30 deletions.
20 changes: 12 additions & 8 deletions src/compiler/moduleNameResolver.ts
@@ -1,4 +1,4 @@
/// <reference path="core.ts" />
/// <reference path="core.ts" />
/// <reference path="diagnosticInformationMap.generated.ts" />

namespace ts {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -830,6 +824,16 @@ namespace ts {
if (resolved) {
return resolved;
}

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;
}
}
}
else {
if (state.traceEnabled) {
Expand Down
68 changes: 47 additions & 21 deletions src/harness/unittests/moduleResolution.ts
@@ -1,4 +1,4 @@
/// <reference path="..\harness.ts" />
/// <reference path="..\harness.ts" />

namespace ts {
export function checkResolvedModule(expected: ResolvedModuleFull, actual: ResolvedModuleFull): boolean {
Expand Down Expand Up @@ -408,31 +408,31 @@ export = C;
"/a/b/c.ts": `/// <reference path="d.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)", () => {
const files = createMap({
"/a/b/c.ts": `/// <reference path="D.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)", () => {
const files = createMap({
"/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)", () => {
const files = createMap({
"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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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({
Expand All @@ -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"], []);
});
});

Expand All @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -762,7 +762,7 @@ import b = require("./moduleB");
}
});

it ("node + rootDirs", () => {
it("node + rootDirs", () => {
test(/*hasDirectoryExists*/ false);
test(/*hasDirectoryExists*/ true);

Expand Down Expand Up @@ -835,7 +835,7 @@ import b = require("./moduleB");
}
});

it ("classic + rootDirs", () => {
it("classic + rootDirs", () => {
test(/*hasDirectoryExists*/ false);

function test(hasDirectoryExists: boolean) {
Expand Down Expand Up @@ -889,7 +889,7 @@ import b = require("./moduleB");
}
});

it ("nested node module", () => {
it("nested node module", () => {
test(/*hasDirectoryExists*/ false);
test(/*hasDirectoryExists*/ true);

Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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: `
Expand All @@ -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,
Expand All @@ -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: `
Expand All @@ -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,
Expand All @@ -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",
]);
}
});
});
}
Expand Up @@ -17,12 +17,14 @@
"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.",
"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. ========"
Expand Down
22 changes: 22 additions & 0 deletions 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();
@@ -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))

@@ -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

@@ -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"
}

18 changes: 18 additions & 0 deletions 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";
@@ -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();
@@ -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";

0 comments on commit 2cf9979

Please sign in to comment.