Skip to content

Commit 9c80909

Browse files
author
Andy Hanson
committed
Resolve symlinks for type reference directives too.
1 parent b6727ea commit 9c80909

42 files changed

Lines changed: 384 additions & 66 deletions

File tree

Some content is hidden

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

src/compiler/moduleNameResolver.ts

Lines changed: 50 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -195,63 +195,66 @@ namespace ts {
195195

196196
const failedLookupLocations: string[] = [];
197197

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

207-
const resolved = resolvedTypeScriptOnly(
208-
loadNodeModuleFromDirectory(Extensions.DtsOnly, candidate, failedLookupLocations,
209-
!directoryProbablyExists(candidateDirectory, host), moduleResolutionState));
214+
return { resolvedTypeReferenceDirective, failedLookupLocations };
210215

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

228-
let resolvedFile: string;
229-
const initialLocationForSecondaryLookup = containingFile && getDirectoryPath(containingFile);
237+
function secondaryLookup(): string | undefined {
238+
let resolvedFile: string;
239+
const initialLocationForSecondaryLookup = containingFile && getDirectoryPath(containingFile);
230240

231-
if (initialLocationForSecondaryLookup !== undefined) {
232-
// check secondary locations
233-
if (traceEnabled) {
234-
trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
235-
}
236-
resolvedFile = resolvedTypeScriptOnly(loadModuleFromNodeModules(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, failedLookupLocations, moduleResolutionState, /*checkOneLevel*/ false));
237-
if (traceEnabled) {
238-
if (resolvedFile) {
239-
trace(host, Diagnostics.Type_reference_directive_0_was_successfully_resolved_to_1_primary_Colon_2, typeReferenceDirectiveName, resolvedFile, false);
241+
if (initialLocationForSecondaryLookup !== undefined) {
242+
// check secondary locations
243+
if (traceEnabled) {
244+
trace(host, Diagnostics.Looking_up_in_node_modules_folder_initial_location_0, initialLocationForSecondaryLookup);
240245
}
241-
else {
246+
resolvedFile = resolvedTypeScriptOnly(loadModuleFromNodeModules(Extensions.DtsOnly, typeReferenceDirectiveName, initialLocationForSecondaryLookup, failedLookupLocations, moduleResolutionState, /*checkOneLevel*/ false));
247+
if (!resolvedFile && traceEnabled) {
242248
trace(host, Diagnostics.Type_reference_directive_0_was_not_resolved, typeReferenceDirectiveName);
243249
}
250+
return resolvedFile;
244251
}
245-
}
246-
else {
247-
if (traceEnabled) {
248-
trace(host, Diagnostics.Containing_file_is_not_specified_and_root_directory_cannot_be_determined_skipping_lookup_in_node_modules_folder);
252+
else {
253+
if (traceEnabled) {
254+
trace(host, Diagnostics.Containing_file_is_not_specified_and_root_directory_cannot_be_determined_skipping_lookup_in_node_modules_folder);
255+
}
249256
}
250257
}
251-
return {
252-
resolvedTypeReferenceDirective: resolvedFile ? { primary: false, resolvedFileName: resolvedFile } : undefined,
253-
failedLookupLocations
254-
};
255258
}
256259

257260
/**
@@ -564,7 +567,7 @@ namespace ts {
564567
}
565568
const resolved = loadModuleFromNodeModules(extensions, moduleName, containingDirectory, failedLookupLocations, state, /*checkOneLevel*/ false);
566569
// 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 };
570+
return resolved && { resolved: { path: realpath(resolved.path, host, traceEnabled), extension: resolved.extension }, isExternalLibraryImport: true };
568571
}
569572
else {
570573
const candidate = normalizePath(combinePaths(containingDirectory, moduleName));
@@ -574,16 +577,16 @@ namespace ts {
574577
}
575578
}
576579

577-
function resolvedWithRealpath(resolved: Resolved, host: ModuleResolutionHost, traceEnabled: boolean): Resolved {
580+
function realpath(path: string, host: ModuleResolutionHost, traceEnabled: boolean): string {
578581
if (!host.realpath) {
579-
return resolved;
582+
return path;
580583
}
581584

582-
const real = normalizePath(host.realpath(resolved.path));
585+
const real = normalizePath(host.realpath(path));
583586
if (traceEnabled) {
584-
trace(host, Diagnostics.Resolving_real_path_for_0_result_1, resolved.path, real);
587+
trace(host, Diagnostics.Resolving_real_path_for_0_result_1, path, real);
585588
}
586-
return { path: real, extension: resolved.extension };
589+
return real;
587590
}
588591

589592
function nodeLoadModuleByRelativeName(extensions: Extensions, candidate: string, failedLookupLocations: string[], onlyRecordFailures: boolean, state: ModuleResolutionState): Resolved | undefined {

src/compiler/program.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ namespace ts {
480480
return resolveModuleNamesWorker(moduleNames, containingFile);
481481
}
482482

483-
// at this point we know that either
483+
// at this point we know that either
484484
// - file has local declarations for ambient modules
485485
// OR
486486
// - old program state is available
@@ -674,7 +674,7 @@ namespace ts {
674674
}
675675

676676
const modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path);
677-
// try to verify results of module resolution
677+
// try to verify results of module resolution
678678
for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) {
679679
const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory);
680680
if (resolveModuleNamesWorker) {
@@ -1395,14 +1395,17 @@ namespace ts {
13951395
// If we already resolved to this file, it must have been a secondary reference. Check file contents
13961396
// for sameness and possibly issue an error
13971397
if (previousResolution) {
1398-
const otherFileText = host.readFile(resolvedTypeReferenceDirective.resolvedFileName);
1399-
if (otherFileText !== getSourceFile(previousResolution.resolvedFileName).text) {
1400-
fileProcessingDiagnostics.add(createDiagnostic(refFile, refPos, refEnd,
1401-
Diagnostics.Conflicting_definitions_for_0_found_at_1_and_2_Consider_installing_a_specific_version_of_this_library_to_resolve_the_conflict,
1402-
typeReferenceDirective,
1403-
resolvedTypeReferenceDirective.resolvedFileName,
1404-
previousResolution.resolvedFileName
1405-
));
1398+
// Don't bother reading the file again if it's the same file.
1399+
if (resolvedTypeReferenceDirective.resolvedFileName !== previousResolution.resolvedFileName) {
1400+
const otherFileText = host.readFile(resolvedTypeReferenceDirective.resolvedFileName);
1401+
if (otherFileText !== getSourceFile(previousResolution.resolvedFileName).text) {
1402+
fileProcessingDiagnostics.add(createDiagnostic(refFile, refPos, refEnd,
1403+
Diagnostics.Conflicting_definitions_for_0_found_at_1_and_2_Consider_installing_a_specific_version_of_this_library_to_resolve_the_conflict,
1404+
typeReferenceDirective,
1405+
resolvedTypeReferenceDirective.resolvedFileName,
1406+
previousResolution.resolvedFileName
1407+
));
1408+
}
14061409
}
14071410
// don't overwrite previous resolution result
14081411
saveResolution = false;

src/harness/harness.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1021,7 +1021,14 @@ namespace Harness {
10211021
useCaseSensitiveFileNames: () => useCaseSensitiveFileNames,
10221022
getNewLine: () => newLine,
10231023
fileExists: fileName => fileMap.contains(toPath(fileName)),
1024-
readFile: (fileName: string): string => fileMap.get(toPath(fileName)).getText(),
1024+
readFile: (fileName: string): string => {
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+
},
10251032
realpath: (fileName: string): ts.Path => {
10261033
const path = toPath(fileName);
10271034
return (realPathMap.get(path) as ts.Path) || path;

tests/baselines/reference/library-reference-1.trace.json

Lines changed: 4 additions & 2 deletions
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
]

tests/baselines/reference/library-reference-10.trace.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
"Found 'package.json' at './types/jquery/package.json'.",
55
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
66
"File 'types/jquery/jquery.d.ts' exist - use it as a name resolution result.",
7-
"======== Type reference directive 'jquery' was successfully resolved to 'types/jquery/jquery.d.ts', primary: true. ========",
7+
"Resolving real path for 'types/jquery/jquery.d.ts', result '/foo/types/jquery/jquery.d.ts'",
8+
"======== Type reference directive 'jquery' was successfully resolved to '/foo/types/jquery/jquery.d.ts', primary: true. ========",
89
"======== Resolving type reference directive 'jquery', containing file '/foo/__inferred type names__.ts', root directory './types'. ========",
910
"Resolving with primary search path './types'",
1011
"Found 'package.json' at './types/jquery/package.json'.",
1112
"'package.json' has 'typings' field 'jquery.d.ts' that references 'types/jquery/jquery.d.ts'.",
1213
"File 'types/jquery/jquery.d.ts' exist - use it as a name resolution result.",
13-
"======== Type reference directive 'jquery' was successfully resolved to 'types/jquery/jquery.d.ts', primary: true. ========"
14+
"Resolving real path for 'types/jquery/jquery.d.ts', result '/foo/types/jquery/jquery.d.ts'",
15+
"======== Type reference directive 'jquery' was successfully resolved to '/foo/types/jquery/jquery.d.ts', primary: true. ========"
1416
]

tests/baselines/reference/library-reference-11.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@
1212
"Found 'package.json' at '/a/node_modules/jquery/package.json'.",
1313
"'package.json' has 'typings' field 'jquery.d.ts' that references '/a/node_modules/jquery/jquery.d.ts'.",
1414
"File '/a/node_modules/jquery/jquery.d.ts' exist - use it as a name resolution result.",
15+
"Resolving real path for '/a/node_modules/jquery/jquery.d.ts', result '/a/node_modules/jquery/jquery.d.ts'",
1516
"======== Type reference directive 'jquery' was successfully resolved to '/a/node_modules/jquery/jquery.d.ts', primary: false. ========"
1617
]

tests/baselines/reference/library-reference-12.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,6 @@
1212
"Found 'package.json' at '/a/node_modules/jquery/package.json'.",
1313
"'package.json' has 'types' field 'dist/jquery.d.ts' that references '/a/node_modules/jquery/dist/jquery.d.ts'.",
1414
"File '/a/node_modules/jquery/dist/jquery.d.ts' exist - use it as a name resolution result.",
15+
"Resolving real path for '/a/node_modules/jquery/dist/jquery.d.ts', result '/a/node_modules/jquery/dist/jquery.d.ts'",
1516
"======== Type reference directive 'jquery' was successfully resolved to '/a/node_modules/jquery/dist/jquery.d.ts', primary: false. ========"
1617
]

tests/baselines/reference/library-reference-13.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
"Resolving with primary search path '/a/types'",
44
"File '/a/types/jquery/package.json' does not exist.",
55
"File '/a/types/jquery/index.d.ts' exist - use it as a name resolution result.",
6+
"Resolving real path for '/a/types/jquery/index.d.ts', result '/a/types/jquery/index.d.ts'",
67
"======== Type reference directive 'jquery' was successfully resolved to '/a/types/jquery/index.d.ts', primary: true. ========"
78
]

tests/baselines/reference/library-reference-14.trace.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
33
"Resolving with primary search path '/a/types'",
44
"File '/a/types/jquery/package.json' does not exist.",
55
"File '/a/types/jquery/index.d.ts' exist - use it as a name resolution result.",
6+
"Resolving real path for '/a/types/jquery/index.d.ts', result '/a/types/jquery/index.d.ts'",
67
"======== Type reference directive 'jquery' was successfully resolved to '/a/types/jquery/index.d.ts', primary: true. ========"
78
]

tests/baselines/reference/library-reference-15.trace.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,6 @@
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 '/a/types/jquery/index.d.ts'",
7+
"======== Type reference directive 'jquery' was successfully resolved to '/a/types/jquery/index.d.ts', primary: true. ========"
78
]

0 commit comments

Comments
 (0)