Skip to content

Commit b6727ea

Browse files
author
Andy Hanson
committed
Only resolve symlinks in node_modules 
1 parent 2eca0af commit b6727ea

35 files changed

Lines changed: 423 additions & 69 deletions

File tree

‎src/compiler/moduleNameResolver.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ namespace ts {
548548
const result = tryResolve(Extensions.TypeScript) || tryResolve(Extensions.JavaScript);
549549
if (result) {
550550
const { resolved, isExternalLibraryImport } = result;
551-
return createResolvedModuleWithFailedLookupLocations(resolved && resolvedWithRealpath(resolved, host, traceEnabled), isExternalLibraryImport, failedLookupLocations);
551+
return createResolvedModuleWithFailedLookupLocations(resolved, isExternalLibraryImport, failedLookupLocations);
552552
}
553553
return { resolvedModule: undefined, failedLookupLocations };
554554

@@ -563,7 +563,8 @@ namespace ts {
563563
trace(host, Diagnostics.Loading_module_0_from_node_modules_folder, moduleName);
564564
}
565565
const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state, /*checkOneLevel*/ false);
566-
return resolved && { resolved, isExternalLibraryImport: true };
566+
// For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files.
567+
return resolved && { resolved: resolvedWithRealpath(resolved, host, traceEnabled), isExternalLibraryImport: true };
567568
}
568569
else {
569570
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));

