Treat explicit imports from node_modules as external library imports (#16364)

Andy · web-flow · commit eef7d8bd3d97 · 2017-08-17T13:26:38.000-07:00
* Treat explicit imports from `node_modules` as external library imports

* Update baselines
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -1694,7 +1694,6 @@ namespace ts {
             if (ambientModule) {
                 return ambientModule;
             }
-            const isRelative = isExternalModuleNameRelative(moduleReference);
             const resolvedModule = getResolvedModule(getSourceFileOfNode(location), moduleReference);
             const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule);
             const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName);
@@ -1718,7 +1717,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
@@ -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
@@ -747,9 +747,10 @@ namespace ts {
                 return { value: resolvedValue && { resolved: resolvedValue, 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
@@ -3953,12 +3953,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,4 @@
+=== /src/index.ts ===
+import { y } from "../node_modules/foo";
+>y : any
+
diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt
@@ -13,9 +13,6 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i
       "exclude": ["node_modules/m2/**/*"]
     }
     
-==== relative.js (0 errors) ====
-    exports.relativeProp = true;
-    
 ==== index.js (0 errors) ====
     var m2 = require('m2');
     var rel = require('./relative');
@@ -51,4 +48,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;
     
diff --git 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"
     ]
 }
diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt
@@ -13,9 +13,6 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i
       "exclude": ["node_modules/m2/**/*"]
     }
     
-==== relative.js (0 errors) ====
-    exports.relativeProp = true;
-    
 ==== index.js (0 errors) ====
     var m2 = require('m2');
     var rel = require('./relative');
@@ -51,4 +48,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;
     
diff --git 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"
     ]
 }
diff --git a/tests/cases/compiler/moduleResolution_explicitNodeModulesImport.ts 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
@@ -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";

Original file line number	Diff line number	Diff line change
`@@ -1543,16 +1543,20 @@ namespace ts {`
`1543`	`1543`	`}`
`1544`	`1544`
`1545`	`1545`	`export function normalizePath(path: string): string {`
	`1546`	`+ return normalizePathAndParts(path).path;`
	`1547`	`+ }`
	`1548`	`+`
	`1549`	`+ export function normalizePathAndParts(path: string): { path: string, parts: string[] } {`
`1546`	`1550`	`path = normalizeSlashes(path);`
`1547`	`1551`	`const rootLength = getRootLength(path);`
`1548`	`1552`	`const root = path.substr(0, rootLength);`
`1549`		`- const normalized = getNormalizedParts(path, rootLength);`
`1550`		`- if (normalized.length) {`
`1551`		`- const joinedParts = root + normalized.join(directorySeparator);`
`1552`		`- return pathEndsWithDirectorySeparator(path) ? joinedParts + directorySeparator : joinedParts;`
	`1553`	`+ const parts = getNormalizedParts(path, rootLength);`
	`1554`	`+ if (parts.length) {`
	`1555`	`+ const joinedParts = root + parts.join(directorySeparator);`
	`1556`	`+ return { path: pathEndsWithDirectorySeparator(path) ? joinedParts + directorySeparator : joinedParts, parts };`
`1553`	`1557`	`}`
`1554`	`1558`	`else {`
`1555`		`- return root;`
	`1559`	`+ return { path: root, parts };`
`1556`	`1560`	`}`
`1557`	`1561`	`}`
`1558`	`1562`
Original file line number	Diff line number	Diff line change
`@@ -747,9 +747,10 @@ namespace ts {`
`747`	`747`	`return { value: resolvedValue && { resolved: resolvedValue, isExternalLibraryImport: true } };`
`748`	`748`	`}`
`749`	`749`	`else {`
`750`		`- const candidate = normalizePath(combinePaths(containingDirectory, moduleName));`
	`750`	`+ const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName));`
`751`	`751`	`const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /onlyRecordFailures/ false, state, /considerPackageJson/ true);`
`752`		`- return resolved && toSearchResult({ resolved, isExternalLibraryImport: false });`
	`752`	`+ // Treat explicit "node_modules" import as an external library import.`
	`753`	`+ return resolved && toSearchResult({ resolved, isExternalLibraryImport: contains(parts, "node_modules") });`
`753`	`754`	`}`
`754`	`755`	`}`
`755`	`756`	`}`