Skip to content

Commit 56370d0

Browse files
sheetalkamatsheetalkamat
committed
If any of the file path option is from node_modules folder, consider only paths in node_modules folder
1 parent c795052 commit 56370d0

7 files changed

+90
-27
lines changed

‎src/compiler/moduleSpecifiers.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -172,24 +172,21 @@ namespace ts.moduleSpecifiers {
172172
importingFileName: string,
173173
importedFileName: string,
174174
host: ModuleSpecifierResolutionHost,
175-
preferSymlinks: boolean,
176175
cb: (fileName: string) => T | undefined
177176
): T | undefined {
178177
const getCanonicalFileName = hostGetCanonicalFileName(host);
179178
const cwd = host.getCurrentDirectory();
180-
const redirects = host.redirectTargetsMap.get(toPath(importedFileName, cwd, getCanonicalFileName));
181-
const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName];
179+
const redirects = host.redirectTargetsMap.get(toPath(importedFileName, cwd, getCanonicalFileName)) || emptyArray;
180+
const importedFileNames = [importedFileName, ...redirects];
182181
const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd));
183-
if (!preferSymlinks) {
184182
const result = forEach(targets, cb);
185183
if (result) return result;
186-
}
187184
const links = host.getProbableSymlinks
188185
? host.getProbableSymlinks(host.getSourceFiles())
189186
: discoverProbableSymlinks(host.getSourceFiles(), getCanonicalFileName, cwd);
190187

191188
const compareStrings = (!host.useCaseSensitiveFileNames || host.useCaseSensitiveFileNames()) ? compareStringsCaseSensitive : compareStringsCaseInsensitive;
192-
const result = forEachEntry(links, (resolved, path) => {
189+
return forEachEntry(links, (resolved, path) => {
193190
if (startsWithDirectory(importingFileName, resolved, getCanonicalFileName)) {
194191
return undefined; // Don't want to a package to globally import from itself
195192
}
@@ -204,8 +201,6 @@ namespace ts.moduleSpecifiers {
204201
if (result) return result;
205202
}
206203
});
207-
return result ||
208-
(preferSymlinks ? forEach(targets, cb) : undefined);
209204
}
210205

211206
/**
@@ -216,14 +211,15 @@ namespace ts.moduleSpecifiers {
216211
const cwd = host.getCurrentDirectory();
217212
const getCanonicalFileName = hostGetCanonicalFileName(host);
218213
const allFileNames = createMap<string>();
214+
let importedFileFromNodeModules = false;
219215
forEachFileNameOfModule(
220216
importingFileName,
221217
importedFileName,
222218
host,
223-
/*preferSymlinks*/ true,
224219
path => {
225220
// dont return value, so we collect everything
226221
allFileNames.set(path, getCanonicalFileName(path));
222+
importedFileFromNodeModules = importedFileFromNodeModules || pathContainsNodeModules(path);
227223
}
228224
);
229225

