Skip to content

Commit 82d4ccc

Browse files
committed
Use original path from resolved module cache when available.
Fixes #26273 Thanks to @ajafff, for partial fix and test (#26310)
1 parent 46d3caa commit 82d4ccc

File tree

6 files changed

+31
-20
lines changed

6 files changed

+31
-20
lines changed

src/compiler/moduleNameResolver.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ namespace ts {
2929
path: string;
3030
extension: Extension;
3131
packageId: PackageId | undefined;
32+
originalPath?: string | true;
3233
}
3334

3435
/** Result of trying to resolve a module at a file. Needs to have 'packageId' added later. */
@@ -62,9 +63,9 @@ namespace ts {
6263
return { fileName: resolved.path, packageId: resolved.packageId };
6364
}
6465

65-
function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, originalPath: string | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations {
66+
function createResolvedModuleWithFailedLookupLocations(resolved: Resolved | undefined, isExternalLibraryImport: boolean, failedLookupLocations: string[]): ResolvedModuleWithFailedLookupLocations {
6667
return {
67-
resolvedModule: resolved && { resolvedFileName: resolved.path, originalPath, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId },
68+
resolvedModule: resolved && { resolvedFileName: resolved.path, originalPath: resolved.originalPath === true ? undefined : resolved.originalPath, extension: resolved.extension, isExternalLibraryImport, packageId: resolved.packageId },
6869
failedLookupLocations
6970
};
7071
}
@@ -754,12 +755,12 @@ namespace ts {
754755
tryResolve(Extensions.JavaScript) ||
755756
(compilerOptions.resolveJsonModule ? tryResolve(Extensions.Json) : undefined));
756757
if (result && result.value) {
757-
const { resolved, originalPath, isExternalLibraryImport } = result.value;
758-
return createResolvedModuleWithFailedLookupLocations(resolved, originalPath, isExternalLibraryImport, failedLookupLocations);
758+
const { resolved, isExternalLibraryImport } = result.value;
759+
return createResolvedModuleWithFailedLookupLocations(resolved, isExternalLibraryImport, failedLookupLocations);
759760
}
760761
return { resolvedModule: undefined, failedLookupLocations };
761762

762-
function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, originalPath?: string, isExternalLibraryImport: boolean }> {
763+
function tryResolve(extensions: Extensions): SearchResult<{ resolved: Resolved, isExternalLibraryImport: boolean }> {
763764
const loader: ResolutionKindSpecificLoader = (extensions, candidate, failedLookupLocations, onlyRecordFailures, state) => nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, onlyRecordFailures, state, /*considerPackageJson*/ true);
764765
const resolved = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loader, failedLookupLocations, state);
765766
if (resolved) {
@@ -774,17 +775,13 @@ namespace ts {
774775
if (!resolved) return undefined;
775776

776777
let resolvedValue = resolved.value;
777-
let originalPath: string | undefined;
778-
if (!compilerOptions.preserveSymlinks && resolvedValue) {
779-
originalPath = resolvedValue.path;
778+
if (!compilerOptions.preserveSymlinks && resolvedValue && !resolvedValue.originalPath) {
780779
const path = realPath(resolvedValue.path, host, traceEnabled);
781-
if (path === originalPath) {
782-
originalPath = undefined;
783-
}
784-
resolvedValue = { ...resolvedValue, path };
780+
const originalPath = path === resolvedValue.path ? undefined : resolvedValue.path;
781+
resolvedValue = { ...resolvedValue, path, originalPath };
785782
}
786783
// For node_modules lookups, get the real path so that multiple accesses to an `npm link`-ed module do not create duplicate files.
787-
return { value: resolvedValue && { resolved: resolvedValue, originalPath, isExternalLibraryImport: true } };
784+
return { value: resolvedValue && { resolved: resolvedValue, isExternalLibraryImport: true } };
788785
}
789786
else {
790787
const { path: candidate, parts } = normalizePathAndParts(combinePaths(containingDirectory, moduleName));
@@ -1234,7 +1231,7 @@ namespace ts {
12341231
trace(host, Diagnostics.Resolution_for_module_0_was_found_in_cache_from_location_1, moduleName, containingDirectory);
12351232
}
12361233
failedLookupLocations.push(...result.failedLookupLocations);
1237-
return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } };
1234+
return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, originalPath: result.resolvedModule.originalPath || true, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } };
12381235
}
12391236
}
12401237

@@ -1246,7 +1243,7 @@ namespace ts {
12461243

12471244
const resolved = tryResolve(Extensions.TypeScript) || tryResolve(Extensions.JavaScript);
12481245
// No originalPath because classic resolution doesn't resolve realPath
1249-
return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*originalPath*/ undefined, /*isExternalLibraryImport*/ false, failedLookupLocations);
1246+
return createResolvedModuleWithFailedLookupLocations(resolved && resolved.value, /*isExternalLibraryImport*/ false, failedLookupLocations);
12501247