‎src/harness/harness.ts‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -958,28 +958,38 @@ namespace Harness {
958958
// Local get canonical file name function, that depends on passed in parameter for useCaseSensitiveFileNames
959959
const getCanonicalFileName = ts.createGetCanonicalFileName(useCaseSensitiveFileNames);
960960

961-
const realPathMap: ts.FileMap<string> = ts.createFileMap<string>();
962-
const fileMap: ts.FileMap<() => ts.SourceFile> = ts.createFileMap<() => ts.SourceFile>();
961+
/** Maps a symlink name to a realpath. Used only for exposing `realpath`. */
962+
const realPathMap = ts.createFileMap<string>();
963+
/**
964+
* Maps a file name to a source file.
965+
* This will have a different SourceFile for every symlink pointing to that file;
966+
* if the program resolves realpaths then symlink entries will be ignored.
967+
*/
968+
const fileMap = ts.createFileMap<ts.SourceFile>();
963969
for (const file of inputFiles) {
964970
if (file.content !== undefined) {
965971
const fileName = ts.normalizePath(file.unitName);
966972
const path = ts.toPath(file.unitName, currentDirectory, getCanonicalFileName);
967973
if (file.fileOptions && file.fileOptions["symlink"]) {
968-
const link = file.fileOptions["symlink"];
969-
const linkPath = ts.toPath(link, currentDirectory, getCanonicalFileName);
970-
realPathMap.set(linkPath, fileName);
971-
fileMap.set(path, (): ts.SourceFile => { throw new Error("Symlinks should always be resolved to a realpath first"); });
974+
const links = file.fileOptions["symlink"].split(",");
975+
for (const link of links) {
976+
const linkPath = ts.toPath(link, currentDirectory, getCanonicalFileName);
977+
realPathMap.set(linkPath, fileName);
978+
// Create a different SourceFile for every symlink.
979+
const sourceFile = createSourceFileAndAssertInvariants(linkPath, file.content, scriptTarget);
980+
fileMap.set(linkPath, sourceFile);
981+
}
972982
}
973983
const sourceFile = createSourceFileAndAssertInvariants(fileName, file.content, scriptTarget);
974-
fileMap.set(path, () => sourceFile);
984+
fileMap.set(path, sourceFile);
975985
}
976986
}
977987

978988
function getSourceFile(fileName: string) {
979989
fileName = ts.normalizePath(fileName);
980-
const path = ts.toPath(fileName, currentDirectory, getCanonicalFileName);
981-
if (fileMap.contains(path)) {
982-
return fileMap.get(path)();
990+
const fromFileMap = fileMap.get(toPath(fileName));
991+
if (fromFileMap) {
992+
return fromFileMap;
983993
}
984994
else if (fileName === fourslashFileName) {
985995
const tsFn = "tests/cases/fourslash/" + fourslashFileName;
@@ -998,6 +1008,9 @@ namespace Harness {
9981008
newLineKind === ts.NewLineKind.LineFeed ? lineFeed :
9991009
Harness.IO.newLine();
10001010

1011+
function toPath(fileName: string): ts.Path {
1012+
return ts.toPath(fileName, currentDirectory, getCanonicalFileName);
1013+
}
10011014

10021015
return {
10031016
getCurrentDirectory: () => currentDirectory,
@@ -1007,24 +1020,19 @@ namespace Harness {
10071020
getCanonicalFileName,
10081021
useCaseSensitiveFileNames: () => useCaseSensitiveFileNames,
10091022
getNewLine: () => newLine,
1010-
fileExists: fileName => {
1011-
const path = ts.toPath(fileName, currentDirectory, getCanonicalFileName);
1012-
return fileMap.contains(path) || (realPathMap && realPathMap.contains(path));
1013-
},
1014-
readFile: (fileName: string): string => {
1015-
return fileMap.get(ts.toPath(fileName, currentDirectory, getCanonicalFileName))().getText();
1023+
fileExists: fileName => fileMap.contains(toPath(fileName)),
1024+
readFile: (fileName: string): string => fileMap.get(toPath(fileName)).getText(),
1025+
realpath: (fileName: string): ts.Path => {
1026+
const path = toPath(fileName);
1027+
return (realPathMap.get(path) as ts.Path) || path;
10161028
},
1017-
realpath: realPathMap && ((f: string) => {
1018-
const path = ts.toPath(f, currentDirectory, getCanonicalFileName);
1019-
return realPathMap.get(path) || path;
1020-
}),
10211029
directoryExists: dir => {
10221030
let path = ts.toPath(dir, currentDirectory, getCanonicalFileName);
10231031
// Strip trailing /, which may exist if the path is a drive root
10241032
if (path[path.length - 1] === "/") {
10251033
path = <ts.Path>path.substr(0, path.length - 1);
10261034
}
1027-
return mapHasFileInDirectory(path, fileMap) || mapHasFileInDirectory(path, realPathMap);
1035+
return mapHasFileInDirectory(path, fileMap);
10281036
},
10291037
getDirectories: d => {
10301038
const path = ts.toPath(d, currentDirectory, getCanonicalFileName);

‎tests/baselines/reference/importWithTrailingSlash.trace.json‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,22 @@
33
"Explicitly specified module resolution kind: 'NodeJs'.",
44
"Loading module as file / folder, candidate module location '/a'.",
55
"File '/a.ts' exist - use it as a name resolution result.",
6-
"Resolving real path for '/a.ts', result '/a.ts'",
76
"======== Module name '.' was successfully resolved to '/a.ts'. ========",
87
"======== Resolving module './' from '/a/test.ts'. ========",
98
"Explicitly specified module resolution kind: 'NodeJs'.",
109
"Loading module as file / folder, candidate module location '/a/'.",
1110
"File '/a/package.json' does not exist.",
1211
"File '/a/index.ts' exist - use it as a name resolution result.",
13-
"Resolving real path for '/a/index.ts', result '/a/index.ts'",
1412
"======== Module name './' was successfully resolved to '/a/index.ts'. ========",
1513
"======== Resolving module '..' from '/a/b/test.ts'. ========",
1614
"Explicitly specified module resolution kind: 'NodeJs'.",
1715
"Loading module as file / folder, candidate module location '/a'.",
1816
"File '/a.ts' exist - use it as a name resolution result.",
19-
"Resolving real path for '/a.ts', result '/a.ts'",
2017
"======== Module name '..' was successfully resolved to '/a.ts'. ========",
2118
"======== Resolving module '../' from '/a/b/test.ts'. ========",
2219
"Explicitly specified module resolution kind: 'NodeJs'.",
2320
"Loading module as file / folder, candidate module location '/a/'.",
2421
"File '/a/package.json' does not exist.",
2522
"File '/a/index.ts' exist - use it as a name resolution result.",
26-
"Resolving real path for '/a/index.ts', result '/a/index.ts'",
2723
"======== Module name '../' was successfully resolved to '/a/index.ts'. ========"
2824
]

‎tests/baselines/reference/moduleResolutionWithExtensions.trace.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
"Module resolution kind is not specified, using 'NodeJs'.",
44
"Loading module as file / folder, candidate module location '/src/a'.",
55
"File '/src/a.ts' exist - use it as a name resolution result.",
6-
"Resolving real path for '/src/a.ts', result '/src/a.ts'",
76
"======== Module name './a' was successfully resolved to '/src/a.ts'. ========",
87
"======== Resolving module './a.js' from '/src/d.ts'. ========",
98
"Module resolution kind is not specified, using 'NodeJs'.",
@@ -13,7 +12,6 @@
1312
"File '/src/a.js.d.ts' does not exist.",
1413
"File name '/src/a.js' has a '.js' extension - stripping it",
1514
"File '/src/a.ts' exist - use it as a name resolution result.",
16-
"Resolving real path for '/src/a.ts', result '/src/a.ts'",
1715
"======== Module name './a.js' was successfully resolved to '/src/a.ts'. ========",
1816
"======== Resolving module './jquery.js' from '/src/jquery_user_1.ts'. ========",
1917
"Module resolution kind is not specified, using 'NodeJs'.",
@@ -25,6 +23,5 @@
2523
"File '/src/jquery.ts' does not exist.",
2624
"File '/src/jquery.tsx' does not exist.",
2725
"File '/src/jquery.d.ts' exist - use it as a name resolution result.",
28-
"Resolving real path for '/src/jquery.d.ts', result '/src/jquery.d.ts'",
2926
"======== Module name './jquery.js' was successfully resolved to '/src/jquery.d.ts'. ========"
3027
]

‎tests/baselines/reference/moduleResolutionWithExtensions_notSupported.trace.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
"Loading module as file / folder, candidate module location '/tsx'.",
55
"File '/tsx.ts' does not exist.",
66
"File '/tsx.tsx' exist - use it as a name resolution result.",
7-
"Resolving real path for '/tsx.tsx', result '/tsx.tsx'",
87
"======== Module name './tsx' was successfully resolved to '/tsx.tsx'. ========",
98
"======== Resolving module './jsx' from '/a.ts'. ========",
109
"Module resolution kind is not specified, using 'NodeJs'.",
@@ -19,7 +18,6 @@
1918
"Loading module as file / folder, candidate module location '/jsx'.",
2019
"File '/jsx.js' does not exist.",
2120
"File '/jsx.jsx' exist - use it as a name resolution result.",
22-
"Resolving real path for '/jsx.jsx', result '/jsx.jsx'",
2321
"======== Module name './jsx' was successfully resolved to '/jsx.jsx'. ========",
2422
"======== Resolving module './js' from '/a.ts'. ========",
2523
"Module resolution kind is not specified, using 'NodeJs'.",
@@ -33,6 +31,5 @@
3331
"File '/js/index.d.ts' does not exist.",
3432
"Loading module as file / folder, candidate module location '/js'.",
3533
"File '/js.js' exist - use it as a name resolution result.",
36-
"Resolving real path for '/js.js', result '/js.js'",
3734
"======== Module name './js' was successfully resolved to '/js.js'. ========"
3835
]

‎tests/baselines/reference/moduleResolutionWithExtensions_notSupported2.trace.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,5 @@
1212
"Loading module as file / folder, candidate module location '/jsx'.",
1313
"File '/jsx.js' does not exist.",
1414
"File '/jsx.jsx' exist - use it as a name resolution result.",
15-
"Resolving real path for '/jsx.jsx', result '/jsx.jsx'",
1615
"======== Module name './jsx' was successfully resolved to '/jsx.jsx'. ========"
1716
]

‎tests/baselines/reference/moduleResolutionWithExtensions_notSupported3.trace.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,5 @@
1212
"Loading module as file / folder, candidate module location '/jsx'.",
1313
"File '/jsx.js' does not exist.",
1414
"File '/jsx.jsx' exist - use it as a name resolution result.",
15-
"Resolving real path for '/jsx.jsx', result '/jsx.jsx'",
1615
"======== Module name './jsx' was successfully resolved to '/jsx.jsx'. ========"
1716
]

‎tests/baselines/reference/moduleResolutionWithExtensions_preferTs.trace.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,5 @@
77
"File '/b.d.ts' does not exist.",
88
"File '/b/package.json' does not exist.",
99
"File '/b/index.ts' exist - use it as a name resolution result.",
10-
"Resolving real path for '/b/index.ts', result '/b/index.ts'",
1110
"======== Module name './b' was successfully resolved to '/b/index.ts'. ========"
1211
]

‎tests/baselines/reference/moduleResolutionWithSymlinks.js‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
//// [tests/cases/compiler/moduleResolutionWithSymlinks.ts] ////
22

33
//// [index.ts]
4+
// When symlinked files are in node_modules, they are resolved with realpath;
5+
// so a linked file does not create a duplicate SourceFile of the real one.
46

57
export class MyClass { private x: number; }
68

@@ -15,23 +17,63 @@ import { MyClass2 } from "./library-b";
1517
let x: MyClass;
1618
let y: MyClass2;
1719
x = y;
18-
y = x;
20+
y = x;
21+
22+
/*
23+
# To reproduce in a real project:
24+
25+
mkdir src; cd src
26+
mkdir library-a
27+
echo 'export class MyClass { private x: number; }' > library-a/index.ts
28+
29+
mkdir library-b; cd library-b
30+
echo 'import {MyClass} from "library-a"; export { MyClass as MyClass2 }' > index.ts
31+
mkdir node_modules; cd node_modules
32+
33+
ln -s ../../library-a library-a # Linux
34+
# Windows: open command prompt as administrator and run: mklink /D library-a ..\..\library-a
35+
36+
cd ../.. # back to src
37+
echo 'import { MyClass } from "./library-a"; import { MyClass2 } from "./library-b"; let x: MyClass; let y: MyClass2; x = y; y = x;' > app.ts
38+
tsc app.ts # Should write to library-a/index.js, library-b/index.js, and app.js
39+
*/
1940

20-
//// [index.js]
41+
42+
//// [/src/library-a/index.js]
43+
// When symlinked files are in node_modules, they are resolved with realpath;
44+
// so a linked file does not create a duplicate SourceFile of the real one.
2145
"use strict";
2246
var MyClass = (function () {
2347
function MyClass() {
2448
}
2549
return MyClass;
2650
}());
2751
exports.MyClass = MyClass;
28-
//// [index.js]
52+
//// [/src/library-b/index.js]
2953
"use strict";
3054
var library_a_1 = require("library-a");
3155
exports.MyClass2 = library_a_1.MyClass;
32-
//// [app.js]
56+
//// [/src/app.js]
3357
"use strict";
3458
var x;
3559
var y;
3660
x = y;
3761
y = x;
62+
/*
63+
# To reproduce in a real project:
64+
65+
mkdir src; cd src
66+
mkdir library-a
67+
echo 'export class MyClass { private x: number; }' > library-a/index.ts
68+
69+
mkdir library-b; cd library-b
70+
echo 'import {MyClass} from "library-a"; export { MyClass as MyClass2 }' > index.ts
71+
mkdir node_modules; cd node_modules
72+
73+
ln -s ../../library-a library-a # Linux
74+
# Windows: open command prompt as administrator and run: mklink /D library-a ..\..\library-a
75+
76+
cd ../.. # back to src
77+
echo 'import { MyClass } from "./library-a"; import { MyClass2 } from "./library-b"; let x: MyClass; let y: MyClass2; x = y; y = x;' > app.ts
78+
tsc app.ts # Should write to library-a/index.js, library-b/index.js, and app.js
79+
*/

‎tests/baselines/reference/moduleResolutionWithSymlinks.symbols‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,32 @@ y = x;
2121
>y : Symbol(y, Decl(app.ts, 4, 3))
2222
>x : Symbol(x, Decl(app.ts, 3, 3))
2323

24+
/*
25+
# To reproduce in a real project:
26+
27+
mkdir src; cd src
28+
mkdir library-a
29+
echo 'export class MyClass { private x: number; }' > library-a/index.ts
30+
31+
mkdir library-b; cd library-b
32+
echo 'import {MyClass} from "library-a"; export { MyClass as MyClass2 }' > index.ts
33+
mkdir node_modules; cd node_modules
34+
35+
ln -s ../../library-a library-a # Linux
36+
# Windows: open command prompt as administrator and run: mklink /D library-a ..\..\library-a
37+
38+
cd ../.. # back to src
39+
echo 'import { MyClass } from "./library-a"; import { MyClass2 } from "./library-b"; let x: MyClass; let y: MyClass2; x = y; y = x;' > app.ts
40+
tsc app.ts # Should write to library-a/index.js, library-b/index.js, and app.js
41+
*/
42+
2443
=== /src/library-a/index.ts ===
44+
// When symlinked files are in node_modules, they are resolved with realpath;
45+
// so a linked file does not create a duplicate SourceFile of the real one.
2546

2647
export class MyClass { private x: number; }
2748
>MyClass : Symbol(MyClass, Decl(index.ts, 0, 0))
28-
>x : Symbol(MyClass.x, Decl(index.ts, 1, 22))
49+
>x : Symbol(MyClass.x, Decl(index.ts, 3, 22))
2950

3051
=== /src/library-b/index.ts ===
3152
import {MyClass} from "library-a";

0 commit comments

Comments
 (0)