From 62cae8afc9ef902c4c72071808b651eceb64caea Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 8 Jun 2017 11:13:27 -0700 Subject: [PATCH 1/2] Treat explicit imports from `node_modules` as external library imports --- src/compiler/checker.ts | 3 +-- src/compiler/core.ts | 14 +++++++++----- src/compiler/moduleNameResolver.ts | 5 +++-- src/compiler/types.ts | 7 +------ .../moduleResolution_explicitNodeModulesImport.js | 12 ++++++++++++ ...uleResolution_explicitNodeModulesImport.symbols | 9 +++++++++ ...oduleResolution_explicitNodeModulesImport.types | 12 ++++++++++++ ...lution_explicitNodeModulesImport_implicitAny.js | 12 ++++++++++++ ...n_explicitNodeModulesImport_implicitAny.symbols | 4 ++++ ...ion_explicitNodeModulesImport_implicitAny.types | 4 ++++ .../moduleResolution_explicitNodeModulesImport.ts | 9 +++++++++ ...lution_explicitNodeModulesImport_implicitAny.ts | 9 +++++++++ 12 files changed, 85 insertions(+), 15 deletions(-) create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport.js create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport.symbols create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport.types create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.js create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.symbols create mode 100644 tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.types create mode 100644 tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts create mode 100644 tests/cases/compiler/moduleResolution_explicitNodeModulesImport_implicitAny.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 266fbe573beb9..e01b71c911ab1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1668,7 +1668,6 @@ namespace ts { if (ambientModule) { return ambientModule; } - const isRelative = isExternalModuleNameRelative(moduleName); const resolvedModule = getResolvedModule(getSourceFileOfNode(location), moduleReference); const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule); const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName); @@ -1692,7 +1691,7 @@ namespace ts { } // May be an untyped module. If so, ignore resolutionDiagnostic. - if (!isRelative && resolvedModule && !extensionIsTypeScript(resolvedModule.extension)) { + if (resolvedModule && resolvedModule.isExternalLibraryImport && !extensionIsTypeScript(resolvedModule.extension)) { if (isForAugmentation) { const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented; error(errorNode, diag, moduleReference, resolvedModule.resolvedFileName); diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 6ca99c2d08696..1c49c02fe2c0c 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1543,16 +1543,20 @@ namespace ts { } export function normalizePath(path: string): string { + return normalizePathAndParts(path).path; + } + + export function normalizePathAndParts(path: string): { path: string, parts: string[] } { path = normalizeSlashes(path); const rootLength = getRootLength(path); const root = path.substr(0, rootLength); - const normalized = getNormalizedParts(path, rootLength); - if (normalized.length) { - const joinedParts = root + normalized.join(directorySeparator); - return pathEndsWithDirectorySeparator(path) ? joinedParts + directorySeparator : joinedParts; + const parts = getNormalizedParts(path, rootLength); + if (parts.length) { + const joinedParts = root + parts.join(directorySeparator); + return { path: pathEndsWithDirectorySeparator(path) ? joinedParts + directorySeparator : joinedParts, parts }; } else { - return root; + return { path: root, parts }; } } diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 4c6616c36d46a..b2f796261ff48 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -724,9 +724,10 @@ namespace ts { return resolved && { value: resolved.value && { resolved: { path: realpath(resolved.value.path, host, traceEnabled), extension: resolved.value.extension }, isExternalLibraryImport: true } }; } else { - const candidate = normalizePath(combinePaths(containingDirectory, moduleName)); + const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName)); const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/ true); - return resolved && toSearchResult({ resolved, isExternalLibraryImport: false }); + // Treat explicit "node_modules" import as an external library import. + return resolved && toSearchResult({ resolved, isExternalLibraryImport: contains(parts, "node_modules") }); } } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8833fbb9d08f8..91fdc2578f3f4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3857,12 +3857,7 @@ namespace ts { export interface ResolvedModule { /** Path of the file the module was resolved to. */ resolvedFileName: string; - /** - * Denotes if 'resolvedFileName' is isExternalLibraryImport and thus should be a proper external module: - * - be a .d.ts file - * - use top level imports\exports - * - don't use tripleslash references - */ + /** True if `resolvedFileName` comes from `node_modules`. */ isExternalLibraryImport?: boolean; } diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.js b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.js new file mode 100644 index 0000000000000..93a083da53af6 --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.js @@ -0,0 +1,12 @@ +//// [tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts] //// + +//// [index.js] +exports.x = 0; + +//// [index.ts] +import { x } from "../node_modules/foo"; + + +//// [index.js] +"use strict"; +exports.__esModule = true; diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.symbols b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.symbols new file mode 100644 index 0000000000000..b635ba2c7c7c6 --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.symbols @@ -0,0 +1,9 @@ +=== /src/index.ts === +import { x } from "../node_modules/foo"; +>x : Symbol(x, Decl(index.ts, 0, 8)) + +=== /node_modules/foo/index.js === +exports.x = 0; +>exports : Symbol(x, Decl(index.js, 0, 0)) +>x : Symbol(x, Decl(index.js, 0, 0)) + diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.types b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.types new file mode 100644 index 0000000000000..ca7541d21404a --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport.types @@ -0,0 +1,12 @@ +=== /src/index.ts === +import { x } from "../node_modules/foo"; +>x : number + +=== /node_modules/foo/index.js === +exports.x = 0; +>exports.x = 0 : 0 +>exports.x : any +>exports : any +>x : any +>0 : 0 + diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.js b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.js new file mode 100644 index 0000000000000..1b5b469570de2 --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.js @@ -0,0 +1,12 @@ +//// [tests/cases/compiler/moduleResolution_explicitNodeModulesImport_implicitAny.ts] //// + +//// [index.js] +exports.x = 0; + +//// [index.ts] +import { y } from "../node_modules/foo"; + + +//// [index.js] +"use strict"; +exports.__esModule = true; diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.symbols b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.symbols new file mode 100644 index 0000000000000..e320d4d18e151 --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.symbols @@ -0,0 +1,4 @@ +=== /src/index.ts === +import { y } from "../node_modules/foo"; +>y : Symbol(y, Decl(index.ts, 0, 8)) + diff --git a/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.types b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.types new file mode 100644 index 0000000000000..a22e0be7a663c --- /dev/null +++ b/tests/baselines/reference/moduleResolution_explicitNodeModulesImport_implicitAny.types @@ -0,0 +1,4 @@ +=== /src/index.ts === +import { y } from "../node_modules/foo"; +>y : any + diff --git a/tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts b/tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts new file mode 100644 index 0000000000000..76c1d14197628 --- /dev/null +++ b/tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts @@ -0,0 +1,9 @@ +// @allowJs: true +// @noImplicitReferences: true +// @maxNodeModuleJsDepth: 1 + +// @Filename: /node_modules/foo/index.js +exports.x = 0; + +// @Filename: /src/index.ts +import { x } from "../node_modules/foo"; diff --git a/tests/cases/compiler/moduleResolution_explicitNodeModulesImport_implicitAny.ts b/tests/cases/compiler/moduleResolution_explicitNodeModulesImport_implicitAny.ts new file mode 100644 index 0000000000000..97320be01377d --- /dev/null +++ b/tests/cases/compiler/moduleResolution_explicitNodeModulesImport_implicitAny.ts @@ -0,0 +1,9 @@ +// @allowJs: true +// @noImplicitReferences: true +// @maxNodeModuleJsDepth: 0 + +// @Filename: /node_modules/foo/index.js +exports.x = 0; + +// @Filename: /src/index.ts +import { y } from "../node_modules/foo"; From 0325322e0967412be7e10517bdbd825f996fe67e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 9 Jun 2017 07:20:39 -0700 Subject: [PATCH 2/2] Update baselines --- .../amd/nodeModulesMaxDepthExceeded.errors.txt | 6 +++--- .../amd/nodeModulesMaxDepthExceeded.json | 8 ++++---- .../node/nodeModulesMaxDepthExceeded.errors.txt | 6 +++--- .../node/nodeModulesMaxDepthExceeded.json | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt index 5b5199ad7ae8b..dc0b88140964a 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt @@ -2,9 +2,6 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type '"10"' is not assignable to ty maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. -==== relative.js (0 errors) ==== - exports.relativeProp = true; - ==== index.js (0 errors) ==== var m2 = require('m2'); var rel = require('./relative'); @@ -40,4 +37,7 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i "b": "hello, world", "person": m3.person }; + +==== relative.js (0 errors) ==== + exports.relativeProp = true; \ No newline at end of file diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json index 9efa0e936acf5..4a9b23468547a 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json @@ -7,14 +7,14 @@ "project": "maxDepthExceeded", "resolvedInputFiles": [ "lib.d.ts", - "maxDepthExceeded/node_modules/m1/relative.js", "maxDepthExceeded/node_modules/m1/index.js", "maxDepthExceeded/root.ts", - "maxDepthExceeded/node_modules/m2/entry.js" + "maxDepthExceeded/node_modules/m2/entry.js", + "maxDepthExceeded/node_modules/m1/relative.js" ], "emittedFiles": [ - "maxDepthExceeded/built/node_modules/m1/relative.js", "maxDepthExceeded/built/node_modules/m1/index.js", - "maxDepthExceeded/built/root.js" + "maxDepthExceeded/built/root.js", + "maxDepthExceeded/built/node_modules/m1/relative.js" ] } \ No newline at end of file diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt index 5b5199ad7ae8b..dc0b88140964a 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt @@ -2,9 +2,6 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type '"10"' is not assignable to ty maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. -==== relative.js (0 errors) ==== - exports.relativeProp = true; - ==== index.js (0 errors) ==== var m2 = require('m2'); var rel = require('./relative'); @@ -40,4 +37,7 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i "b": "hello, world", "person": m3.person }; + +==== relative.js (0 errors) ==== + exports.relativeProp = true; \ No newline at end of file diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json index 9efa0e936acf5..4a9b23468547a 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json @@ -7,14 +7,14 @@ "project": "maxDepthExceeded", "resolvedInputFiles": [ "lib.d.ts", - "maxDepthExceeded/node_modules/m1/relative.js", "maxDepthExceeded/node_modules/m1/index.js", "maxDepthExceeded/root.ts", - "maxDepthExceeded/node_modules/m2/entry.js" + "maxDepthExceeded/node_modules/m2/entry.js", + "maxDepthExceeded/node_modules/m1/relative.js" ], "emittedFiles": [ - "maxDepthExceeded/built/node_modules/m1/relative.js", "maxDepthExceeded/built/node_modules/m1/index.js", - "maxDepthExceeded/built/root.js" + "maxDepthExceeded/built/root.js", + "maxDepthExceeded/built/node_modules/m1/relative.js" ] } \ No newline at end of file