@@ -237,7 +233,10 @@ namespace ts.moduleSpecifiers {
237233
let pathsInDirectory: string[] | undefined;
238234
allFileNames.forEach((canonicalFileName, fileName) => {
239235
if (startsWith(canonicalFileName, directoryStart)) {
240-
(pathsInDirectory || (pathsInDirectory = [])).push(fileName);
236+
// If the importedFile is from node modules, use only paths in node_modules folder as option
237+
if (!importedFileFromNodeModules || pathContainsNodeModules(fileName)) {
238+
(pathsInDirectory || (pathsInDirectory = [])).push(fileName);
239+
}
241240
allFileNames.delete(fileName);
242241
}
243242
});

‎src/compiler/transformers/declarations.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ namespace ts {
374374

375375
// omit references to files from node_modules (npm may disambiguate module
376376
// references when installing this package, making the path is unreliable).
377-
if (startsWith(fileName, "node_modules/") || fileName.indexOf("/node_modules/") !== -1) {
377+
if (startsWith(fileName, "node_modules/") || pathContainsNodeModules(fileName)) {
378378
return;
379379
}
380380

‎src/services/codefixes/importFixes.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,6 @@ namespace ts.codefix {
837837
from.fileName,
838838
to.fileName,
839839
moduleSpecifierResolutionHost,
840-
/*preferSymlinks*/ false,
841840
toPath => {
842841
const toFile = program.getSourceFile(toPath);
843842
// Determine to import using toPath only if toPath is what we were looking at

‎tests/baselines/reference/declarationEmitReexportedSymlinkReference3.errors.txt

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
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.
2+
3+
4+
==== tests/cases/compiler/monorepo/pkg3/tsconfig.json (0 errors) ====
5+
{
6+
"compilerOptions": {
7+
"outDir": "dist",
8+
"rootDir": "src",
9+
"target": "es5",
10+
"module": "commonjs",
11+
"strict": true,
12+
"esModuleInterop": true,
13+
"declaration": true
14+
}
15+
}
16+
17+
==== tests/cases/compiler/monorepo/pkg1/dist/index.d.ts (0 errors) ====
18+
export * from './types';
19+
==== tests/cases/compiler/monorepo/pkg1/dist/types.d.ts (0 errors) ====
20+
export declare type A = {
21+
id: string;
22+
};
23+
export declare type B = {
24+
id: number;
25+
};
26+
export declare type IdType = A | B;
27+
export declare class MetadataAccessor<T, D extends IdType = IdType> {
28+
readonly key: string;
29+
private constructor();
30+
toString(): string;
31+
static create<T, D extends IdType = IdType>(key: string): MetadataAccessor<T, D>;
32+
}
33+
==== tests/cases/compiler/monorepo/pkg1/package.json (0 errors) ====
34+
{
35+
"name": "@raymondfeng/pkg1",
36+
"version": "1.0.0",
37+
"description": "",
38+
"main": "dist/index.js",
39+
"typings": "dist/index.d.ts"
40+
}
41+
==== tests/cases/compiler/monorepo/pkg2/dist/index.d.ts (0 errors) ====
42+
export * from './types';
43+
==== tests/cases/compiler/monorepo/pkg2/dist/types.d.ts (0 errors) ====
44+
export {MetadataAccessor} from '@raymondfeng/pkg1';
45+
==== tests/cases/compiler/monorepo/pkg2/package.json (0 errors) ====
46+
{
47+
"name": "@raymondfeng/pkg2",
48+
"version": "1.0.0",
49+
"description": "",
50+
"main": "dist/index.js",
51+
"typings": "dist/index.d.ts"
52+
}
53+
==== tests/cases/compiler/monorepo/pkg3/src/index.ts (0 errors) ====
54+
export * from './keys';
55+
==== tests/cases/compiler/monorepo/pkg3/src/keys.ts (1 errors) ====
56+
import {MetadataAccessor} from "@raymondfeng/pkg2";
57+
58+
export const ADMIN = MetadataAccessor.create<boolean>('1');
59+
~~~~~
60+
!!! 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/baselines/reference/declarationEmitReexportedSymlinkReference3.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,5 @@ Object.defineProperty(exports, "__esModule", { value: true });
6565
__exportStar(require("./keys"), exports);
6666

6767

68-
//// [keys.d.ts]
69-
import { MetadataAccessor } from "@raymondfeng/pkg2";
70-
export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
7168
//// [index.d.ts]
7269
export * from './keys';

‎tests/baselines/reference/tsc/declarationEmit/when-pkg-references-sibling-package-through-indirect-symlink-moduleCaseChange.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ var pkg2_1 = require("@raymondfeng/pkg2");
6262
exports.ADMIN = pkg2_1.MetadataAccessor.create('1');
6363

6464

65-
//// [/user/username/projects/myproject/pkg3/dist/keys.d.ts]
66-
import { MetadataAccessor } from "@raymondfeng/pkg2";
67-
export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
68-
69-
7065
//// [/user/username/projects/myproject/pkg3/dist/index.js]
7166
"use strict";
7267
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
@@ -89,6 +84,12 @@ export * from './keys';
8984

9085

9186
Output::
87+
pkg3/src/keys.ts:2: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.
88+
89+
2 export const ADMIN = MetadataAccessor.create<boolean>('1');
90+
   ~~~~~
91+
92+
9293
/a/lib/lib.d.ts
9394

9495
/user/username/projects/myProject/pkg1/dist/types.d.ts
@@ -103,6 +104,9 @@ Output::
103104

104105
/user/username/projects/myproject/pkg3/src/index.ts
105106

107+
108+
Found 1 error.
109+
106110

107111

108112
Program root files: ["/user/username/projects/myproject/pkg3/src/index.ts","/user/username/projects/myproject/pkg3/src/keys.ts"]
@@ -122,4 +126,4 @@ FsWatches::
122126

123127
FsWatchesRecursive::
124128

125-
exitCode:: ExitStatus.Success
129+
exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped

‎tests/baselines/reference/tsc/declarationEmit/when-pkg-references-sibling-package-through-indirect-symlink.js

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,6 @@ var pkg2_1 = require("@raymondfeng/pkg2");
6262
exports.ADMIN = pkg2_1.MetadataAccessor.create('1');
6363

6464

65-
//// [/user/username/projects/myproject/pkg3/dist/keys.d.ts]
66-
import { MetadataAccessor } from "@raymondfeng/pkg2";
67-
export declare const ADMIN: MetadataAccessor<boolean, import("../../pkg1/dist").IdType>;
68-
69-
7065
//// [/user/username/projects/myproject/pkg3/dist/index.js]
7166
"use strict";
7267
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
@@ -89,6 +84,12 @@ export * from './keys';
8984

9085

9186
Output::
87+
pkg3/src/keys.ts:2: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.
88+
89+
2 export const ADMIN = MetadataAccessor.create<boolean>('1');
90+
   ~~~~~
91+
92+
9293
/a/lib/lib.d.ts
9394

9495
/user/username/projects/myproject/pkg1/dist/types.d.ts
@@ -103,6 +104,9 @@ Output::
103104

104105
/user/username/projects/myproject/pkg3/src/index.ts
105106

107+
108+
Found 1 error.
109+
106110

107111

108112
Program root files: ["/user/username/projects/myproject/pkg3/src/index.ts","/user/username/projects/myproject/pkg3/src/keys.ts"]
@@ -122,4 +126,4 @@ FsWatches::
122126

123127
FsWatchesRecursive::
124128

125-
exitCode:: ExitStatus.Success
129+
exitCode:: ExitStatus.DiagnosticsPresent_OutputsSkipped

0 commit comments

Comments
 (0)