Skip to content

Commit 132fa70

Browse files
author
Andy Hanson
committed
Respond to comments
1 parent 432808d commit 132fa70

7 files changed

+19
-11
lines changed

‎src/compiler/moduleNameResolver.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,15 @@ namespace ts {
6767
}
6868

6969
/** Reads from "main" or "types"/"typings" depending on `extensions`. */
70-
function tryReadPackageJsonFields(ts: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined {
70+
function tryReadPackageJsonFields(readTypes: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined {
7171
const jsonContent = readJson(packageJsonPath, state.host);
72-
const file = ts ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main");
73-
if (!file && state.traceEnabled) {
74-
trace(state.host, Diagnostics.package_json_does_not_have_a_0_field, ts ? "types" : "main");
75-
}
76-
return file;
72+
return readTypes ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main");
7773

7874
function tryReadFromField(fieldName: "typings" | "types" | "main"): string | undefined {
7975
if (!hasProperty(jsonContent, fieldName)) {
76+
if (state.traceEnabled) {
77+
trace(state.host, Diagnostics.package_json_does_not_have_a_0_field, fieldName);
78+
}
8079
return;
8180
}
8281

@@ -690,7 +689,8 @@ namespace ts {
690689
return { resolvedModule: undefined, failedLookupLocations };
691690

692691
function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> {
693-
const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, nodeLoadModuleByRelativeName, failedLookupLocations, state);
692+
const loader: ResolutionKindSpecificLoader = (extensions, candidate, failedLookupLocations, onlyRecordFailures, state) => nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/true);
693+
const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loader, failedLookupLocations, state);
694694
if (resolved) {
695695
return toSearchResult({ resolved, isExternalLibraryImport: false });
696696
}
@@ -705,7 +705,7 @@ namespace ts {
705705
}
706706
else {
707707
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));
708-
const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state);
708+
const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/true);
709709
return resolved && toSearchResult({ resolved, isExternalLibraryImport: false });
710710
}
711711
}
@@ -723,7 +723,7 @@ namespace ts {
723723
return real;
724724
}
725725

726-
function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson = true): Resolved | undefined {
726+
function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson: boolean): Resolved | undefined {
727727
if (state.traceEnabled) {
728728
trace(state.host, Diagnostics.Loading_module_as_file_Slash_folder_candidate_module_location_0_target_file_type_1, candidate, Extensions[extensions]);
729729
}
@@ -855,8 +855,7 @@ namespace ts {
855855
trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath);
856856
}
857857

858-
const ts = extensions !== Extensions.JavaScript;
859-
const file = tryReadPackageJsonFields(ts, packageJsonPath, candidate, state);
858+
const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, packageJsonPath, candidate, state);
860859
if (!file) {
861860
return undefined;
862861
}

‎tests/baselines/reference/library-reference-12.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"Directory '/a/b/node_modules' does not exist, skipping all lookups in it.",
66
"File '/a/node_modules/jquery.d.ts' does not exist.",
77
"Found 'package.json' at '/a/node_modules/jquery/package.json'.",
8+
"'package.json' does not have a 'typings' field.",
89
"'package.json' has 'types' field 'dist/jquery.d.ts' that references '/a/node_modules/jquery/dist/jquery.d.ts'.",
910
"File '/a/node_modules/jquery/dist/jquery.d.ts' exist - use it as a name resolution result.",
1011
"Resolving real path for '/a/node_modules/jquery/dist/jquery.d.ts', result '/a/node_modules/jquery/dist/jquery.d.ts'",

‎tests/baselines/reference/library-reference-2.trace.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22
"======== Resolving type reference directive 'jquery', containing file '/consumer.ts', root directory '/types'. ========",
33
"Resolving with primary search path '/types'",
44
"Found 'package.json' at '/types/jquery/package.json'.",
5+
"'package.json' does not have a 'typings' field.",
56
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",
67
"File '/types/jquery/jquery.d.ts' exist - use it as a name resolution result.",
78
"Resolving real path for '/types/jquery/jquery.d.ts', result '/types/jquery/jquery.d.ts'",
89
"======== Type reference directive 'jquery' was successfully resolved to '/types/jquery/jquery.d.ts', primary: true. ========",
910
"======== Resolving type reference directive 'jquery', containing file 'test/__inferred type names__.ts', root directory '/types'. ========",
1011
"Resolving with primary search path '/types'",
1112
"Found 'package.json' at '/types/jquery/package.json'.",
13+
"'package.json' does not have a 'typings' field.",
1214
"'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.",
1315
"File '/types/jquery/jquery.d.ts' exist - use it as a name resolution result.",
1416
"Resolving real path for '/types/jquery/jquery.d.ts', result '/types/jquery/jquery.d.ts'",

‎tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"File '/node_modules/normalize.css.tsx' does not exist.",
77
"File '/node_modules/normalize.css.d.ts' does not exist.",
88
"Found 'package.json' at '/node_modules/normalize.css/package.json'.",
9+
"'package.json' does not have a 'typings' field.",
910
"'package.json' does not have a 'types' field.",
1011
"File '/node_modules/normalize.css/index.ts' does not exist.",
1112
"File '/node_modules/normalize.css/index.tsx' does not exist.",

‎tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"File '/node_modules/foo.tsx' does not exist.",
77
"File '/node_modules/foo.d.ts' does not exist.",
88
"Found 'package.json' at '/node_modules/foo/package.json'.",
9+
"'package.json' does not have a 'typings' field.",
910
"'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.",
1011
"File '/node_modules/foo/foo.js' exist - use it as a name resolution result.",
1112
"File '/node_modules/foo/foo.js' has an unsupported extension, so skipping it.",

‎tests/baselines/reference/packageJsonMain.trace.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"File '/node_modules/foo.tsx' does not exist.",
77
"File '/node_modules/foo.d.ts' does not exist.",
88
"Found 'package.json' at '/node_modules/foo/package.json'.",
9+
"'package.json' does not have a 'typings' field.",
910
"'package.json' does not have a 'types' field.",
1011
"File '/node_modules/foo/index.ts' does not exist.",
1112
"File '/node_modules/foo/index.tsx' does not exist.",
@@ -28,6 +29,7 @@
2829
"File '/node_modules/bar.tsx' does not exist.",
2930
"File '/node_modules/bar.d.ts' does not exist.",
3031
"Found 'package.json' at '/node_modules/bar/package.json'.",
32+
"'package.json' does not have a 'typings' field.",
3133
"'package.json' does not have a 'types' field.",
3234
"File '/node_modules/bar/index.ts' does not exist.",
3335
"File '/node_modules/bar/index.tsx' does not exist.",
@@ -48,6 +50,7 @@
4850
"File '/node_modules/baz.tsx' does not exist.",
4951
"File '/node_modules/baz.d.ts' does not exist.",
5052
"Found 'package.json' at '/node_modules/baz/package.json'.",
53+
"'package.json' does not have a 'typings' field.",
5154
"'package.json' does not have a 'types' field.",
5255
"File '/node_modules/baz/index.ts' does not exist.",
5356
"File '/node_modules/baz/index.tsx' does not exist.",

‎tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
"File '/node_modules/foo.tsx' does not exist.",
77
"File '/node_modules/foo.d.ts' does not exist.",
88
"Found 'package.json' at '/node_modules/foo/package.json'.",
9+
"'package.json' does not have a 'typings' field.",
910
"'package.json' does not have a 'types' field.",
1011
"File '/node_modules/foo/index.ts' does not exist.",
1112
"File '/node_modules/foo/index.tsx' does not exist.",

0 commit comments

Comments
 (0)