12511248
function tryResolve(extensions: Extensions): SearchResult<Resolved> {
12521249
const resolvedUsingSettings = tryLoadModuleUsingOptionalResolutionSettings(extensions, moduleName, containingDirectory, loadModuleFromFileNoPackageId, failedLookupLocations, state);
@@ -1293,7 +1290,7 @@ namespace ts {
12931290
const state: ModuleResolutionState = { compilerOptions, host, traceEnabled };
12941291
const failedLookupLocations: string[] = [];
12951292
const resolved = loadModuleFromNodeModulesOneLevel(Extensions.DtsOnly, moduleName, globalCache, failedLookupLocations, state);
1296-
return createResolvedModuleWithFailedLookupLocations(resolved, /*originalPath*/ undefined, /*isExternalLibraryImport*/ true, failedLookupLocations);
1293+
return createResolvedModuleWithFailedLookupLocations(resolved, /*isExternalLibraryImport*/ true, failedLookupLocations);
12971294
}
12981295

12991296
/**

src/testRunner/unittests/moduleResolution.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,23 @@ namespace ts {
427427
resolution = resolveModuleName("a", "/foo.ts", compilerOptions, host, cache);
428428
assert.isUndefined(resolution.resolvedModule, "lookup in parent directory doesn't hit the cache");
429429
});
430+
431+
it("preserves originalPath on cache hit", () => {
432+
const host = createModuleResolutionHost(
433+
/*hasDirectoryExists*/ true,
434+
{ name: "/linked/index.d.ts", symlinks: ["/app/node_modules/linked/index.d.ts"] },
435+
{ name: "/app/node_modules/linked/package.json", content: '{"version": "0.0.0", "main": "./index"}' },
436+
);
437+
const cache = createModuleResolutionCache("/", (f) => f);
438+
const compilerOptions: CompilerOptions = { moduleResolution: ModuleResolutionKind.NodeJs };
439+
checkResolution(resolveModuleName("linked", "/app/src/app.ts", compilerOptions, host, cache));
440+
checkResolution(resolveModuleName("linked", "/app/lib/main.ts", compilerOptions, host, cache));
441+
442+
function checkResolution(resolution: ResolvedModuleWithFailedLookupLocations) {
443+
checkResolvedModule(resolution.resolvedModule, createResolvedModule("/linked/index.d.ts", /*isExternalLibraryImport*/ true));
444+
assert.strictEqual(resolution.resolvedModule!.originalPath, "/app/node_modules/linked/index.d.ts");
445+
}
446+
});
430447
});
431448

432449
describe("Module resolution - relative imports", () => {

src/testRunner/unittests/tsserverProjectSystem.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8415,7 +8415,7 @@ new C();`
84158415
expectedTrace.push(`Loading module '${moduleName}' from 'node_modules' folder, target file type 'TypeScript'.`);
84168416
getExpectedMissedLocationResolutionTrace(host, expectedTrace, getDirectoryPath(file.path), module, moduleName, /*useNodeModules*/ true, cacheLocation);
84178417
expectedTrace.push(`Resolution for module '${moduleName}' was found in cache from location '${cacheLocation}'.`);
8418-
getExpectedResolutionTraceFooter(expectedTrace, module, moduleName, /*addRealPathTrace*/ true, /*ignoreModuleFileFound*/ true);
8418+
getExpectedResolutionTraceFooter(expectedTrace, module, moduleName, /*addRealPathTrace*/ false, /*ignoreModuleFileFound*/ true);
84198419
return expectedTrace;
84208420
}
84218421

tests/baselines/reference/cachedModuleResolution1.trace.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@
1414
"Explicitly specified module resolution kind: 'NodeJs'.",
1515
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
1616
"Resolution for module 'foo' was found in cache from location '/a/b/c'.",
17-
"Resolving real path for '/a/b/node_modules/foo.d.ts', result '/a/b/node_modules/foo.d.ts'.",
1817
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo.d.ts'. ========"
1918
]

tests/baselines/reference/cachedModuleResolution2.trace.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@
1414
"Directory '/a/b/c/d/e/node_modules' does not exist, skipping all lookups in it.",
1515
"Directory '/a/b/c/d/node_modules' does not exist, skipping all lookups in it.",
1616
"Resolution for module 'foo' was found in cache from location '/a/b/c'.",
17-
"Resolving real path for '/a/b/node_modules/foo.d.ts', result '/a/b/node_modules/foo.d.ts'.",
1817
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo.d.ts'. ========"
1918
]

tests/baselines/reference/cachedModuleResolution5.trace.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@
1414
"Explicitly specified module resolution kind: 'NodeJs'.",
1515
"Loading module 'foo' from 'node_modules' folder, target file type 'TypeScript'.",
1616
"Resolution for module 'foo' was found in cache from location '/a/b'.",
17-
"Resolving real path for '/a/b/node_modules/foo.d.ts', result '/a/b/node_modules/foo.d.ts'.",
1817
"======== Module name 'foo' was successfully resolved to '/a/b/node_modules/foo.d.ts'. ========"
1918
]

0 commit comments

Comments
 (0)