moduleNameResolver: preserve originalPath of symlinked module on cache hit #26310
Conversation
src/compiler/moduleNameResolver.ts
Outdated
| @@ -1233,7 +1233,7 @@ namespace ts { | |||
| trace(host, Diagnostics.Resolution_for_module_0_was_found_in_cache_from_location_1, moduleName, containingDirectory); | |||
| } | |||
| failedLookupLocations.push(...result.failedLookupLocations); | |||
| return { value: result.resolvedModule && { path: result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } }; | |||
| return { value: result.resolvedModule && { path: result.resolvedModule.originalPath || result.resolvedModule.resolvedFileName, extension: result.resolvedModule.extension, packageId: result.resolvedModule.packageId } }; | |||
There was a problem hiding this comment.
I think instead of doing this we should also add another field originalPath to resolved so that we don't have to do original path calculation again for symlinks
also add test for cached realpath result
| @@ -29,6 +29,7 @@ namespace ts { | |||
| path: string; | |||
| extension: Extension; | |||
| packageId: PackageId | undefined; | |||
| originalPath: string | undefined; | |||
There was a problem hiding this comment.
Make this optional so you dont need to set it in withPackageId
| let originalPath: string | undefined; | ||
| if (!compilerOptions.preserveSymlinks && resolvedValue) { | ||
| originalPath = resolvedValue.path; | ||
| if (!compilerOptions.preserveSymlinks && resolvedValue && !resolvedValue.originalPath) { |
There was a problem hiding this comment.
Hmm we need better way to identify that its from cache and no need to calculate originalPath since when its not symbollink and its coming from cache we shouldnt need to do realPath logic here. Considering this involves more changes than previously expected, do you mind if i take it over?
There was a problem hiding this comment.
I don't mind you taking over.
I think this could easily be fixed by always setting originalPath to the original path regardless of the result of realpath. If originalPath is present it's from the cache, otherwise resolve the path using realpath
|
Superseded by #26377 |
Fixes #26273
/r=@sheetalkamat