For import fix, prefer symlink over a real path#20395
Conversation
9206617 to
751ad7b
Compare
751ad7b to
52322bd
Compare
| */ | ||
| export interface ResolvedModuleFull extends ResolvedModule { | ||
| /* @internal */ | ||
| readonly originalPath: string | undefined; |
There was a problem hiding this comment.
you would want to add check for originalPath in moduleResolutionIsEqualTo as well
There was a problem hiding this comment.
Why is originalPath not just optional? Also why is it in ResolvedModuleFull and not in ResolvedModule?
There was a problem hiding this comment.
The only reason we have both ResolvedModule and ResolvedModuleFull is backwards compatibility. Since this is an internal property it doesn't really matter where it goes.
There was a problem hiding this comment.
Why is it then string|undefined.. Looking at the changes it seems like it could be made optional instead to avoid setting it to undefined in all the other cases except 1
| */ | ||
| export interface ResolvedModuleFull extends ResolvedModule { | ||
| /* @internal */ | ||
| readonly originalPath: string | undefined; |
There was a problem hiding this comment.
Why is originalPath not just optional? Also why is it in ResolvedModuleFull and not in ResolvedModule?
| const { baseUrl, paths, rootDirs } = options; | ||
| const choicesForEachExportingModule = mapIterator(arrayIterator(moduleSymbols), moduleSymbol => { | ||
| const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; | ||
| const moduleFileName = getOriginalModulePath(program, moduleSymbol.valueDeclaration.getSourceFile()); |
There was a problem hiding this comment.
Also this doesnt seem right... If the sourceFile2(from firstDefined call from getOriginalModulePath) which included this moduleSymbol is in different folder than the sourceFile this fix is asked for it might not be correct right? The sourceFile could refer to the symlinked location through some other node_modules symlink... (in the sense orignalFilePath for referencing this moduleSymbol could be different while resolving through sourceFile and sourceFile2?
There was a problem hiding this comment.
It's possible that there are multiple symlinks to the same module, although it's not technically wrong to use one or the other since it works either way; we could iterate over all known symlinks though and take the best, would that be better?
cf7ac60 to
0659892
Compare
5e3d176 to
095cdb9
Compare
| const resolved = nodeLoadModuleByRelativeName(extensions, candidate, failedLookupLocations, /*onlyRecordFailures*/ false, state, /*considerPackageJson*/ true); | ||
| // Treat explicit "node_modules" import as an external library import. | ||
| return resolved && toSearchResult({ resolved, isExternalLibraryImport: contains(parts, "node_modules") }); | ||
| return resolved && toSearchResult({ resolved, originalPath: undefined, isExternalLibraryImport: contains(parts, "node_modules") }); |
There was a problem hiding this comment.
Do not set originalPath property
|
@sheetalkamat Good to go? |
Fixes #19893
Replaces #19910