Skip to content

Commit 1510674

Browse files
author
Andy
authored
Merge pull request #12020 from Microsoft/symlink3
Only resolve symlinks in `node_modules`
2 parents 9a3dd3e + 6c7e1b6 commit 1510674

File tree

67 files changed

+705
-127
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+705
-127
lines changed

src/compiler/moduleNameResolver.ts

+52-48
Original file line numberDiff line numberDiff line change
@@ -193,63 +193,66 @@ namespace ts {
193193

194194
const failedLookupLocations: string[] = [];
195195

196-
// Check primary library paths
197-
if (typeRoots && typeRoots.length) {
196+
let resolved = primaryLookup();
197+
let primary = true;
198+
if (!resolved) {
199+
resolved = secondaryLookup();
200+
primary = false;
201+
}
202+
203+
let resolvedTypeReferenceDirective: ResolvedTypeReferenceDirective | undefined;
204+
if (resolved) {
205+
resolved = realpath(resolved, host, traceEnabled);
198206
if (traceEnabled) {
199-
trace(host, Diagnostics.Resolving_with_primary_search_path_0, typeRoots.join(", "));
207+
trace(host, Diagnostics.Type_reference_directive_0_was_successfully_resolved_to_1_primary_Colon_2, typeReferenceDirectiveName, resolved, primary);
200208
}
201-
for (const typeRoot of typeRoots) {
202-
const candidate = combinePaths(typeRoot, typeReferenceDirectiveName);
203-
const candidateDirectory = getDirectoryPath(candidate);
209+
resolvedTypeReferenceDirective = { primary, resolvedFileName: resolved };
210+
}
204211

205-
const resolved = resolvedTypeScriptOnly(
206-
loadNodeModuleFromDirectory(Extensions.DtsOnly, candidate, failedLookupLocations,
207-
!directoryProbablyExists(candidateDirectory, host), moduleResolutionState));
212+
return { resolvedTypeReferenceDirective, failedLookupLocations };
208213

209-
if (resolved) {
210-
if (traceEnabled) {
211-
trace(host, Diagnostics.Type_reference_directive_0_was_successfully_resolved_to_1_primary_Colon_2, typeReferenceDirectiveName, resolved, true);
212-
}
213-
return {
214-
resolvedTypeReferenceDirective: { primary: true, resolvedFileName: resolved },
215-
failedLookupLocations
216-
};
214+
function primaryLookup(): string | undefined {
215+
// Check primary library paths
216+
if (typeRoots && typeRoots.length) {
217+
if (traceEnabled) {
218+
trace(host, Diagnostics.Resolving_with_primary_search_path_0, typeRoots.join(", "));
217219
}
220+
return forEach(typeRoots, typeRoot => {
221+
const candidate = combinePaths(typeRoot, typeReferenceDirectiveName);
222+
const candidateDirectory = getDirectoryPath(candidate);
223+
return resolvedTypeScriptOnly(
224+
loadNodeModuleFromDirectory(Extensions.DtsOnly, candidate, failedLookupLocations,
225+
!directoryProbablyExists(candidateDirectory, host), moduleResolutionState));
226+
});
218227
}
219-
}
220-
else {
221-
if (traceEnabled) {
222-
trace(host, Diagnostics.Root_directory_cannot_be_determined_skipping_primary_search_paths);
228+
else {
229+
if (traceEnabled) {
230+
trace(host, Diagnostics.Root_directory_cannot_be_determined_skipping_primary_search_paths);
231+
}
223232
}
224233
}
225234

226-
let resolvedFile: string;
227-
const initialLocationForSecondaryLookup = containingFile && getDirectoryPath(containingFile);
235+
function secondaryLookup(): string | undefined {
236+
let resolvedFile: string;
237+
const initialLocationForSecondaryLookup = containingFile && getDirectoryPath(containingFile);
228238

229-
if (initialLocationForSecondaryLookup !== undefined) {
230-
// check secondary locations
231-
if (traceEnabled) {
232-
trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
233-
}
234-
resolvedFile = resolvedTypeScriptOnly(loadModuleFromNodeModules(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, failedLookupLocations, moduleResolutionState));
235-
if (traceEnabled) {
236-
if (resolvedFile) {
237-
trace(host, Diagnostics.Type_reference_directive_0_was_successfully_resolved_to_1_primary_Colon_2, typeReferenceDirectiveName, resolvedFile, false);
239+
if (initialLocationForSecondaryLookup !== undefined) {
240+
// check secondary locations
241+
if (traceEnabled) {
242+
trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
238243
}
239-
else {
244+
resolvedFile = resolvedTypeScriptOnly(loadModuleFromNodeModules(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, failedLookupLocations, moduleResolutionState));
245+
if (!resolvedFile && traceEnabled) {
240246
trace(host, Diagnostics.Type_reference_directive_0_was_not_resolved, typeReferenceDirectiveName);
241247
}
248+
return resolvedFile;
242249
}
243-
}
244-
else {
245-
if (traceEnabled) {
246-
trace(host, Diagnostics.Containing_file_is_not_specified_and_root_directory_cannot_be_determined_skipping_lookup_in_node_modules_folder);
250+
else {
251+
if (traceEnabled) {
252+
trace(host, Diagnostics.Containing_file_is_not_specified_and_root_directory_cannot_be_determined_skipping_lookup_in_node_modules_folder);
253+
}
247254
}
248255
}
249-
return {
250-
resolvedTypeReferenceDirective: resolvedFile ? { primary: false, resolvedFileName: resolvedFile } : undefined,
251-
failedLookupLocations
252-
};
253256
}
254257

255258
/**
@@ -546,7 +549,7 @@ namespace ts {
546549
const result = tryResolve(Extensions.TypeScript) || tryResolve(Extensions.JavaScript);
547550
if (result) {
548551
const { resolved, isExternalLibraryImport } = result;
549-
return createResolvedModuleWithFailedLookupLocations(resolved && resolvedWithRealpath(resolved, host, traceEnabled), isExternalLibraryImport, failedLookupLocations);
552+
return createResolvedModuleWithFailedLookupLocations(resolved, isExternalLibraryImport, failedLookupLocations);
550553
}
551554
return { resolvedModule: undefined, failedLookupLocations };
552555

@@ -561,7 +564,8 @@ namespace ts {
561564
trace(host, Diagnostics.Loading_module_0_from_node_modules_folder, moduleName);
562565
}
563566
const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state);
564-
return resolved && { resolved, isExternalLibraryImport: true };
567+
// For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files.
568+
return resolved && { resolved: { path: realpath(resolved.path, host, traceEnabled), extension: resolved.extension }, isExternalLibraryImport: true };
565569
}
566570
else {
567571
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));
@@ -571,16 +575,16 @@ namespace ts {
571575
}
572576
}
573577

574-
function resolvedWithRealpath(resolved: Resolved, host: ModuleResolutionHost, traceEnabled: boolean): Resolved {
578+
function realpath(path: string, host: ModuleResolutionHost, traceEnabled: boolean): string {
575579
if (!host.realpath) {
576-
return resolved;
580+
return path;
577581
}
578582

579-
const real = normalizePath(host.realpath(resolved.path));
583+
const real = normalizePath(host.realpath(path));
580584
if (traceEnabled) {
581-
trace(host, Diagnostics.Resolving_real_path_for_0_result_1, resolved.path, real);
585+
trace(host, Diagnostics.Resolving_real_path_for_0_result_1, path, real);
582586
}
583-
return { path: real, extension: resolved.extension };
587+
return real;
584588
}
585589

586590
function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: Push<string>, onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined {

src/compiler/program.ts

+11-8
Original file line numberDiff line numberDiff line change
@@ -1404,14 +1404,17 @@ namespace ts {
14041404
// If we already resolved to this file, it must have been a secondary reference. Check file contents
14051405
// for sameness and possibly issue an error
14061406
if (previousResolution) {
1407-
const otherFileText = host.readFile(resolvedTypeReferenceDirective.resolvedFileName);
1408-
if (otherFileText !== getSourceFile(previousResolution.resolvedFileName).text) {
1409-
fileProcessingDiagnostics.add(createDiagnostic(refFile, refPos, refEnd,
1410-
Diagnostics.Conflicting_definitions_for_0_found_at_1_and_2_Consider_installing_a_specific_version_of_this_library_to_resolve_the_conflict,
1411-
typeReferenceDirective,
1412-
resolvedTypeReferenceDirective.resolvedFileName,
1413-
previousResolution.resolvedFileName
1414-
));
1407+
// Don't bother reading the file again if it's the same file.
1408+
if (resolvedTypeReferenceDirective.resolvedFileName !== previousResolution.resolvedFileName) {
1409+
const otherFileText = host.readFile(resolvedTypeReferenceDirective.resolvedFileName);
1410+
if (otherFileText !== getSourceFile(previousResolution.resolvedFileName).text) {
1411+
fileProcessingDiagnostics.add(createDiagnostic(refFile, refPos, refEnd,
1412+
Diagnostics.Conflicting_definitions_for_0_found_at_1_and_2_Consider_installing_a_specific_version_of_this_library_to_resolve_the_conflict,
1413+
typeReferenceDirective,
1414+
resolvedTypeReferenceDirective.resolvedFileName,
1415+
previousResolution.resolvedFileName
1416+
));
1417+
}
14151418
}
14161419
// don't overwrite previous resolution result
14171420
saveResolution = false;

src/harness/harness.ts

+35-20
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,26 @@ 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-
},
1023+
fileExists: fileName => fileMap.contains(toPath(fileName)),
10141024
readFile: (fileName: string): string => {
1015-
return fileMap.get(ts.toPath(fileName, currentDirectory, getCanonicalFileName))().getText();
1025+
const file = fileMap.get(toPath(fileName));
1026+
if (ts.endsWith(fileName, "json")) {
1027+
// strip comments
1028+
return file.getText();
1029+
}
1030+
return file.text;
1031+
},
1032+
realpath: (fileName: string): ts.Path => {
1033+
const path = toPath(fileName);
1034+
return (realPathMap.get(path) as ts.Path) || path;
10161035
},
1017-
realpath: realPathMap && ((f: string) => {
1018-
const path = ts.toPath(f, currentDirectory, getCanonicalFileName);
1019-
return realPathMap.get(path) || path;
1020-
}),
10211036
directoryExists: dir => {
10221037
let path = ts.toPath(dir, currentDirectory, getCanonicalFileName);
10231038
// Strip trailing /, which may exist if the path is a drive root
10241039
if (path[path.length - 1] === "/") {
10251040
path = <ts.Path>path.substr(0, path.length - 1);
10261041
}
1027-
return mapHasFileInDirectory(path, fileMap) || mapHasFileInDirectory(path, realPathMap);
1042+
return mapHasFileInDirectory(path, fileMap);
10281043
},
10291044
getDirectories: d => {
10301045
const path = ts.toPath(d, currentDirectory, getCanonicalFileName);

tests/baselines/reference/importWithTrailingSlash.trace.json

-4
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/library-reference-1.trace.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
"Resolving with primary search path 'types'",
44
"File 'types/jquery/package.json' does not exist.",
55
"File 'types/jquery/index.d.ts' exist - use it as a name resolution result.",
6-
"======== Type reference directive 'jquery' was successfully resolved to 'types/jquery/index.d.ts', primary: true. ========",
6+
"Resolving real path for 'types/jquery/index.d.ts', result '/src/types/jquery/index.d.ts'",
7+
"======== Type reference directive 'jquery' was successfully resolved to '/src/types/jquery/index.d.ts', primary: true. ========",
78
"======== Resolving type reference directive 'jquery', containing file '/src/__inferred type names__.ts', root directory 'types'. ========",
89
"Resolving with primary search path 'types'",
910
"File 'types/jquery/package.json' does not exist.",
1011
"File 'types/jquery/index.d.ts' exist - use it as a name resolution result.",
11-
"======== Type reference directive 'jquery' was successfully resolved to 'types/jquery/index.d.ts', primary: true. ========"
12+
"Resolving real path for 'types/jquery/index.d.ts', result '/src/types/jquery/index.d.ts'",
13+
"======== Type reference directive 'jquery' was successfully resolved to '/src/types/jquery/index.d.ts', primary: true. ========"
1214
]

0 commit comments

Comments
 (0)