From 132fa7093ed64bc0e78a9a3bb4ca7a7325e540be Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 26 Jan 2017 14:26:49 -0800 Subject: [PATCH] Respond to comments --- src/compiler/moduleNameResolver.ts | 21 +++++++++---------- .../reference/library-reference-12.trace.json | 1 + .../reference/library-reference-2.trace.json | 2 ++ ...lutionWithExtensions_unexpected.trace.json | 1 + ...utionWithExtensions_unexpected2.trace.json | 1 + .../reference/packageJsonMain.trace.json | 3 +++ .../packageJsonMain_isNonRecursive.trace.json | 1 + 7 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 519b1d5fad2..68b043f1439 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -67,16 +67,15 @@ namespace ts { } /** Reads from "main" or "types"/"typings" depending on `extensions`. */ - function tryReadPackageJsonFields(ts: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined { + function tryReadPackageJsonFields(readTypes: boolean, packageJsonPath: string, baseDirectory: string, state: ModuleResolutionState): string | undefined { const jsonContent = readJson(packageJsonPath, state.host); - const file = ts ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main"); - if (!file && state.traceEnabled) { - trace(state.host, Diagnostics.package_json_does_not_have_a_0_field, ts ? "types" : "main"); - } - return file; + return readTypes ? tryReadFromField("typings") || tryReadFromField("types") : tryReadFromField("main"); function tryReadFromField(fieldName: "typings" | "types" | "main"): string | undefined { if (!hasProperty(jsonContent, fieldName)) { + if (state.traceEnabled) { + trace(state.host, Diagnostics.package_json_does_not_have_a_0_field, fieldName); + } return; } @@ -690,7 +689,8 @@ namespace ts { return { resolvedModule: undefined, failedLookupLocations }; function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> { - const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, nodeLoadModuleByRelativeName, failedLookupLocations, state); + const loader: ResolutionKindSpecificLoader = (extensions, candidate, failedLookupLocations, onlyRecordFailures, state) => nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/true); + const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loader, failedLookupLocations, state); if (resolved) { return toSearchResult({ resolved, isExternalLibraryImport: false }); } @@ -705,7 +705,7 @@ namespace ts { } else { const candidate = normalizePath(combinePaths(containingDirectory, moduleName)); - const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state); + const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/true); return resolved && toSearchResult({ resolved, isExternalLibraryImport: false }); } } @@ -723,7 +723,7 @@ namespace ts { return real; } - function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson = true): Resolved | undefined { + function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push, onlyRecordFailures: boolean, state: ModuleResolutionState, considerPackageJson: boolean): Resolved | undefined { if (state.traceEnabled) { trace(state.host, Diagnostics.Loading_module_as_file_Slash_folder_candidate_module_location_0_target_file_type_1, candidate, Extensions[extensions]); } @@ -855,8 +855,7 @@ namespace ts { trace(state.host, Diagnostics.Found_package_json_at_0, packageJsonPath); } - const ts = extensions !== Extensions.JavaScript; - const file = tryReadPackageJsonFields(ts, packageJsonPath, candidate, state); + const file = tryReadPackageJsonFields(extensions !== Extensions.JavaScript, packageJsonPath, candidate, state); if (!file) { return undefined; } diff --git a/tests/baselines/reference/library-reference-12.trace.json b/tests/baselines/reference/library-reference-12.trace.json index 14e46647d6d..75b53d589ee 100644 --- a/tests/baselines/reference/library-reference-12.trace.json +++ b/tests/baselines/reference/library-reference-12.trace.json @@ -5,6 +5,7 @@ "Directory '/a/b/node_modules' does not exist, skipping all lookups in it.", "File '/a/node_modules/jquery.d.ts' does not exist.", "Found 'package.json' at '/a/node_modules/jquery/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' has 'types' field 'dist/jquery.d.ts' that references '/a/node_modules/jquery/dist/jquery.d.ts'.", "File '/a/node_modules/jquery/dist/jquery.d.ts' exist - use it as a name resolution result.", "Resolving real path for '/a/node_modules/jquery/dist/jquery.d.ts', result '/a/node_modules/jquery/dist/jquery.d.ts'", diff --git a/tests/baselines/reference/library-reference-2.trace.json b/tests/baselines/reference/library-reference-2.trace.json index 8837f2c9cb1..ef0936a35a2 100644 --- a/tests/baselines/reference/library-reference-2.trace.json +++ b/tests/baselines/reference/library-reference-2.trace.json @@ -2,6 +2,7 @@ "======== Resolving type reference directive 'jquery', containing file '/consumer.ts', root directory '/types'. ========", "Resolving with primary search path '/types'", "Found 'package.json' at '/types/jquery/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.", "File '/types/jquery/jquery.d.ts' exist - use it as a name resolution result.", "Resolving real path for '/types/jquery/jquery.d.ts', result '/types/jquery/jquery.d.ts'", @@ -9,6 +10,7 @@ "======== Resolving type reference directive 'jquery', containing file 'test/__inferred type names__.ts', root directory '/types'. ========", "Resolving with primary search path '/types'", "Found 'package.json' at '/types/jquery/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' has 'types' field 'jquery.d.ts' that references '/types/jquery/jquery.d.ts'.", "File '/types/jquery/jquery.d.ts' exist - use it as a name resolution result.", "Resolving real path for '/types/jquery/jquery.d.ts', result '/types/jquery/jquery.d.ts'", diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected.trace.json index c53a32ef97d..8c820e07e48 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' does not have a 'typings' field.", "'package.json' does not have a 'types' field.", "File '/node_modules/normalize.css/index.ts' does not exist.", "File '/node_modules/normalize.css/index.tsx' does not exist.", diff --git a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json index f007bc77d75..e550bb41eab 100644 --- a/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json +++ b/tests/baselines/reference/moduleResolutionWithExtensions_unexpected2.trace.json @@ -6,6 +6,7 @@ "File '/node_modules/foo.tsx' does not exist.", "File '/node_modules/foo.d.ts' does not exist.", "Found 'package.json' at '/node_modules/foo/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' has 'types' field 'foo.js' that references '/node_modules/foo/foo.js'.", "File '/node_modules/foo/foo.js' exist - use it as a name resolution result.", "File '/node_modules/foo/foo.js' has an unsupported extension, so skipping it.", diff --git a/tests/baselines/reference/packageJsonMain.trace.json b/tests/baselines/reference/packageJsonMain.trace.json index 9b71c672337..a167e8dfec6 100644 --- a/tests/baselines/reference/packageJsonMain.trace.json +++ b/tests/baselines/reference/packageJsonMain.trace.json @@ -6,6 +6,7 @@ "File '/node_modules/foo.tsx' does not exist.", "File '/node_modules/foo.d.ts' does not exist.", "Found 'package.json' at '/node_modules/foo/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' does not have a 'types' field.", "File '/node_modules/foo/index.ts' does not exist.", "File '/node_modules/foo/index.tsx' does not exist.", @@ -28,6 +29,7 @@ "File '/node_modules/bar.tsx' does not exist.", "File '/node_modules/bar.d.ts' does not exist.", "Found 'package.json' at '/node_modules/bar/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' does not have a 'types' field.", "File '/node_modules/bar/index.ts' does not exist.", "File '/node_modules/bar/index.tsx' does not exist.", @@ -48,6 +50,7 @@ "File '/node_modules/baz.tsx' does not exist.", "File '/node_modules/baz.d.ts' does not exist.", "Found 'package.json' at '/node_modules/baz/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' does not have a 'types' field.", "File '/node_modules/baz/index.ts' does not exist.", "File '/node_modules/baz/index.tsx' does not exist.", diff --git a/tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json b/tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json index 9d1f2f9bedf..53e1ad25605 100644 --- a/tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json +++ b/tests/baselines/reference/packageJsonMain_isNonRecursive.trace.json @@ -6,6 +6,7 @@ "File '/node_modules/foo.tsx' does not exist.", "File '/node_modules/foo.d.ts' does not exist.", "Found 'package.json' at '/node_modules/foo/package.json'.", + "'package.json' does not have a 'typings' field.", "'package.json' does not have a 'types' field.", "File '/node_modules/foo/index.ts' does not exist.", "File '/node_modules/foo/index.tsx' does not exist.",