Skip to content
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.

Sign up for GitHub

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

Resolve using package json's "main" field when resolving typescript extensions #24112

Merged
merged 3 commits into from
May 15, 2018
Merged

Resolve using package json's "main" field when resolving typescript extensions #24112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7be8595
Add test for module resolution with package json containing main fiel…
sheetalkamat May 14, 2018
22d5b0e
Do not remove extension js or jsx from the subModuleName if the subMo…
sheetalkamat May 14, 2018
dc50fe5
Apart from typing, use main field in package json to resolve typescri…
sheetalkamat May 14, 2018
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
19 changes: 15 additions & 4 deletions src/compiler/moduleNameResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,8 +1012,10 @@ namespace ts {
}
else {
const jsPath = tryReadPackageJsonFields(/*readTypes*/ false, packageJsonContent, nodeModuleDirectory, state);
if (typeof jsPath === "string") {
subModuleName = removeExtension(removeExtension(jsPath.substring(nodeModuleDirectory.length + 1), Extension.Js), Extension.Jsx) + Extension.Dts;
if (typeof jsPath === "string" && jsPath.length > nodeModuleDirectory.length) {
const potentialSubModule = jsPath.substring(nodeModuleDirectory.length + 1);
subModuleName = (forEach(supportedJavascriptExtensions, extension =>
tryRemoveExtension(potentialSubModule, extension)) || potentialSubModule) + Extension.Dts;
}
else {
subModuleName = "index.d.ts";
Expand Down Expand Up @@ -1047,9 +1049,18 @@ namespace ts {
}

function loadModuleFromPackageJson(jsonContent: PackageJsonPathFields, extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, state: ModuleResolutionState): PathAndExtension | undefined {
const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript && extensions !== Extensions.Json, jsonContent, candidate, state);
let file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript && extensions !== Extensions.Json, jsonContent, candidate, state);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: might just modify tryReadPackageJsonFields to take separate readTypes and readMain parameters, if we might want both at once.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most both cases different conditions determine if we read main types so i would rather leave it as is.

if (!file) {
return undefined;
if (extensions === Extensions.TypeScript) {
// When resolving typescript modules, try resolving using main field as well
file = tryReadPackageJsonFields(/*readTypes*/ false, jsonContent, candidate, state);
if (!file) {
return undefined;
}
}
else {
return undefined;
}
}

const onlyRecordFailures = !directoryProbablyExists(getDirectoryPath(file), state.host);
Expand Down
7 changes: 7 additions & 0 deletions src/harness/unittests/moduleResolution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ namespace ts {
"/a/b/foo.ts",
"/a/b/foo.tsx",
"/a/b/foo.d.ts",
"/c/d",
"/c/d.ts",
"/c/d.tsx",
"/c/d.d.ts",
"/c/d/index.ts",
"/c/d/index.tsx",
"/c/d/index.d.ts",
"/a/b/foo/index.ts",
"/a/b/foo/index.tsx",
]);
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/duplicatePackage_relativeImportWithinPackage.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"File '/node_modules/a/node_modules/foo.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' does not have a 'main' field.",
"File '/node_modules/a/node_modules/foo/index.ts' does not exist.",
"File '/node_modules/a/node_modules/foo/index.tsx' does not exist.",
"File '/node_modules/a/node_modules/foo/index.d.ts' exist - use it as a name resolution result.",
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/duplicatePackage_relativeImportWithinPackage_scoped.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"File '/node_modules/a/node_modules/@foo/bar.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' does not have a 'main' field.",
"File '/node_modules/a/node_modules/@foo/bar/index.ts' does not exist.",
"File '/node_modules/a/node_modules/@foo/bar/index.tsx' does not exist.",
"File '/node_modules/a/node_modules/@foo/bar/index.d.ts' exist - use it as a name resolution result.",
Expand Down
8 changes: 8 additions & 0 deletions tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@
"File '/node_modules/normalize.css.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'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.",
"Loading module as file / folder, candidate module location '/node_modules/normalize.css/normalize.css', target file type 'TypeScript'.",
"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.",
"Directory '/node_modules/normalize.css/normalize.css' does not exist, skipping all lookups in it.",
"File '/node_modules/normalize.css/index.ts' does not exist.",
"File '/node_modules/normalize.css/index.tsx' does not exist.",
"File '/node_modules/normalize.css/index.d.ts' does not exist.",
Expand Down
15 changes: 15 additions & 0 deletions ...elines/reference/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//// [tests/cases/compiler/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.ts] ////

//// [package.json]
{ "name": "foo", "version": "1.2.3", "main": "src/index.js" }

//// [index.d.ts]
export const x: number;

//// [index.ts]
import { x } from "foo";


//// [index.js]
"use strict";
exports.__esModule = true;
8 changes: 8 additions & 0 deletions ...s/reference/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== /node_modules/foo/src/index.d.ts ===
export const x: number;
>x : Symbol(x, Decl(index.d.ts, 0, 12))

=== /index.ts ===
import { x } from "foo";
>x : Symbol(x, Decl(index.ts, 0, 8))

26 changes: 26 additions & 0 deletions ...eference/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
[
"======== Resolving module 'foo' from '/index.ts'. ========",
"Module resolution kind is not specified, using 'NodeJs'.",
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'src/index.js' that references '/node_modules/foo/src/index.js'.",
"Found 'package.json' at '/node_modules/foo/package.json'. Package ID is 'foo/src/[email protected]'.",
"File '/node_modules/foo.ts' does not exist.",
"File '/node_modules/foo.tsx' does not exist.",
"File '/node_modules/foo.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'src/index.js' that references '/node_modules/foo/src/index.js'.",
"File '/node_modules/foo/src/index.js' does not exist.",
"Loading module as file / folder, candidate module location '/node_modules/foo/src/index.js', target file type 'TypeScript'.",
"File '/node_modules/foo/src/index.js.ts' does not exist.",
"File '/node_modules/foo/src/index.js.tsx' does not exist.",
"File '/node_modules/foo/src/index.js.d.ts' does not exist.",
"File name '/node_modules/foo/src/index.js' has a '.js' extension - stripping it.",
"File '/node_modules/foo/src/index.ts' does not exist.",
"File '/node_modules/foo/src/index.tsx' does not exist.",
"File '/node_modules/foo/src/index.d.ts' exist - use it as a name resolution result.",
"Resolving real path for '/node_modules/foo/src/index.d.ts', result '/node_modules/foo/src/index.d.ts'.",
"======== Module name 'foo' was successfully resolved to '/node_modules/foo/src/index.d.ts'. ========"
]
8 changes: 8 additions & 0 deletions ...nes/reference/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
=== /node_modules/foo/src/index.d.ts ===
export const x: number;
>x : number

=== /index.ts ===
import { x } from "foo";
>x : number

28 changes: 28 additions & 0 deletions tests/baselines/reference/packageJsonMain.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
"File '/node_modules/foo.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'oof' that references '/node_modules/foo/oof'.",
"File '/node_modules/foo/oof' does not exist.",
"Loading module as file / folder, candidate module location '/node_modules/foo/oof', target file type 'TypeScript'.",
"File '/node_modules/foo/oof.ts' does not exist.",
"File '/node_modules/foo/oof.tsx' does not exist.",
"File '/node_modules/foo/oof.d.ts' does not exist.",
"Directory '/node_modules/foo/oof' does not exist, skipping all lookups in it.",
"File '/node_modules/foo/index.ts' does not exist.",
"File '/node_modules/foo/index.tsx' does not exist.",
"File '/node_modules/foo/index.d.ts' does not exist.",
Expand Down Expand Up @@ -40,6 +47,18 @@
"File '/node_modules/bar.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'rab.js' that references '/node_modules/bar/rab.js'.",
"File '/node_modules/bar/rab.js' exist - use it as a name resolution result.",
"File '/node_modules/bar/rab.js' has an unsupported extension, so skipping it.",
"Loading module as file / folder, candidate module location '/node_modules/bar/rab.js', target file type 'TypeScript'.",
"File '/node_modules/bar/rab.js.ts' does not exist.",
"File '/node_modules/bar/rab.js.tsx' does not exist.",
"File '/node_modules/bar/rab.js.d.ts' does not exist.",
"File name '/node_modules/bar/rab.js' has a '.js' extension - stripping it.",
"File '/node_modules/bar/rab.ts' does not exist.",
"File '/node_modules/bar/rab.tsx' does not exist.",
"File '/node_modules/bar/rab.d.ts' does not exist.",
"Directory '/node_modules/bar/rab.js' does not exist, skipping all lookups in it.",
"File '/node_modules/bar/index.ts' does not exist.",
"File '/node_modules/bar/index.tsx' does not exist.",
"File '/node_modules/bar/index.d.ts' does not exist.",
Expand Down Expand Up @@ -67,6 +86,15 @@
"File '/node_modules/baz.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'zab' that references '/node_modules/baz/zab'.",
"File '/node_modules/baz/zab' does not exist.",
"Loading module as file / folder, candidate module location '/node_modules/baz/zab', target file type 'TypeScript'.",
"File '/node_modules/baz/zab.ts' does not exist.",
"File '/node_modules/baz/zab.tsx' does not exist.",
"File '/node_modules/baz/zab.d.ts' does not exist.",
"File '/node_modules/baz/zab/index.ts' does not exist.",
"File '/node_modules/baz/zab/index.tsx' does not exist.",
"File '/node_modules/baz/zab/index.d.ts' does not exist.",
"File '/node_modules/baz/index.ts' does not exist.",
"File '/node_modules/baz/index.tsx' does not exist.",
"File '/node_modules/baz/index.d.ts' does not exist.",
Expand Down
9 changes: 9 additions & 0 deletions tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@
"File '/node_modules/foo.d.ts' does not exist.",
"'package.json' does not have a 'typings' field.",
"'package.json' does not have a 'types' field.",
"'package.json' has 'main' field 'oof' that references '/node_modules/foo/oof'.",
"File '/node_modules/foo/oof' does not exist.",
"Loading module as file / folder, candidate module location '/node_modules/foo/oof', target file type 'TypeScript'.",
"File '/node_modules/foo/oof.ts' does not exist.",
"File '/node_modules/foo/oof.tsx' does not exist.",
"File '/node_modules/foo/oof.d.ts' does not exist.",
"File '/node_modules/foo/oof/index.ts' does not exist.",
"File '/node_modules/foo/oof/index.tsx' does not exist.",
"File '/node_modules/foo/oof/index.d.ts' does not exist.",
"File '/node_modules/foo/index.ts' does not exist.",
"File '/node_modules/foo/index.tsx' does not exist.",
"File '/node_modules/foo/index.d.ts' does not exist.",
Expand Down
10 changes: 10 additions & 0 deletions ...s/cases/compiler/moduleResolution_packageJson_yesAtPackageRoot_mainFieldInSubDirectory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @traceResolution: true

// @Filename: /node_modules/foo/package.json
{ "name": "foo", "version": "1.2.3", "main": "src/index.js" }

// @Filename: /node_modules/foo/src/index.d.ts
export const x: number;

// @Filename: /index.ts
import { x } from "foo";