Add "preserveSymlinks" option #16661
Conversation
ghost
requested a review
from 
|
|
||
| // @filename: /linked/index.d.ts | ||
| // @symlink: /app/node_modules/linked/index.d.ts,/app/node_modules/linked2/index.d.ts | ||
| export { real } from "real"; |
There was a problem hiding this comment.
Would it make sense to write a unit test (possible replacing this)? We seem to test similar behavior in src/harness/unittests/moduleResolution.ts
There was a problem hiding this comment.
Added a test to moduleResolution.ts. I think the compiler test is valuable though, since it covers some assumptions we have about the effects of resolving / not resolving symlinks.
| @@ -39,6 +39,7 @@ | |||
| // "typeRoots": [], /* List of folders to include type definitions from. */ | |||
| // "types": [], /* Type declaration files to be included in compilation. */ | |||
| // "allowSyntheticDefaultImports": true, /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */ | |||
| @@ -188,7 +188,10 @@ namespace ts { | |||
|
|
|||
| let resolvedTypeReferenceDirective: ResolvedTypeReferenceDirective | undefined; | |||
| if (resolved) { | |||
| resolved = realpath(resolved, host, traceEnabled); | |||
| if (!options.preserveSymlinks) { | |||
| resolved = realpath(resolved, host, traceEnabled); | |||
There was a problem hiding this comment.
Can you add a doc-comment describing what ModuleResolutionHost.realpath (called by realpath) is supposed to do? It's not obvious to me from the name that it is supposed to resolve symbolic links.
There was a problem hiding this comment.
realpath is declared optional but oyu never check for its functionality. tsc should check if the current host supports realpath, and we should probably be resilient or throw if realpath isn't available.
Also, I think that realpath should be camelCase to be consistent with the rest of the APIs.
There was a problem hiding this comment.
The first action taken by realpath here is to test if (!host.realpath) return path;
I can change the name of the local function but wouldn't changing the name of the host method break existing hosts?
ghost
mentioned this pull request
|
cool |
Fixes #14760
If this option is turned on, we will not resolve symlinks for imports from node_modules. (We already don't resolve them outside.)
--resolveSymlinks falseto be like webpack.