[Fix] preserveSymlinks: false ensure that files are realpathed
#197
Conversation
| { | ||
| "files": "test/resolver/nested_symlinks/mylib/*.js", | ||
| "rules": { | ||
| "no-throw-literal": 0, |
There was a problem hiding this comment.
Instead of throw 'async: no match'; you could just do throw new Error('async: no match');
There was a problem hiding this comment.
sure, but then the failure output includes a stack trace, which makes it harder to read :-)
|
I tested this in a PNPM repo and confirmed that |
|
This PR looks good to me. It also fixes my original issue with rollup-plugin-node-resolve. @ljharb let me know how I can help get this in. I'm not at all familiar with the preserve-symlinks-main behavior but I could learn! |
|
k, looked into nodejs/node#19911 and replicated its test case; it seems More test cases would be great, but also the appveyor Windows tests seem to be failing :-/ |
ljharb
force-pushed
the
preserve-symlinks
branch
from
c659407
to
3a82b41
Compare
ljharb
force-pushed
the
preserve-symlinks
branch
2 times, most recently
from
74849a1
to
f7da566
Compare
- [New] `core`: add `_debug_agent` core module, in node 1 through 7 - [Fix] `preserveSymlinks: false` ensure that files are realpathed (#197, #195) - [Refactor] make `maybeUnwrapSymlink` - [Meta] clean up license so github can detect it - [Dev Deps] update `tape` - [Tests] fix symlinks in windows/appveyor - [Tests] up to `node` `v12.7`, `v10.16`, `v8.16` - [Tests] gitignore file created in tests; remove it in test setup
|
Seems like this caused rollup/rollup-plugin-commonjs#400 :-/ hopefully we can roll forward instead of reverting |
|
😊 At least it wasn't a PATCH bump! |
Fixes #196.
This very well may not have the correct behavior of
--preserve-symlinks-main; I think we'll need some more tests before landing this.