If any of the file path option is from node_modules folder, consider …

…only paths in node_modules folder
microsoft · Mar 17, 2020 · 2f1a3ab · 2f1a3ab
1 parent c795052
commit 2f1a3ab
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 19 deletions.
diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts
@@ -177,8 +177,8 @@ namespace ts.moduleSpecifiers {
     ): T | undefined {
         const getCanonicalFileName = hostGetCanonicalFileName(host);
         const cwd = host.getCurrentDirectory();
-        const redirects = host.redirectTargetsMap.get(toPath(importedFileName, cwd, getCanonicalFileName));
-        const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName];
+        const redirects = host.redirectTargetsMap.get(toPath(importedFileName, cwd, getCanonicalFileName)) || emptyArray;
+        const importedFileNames = [importedFileName, ...redirects];
         const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd));
         if (!preferSymlinks) {
             const result = forEach(targets, cb);
@@ -216,6 +216,7 @@ namespace ts.moduleSpecifiers {
         const cwd = host.getCurrentDirectory();
         const getCanonicalFileName = hostGetCanonicalFileName(host);
         const allFileNames = createMap<string>();
+        let importedFileFromNodeModules = false;
         forEachFileNameOfModule(
             importingFileName,
             importedFileName,
@@ -224,6 +225,7 @@ namespace ts.moduleSpecifiers {
             path => {
                 // dont return value, so we collect everything
                 allFileNames.set(path, getCanonicalFileName(path));
+                importedFileFromNodeModules = importedFileFromNodeModules || pathContainsNodeModules(path);
             }
         );
 
@@ -237,7 +239,10 @@ namespace ts.moduleSpecifiers {
             let pathsInDirectory: string[] | undefined;
             allFileNames.forEach((canonicalFileName, fileName) => {
                 if (startsWith(canonicalFileName, directoryStart)) {
-                    (pathsInDirectory || (pathsInDirectory = [])).push(fileName);
+                    // If the importedFile is from node modules, use only paths in node_modules folder as option
+                    if (!importedFileFromNodeModules || pathContainsNodeModules(fileName)) {
+                        (pathsInDirectory || (pathsInDirectory = [])).push(fileName);
+                    }
                     allFileNames.delete(fileName);
                 }
             });

diff --git a/src/compiler/transformers/declarations.ts b/src/compiler/transformers/declarations.ts
@@ -374,7 +374,7 @@ namespace ts {
 
                         // omit references to files from node_modules (npm may disambiguate module
                         // references when installing this package, making the path is unreliable).
-                        if (startsWith(fileName, "node_modules/") || fileName.indexOf("/node_modules/") !== -1) {
+                        if (startsWith(fileName, "node_modules/") || pathContainsNodeModules(fileName)) {
                             return;
                         }
 

diff --git a/tests/baselines/reference/declarationEmitReexportedSymlinkReference3.errors.txt b/tests/baselines/reference/declarationEmitReexportedSymlinkReference3.errors.txt
@@ -0,0 +1,60 @@
+tests/cases/compiler/monorepo/pkg3/src/keys.ts(3,14): error TS2742: The inferred type of 'ADMIN' cannot be named without a reference to '../../pkg2/node_modules/@raymondfeng/pkg1/dist'. This is likely not portable. A type annotation is necessary.
+
+
+==== tests/cases/compiler/monorepo/pkg3/tsconfig.json (0 errors) ====
+    {
+        "compilerOptions": {
+          "outDir": "dist",
+          "rootDir": "src",
+          "target": "es5",
+          "module": "commonjs",
+          "strict": true,
+          "esModuleInterop": true,
+          "declaration": true
+        }
+    }
+
+==== tests/cases/compiler/monorepo/pkg1/dist/index.d.ts (0 errors) ====
+    export * from './types';
+==== tests/cases/compiler/monorepo/pkg1/dist/types.d.ts (0 errors) ====
+    export declare type A = {
+        id: string;
+    };
+    export declare type B = {
+        id: number;
+    };
+    export declare type IdType = A | B;
+    export declare class MetadataAccessor<T, D extends IdType = IdType> {
+        readonly key: string;
+        private constructor();
+        toString(): string;
+        static create<T, D extends IdType = IdType>(key: string): MetadataAccessor<T, D>;
+    }
+==== tests/cases/compiler/monorepo/pkg1/package.json (0 errors) ====
+    {
+        "name": "@raymondfeng/pkg1",
+        "version": "1.0.0",
+        "description": "",
+        "main": "dist/index.js",
+        "typings": "dist/index.d.ts"
+    }
+==== tests/cases/compiler/monorepo/pkg2/dist/index.d.ts (0 errors) ====
+    export * from './types';
+==== tests/cases/compiler/monorepo/pkg2/dist/types.d.ts (0 errors) ====
+    export {MetadataAccessor} from '@raymondfeng/pkg1';
+==== tests/cases/compiler/monorepo/pkg2/package.json (0 errors) ====
+    {
+        "name": "@raymondfeng/pkg2",
+        "version": "1.0.0",
+        "description": "",
+        "main": "dist/index.js",
+        "typings": "dist/index.d.ts"
+    }
+==== tests/cases/compiler/monorepo/pkg3/src/index.ts (0 errors) ====
+    export * from './keys';
+==== tests/cases/compiler/monorepo/pkg3/src/keys.ts (1 errors) ====
+    import {MetadataAccessor} from "@raymondfeng/pkg2";
+
+    export const ADMIN = MetadataAccessor.create<boolean>('1');
+                 ~~~~~
+!!! error TS2742: The inferred type of 'ADMIN' cannot be named without a reference to '../../pkg2/node_modules/@raymondfeng/pkg1/dist'. This is likely not portable. A type annotation is necessary.
diff --git a/tests/baselines/reference/declarationEmitReexportedSymlinkReference3.js b/tests/baselines/reference/declarationEmitReexportedSymlinkReference3.js
@@ -65,8 +65,5 @@ Object.defineProperty(exports, "__esModule", { value: true });
 __exportStar(require("./keys"), exports);
 
 
-//// [keys.d.ts]
-import { MetadataAccessor } from "@raymondfeng/pkg2";
-export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
 //// [index.d.ts]
 export * from './keys';
diff --git a/...tionEmit/when-pkg-references-sibling-package-through-indirect-symlink-moduleCaseChange.js b/...tionEmit/when-pkg-references-sibling-package-through-indirect-symlink-moduleCaseChange.js
@@ -62,11 +62,6 @@ var pkg2_1 = require("@raymondfeng/pkg2");
 exports.ADMIN = pkg2_1.MetadataAccessor.create('1');
 
 
-//// [/user/username/projects/myproject/pkg3/dist/keys.d.ts]
-import { MetadataAccessor } from "@raymondfeng/pkg2";
-export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
-
-
 //// [/user/username/projects/myproject/pkg3/dist/index.js]
 "use strict";
 var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
@@ -89,6 +84,12 @@ export * from './keys';
 
 
 Output::
+[96mpkg3/src/keys.ts[0m:[93m2[0m:[93m14[0m - [91merror[0m[90m TS2742: [0mThe inferred type of 'ADMIN' cannot be named without a reference to '../../pkg2/node_modules/@raymondfeng/pkg1/dist'. This is likely not portable. A type annotation is necessary.
+
+[7m2[0m export const ADMIN = MetadataAccessor.create<boolean>('1');
+[7m [0m [91m             ~~~~~[0m
+
+
 /a/lib/lib.d.ts
 
 /user/username/projects/myProject/pkg1/dist/types.d.ts
@@ -103,6 +104,9 @@ Output::
 
 /user/username/projects/myproject/pkg3/src/index.ts
 
+
+Found 1 error.
+
 
 
 Program root files: ["/user/username/projects/myproject/pkg3/src/index.ts","/user/username/projects/myproject/pkg3/src/keys.ts"]
@@ -122,4 +126,4 @@ FsWatches::
 
 FsWatchesRecursive::
 
-exitCode:: ExitStatus.Success
+exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped
diff --git a/...rence/tsc/declarationEmit/when-pkg-references-sibling-package-through-indirect-symlink.js b/...rence/tsc/declarationEmit/when-pkg-references-sibling-package-through-indirect-symlink.js
@@ -62,11 +62,6 @@ var pkg2_1 = require("@raymondfeng/pkg2");
 exports.ADMIN = pkg2_1.MetadataAccessor.create('1');
 
 
-//// [/user/username/projects/myproject/pkg3/dist/keys.d.ts]
-import { MetadataAccessor } from "@raymondfeng/pkg2";
-export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
-
-
 //// [/user/username/projects/myproject/pkg3/dist/index.js]
 "use strict";
 var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
@@ -89,6 +84,12 @@ export * from './keys';
 
 
 Output::
+[96mpkg3/src/keys.ts[0m:[93m2[0m:[93m14[0m - [91merror[0m[90m TS2742: [0mThe inferred type of 'ADMIN' cannot be named without a reference to '../../pkg2/node_modules/@raymondfeng/pkg1/dist'. This is likely not portable. A type annotation is necessary.
+
+[7m2[0m export const ADMIN = MetadataAccessor.create<boolean>('1');
+[7m [0m [91m             ~~~~~[0m
+
+
 /a/lib/lib.d.ts
 
 /user/username/projects/myproject/pkg1/dist/types.d.ts
@@ -103,6 +104,9 @@ Output::
 
 /user/username/projects/myproject/pkg3/src/index.ts
 
+
+Found 1 error.
+
 
 
 Program root files: ["/user/username/projects/myproject/pkg3/src/index.ts","/user/username/projects/myproject/pkg3/src/keys.ts"]
@@ -122,4 +126,4 @@ FsWatches::
 
 FsWatchesRecursive::
 
-exitCode:: ExitStatus.Success
+exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped