fix: separately export __proto__ for compatibility with CJS Transpiler Re-exports#5380
fix: separately export __proto__ for compatibility with CJS Transpiler Re-exports#5380lukastaegert merged 12 commits intomasterfrom
__proto__ for compatibility with CJS Transpiler Re-exports#5380Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
__proto__ for compatibility with CJS Transpiler Re-exports__proto__ for compatibility with CJS Transpiler Re-exports
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#fix/5377Notice: Ensure you have installed Rust nightly. If you haven't installed it yet, please first see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust, then see https://rust-lang.github.io/rustup/concepts/channels.html to learn how to install Rust nightly. or load it into the REPL: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5380 +/- ##
=======================================
Coverage 98.81% 98.81%
=======================================
Files 232 232
Lines 9009 9013 +4
Branches 2351 2353 +2
=======================================
+ Hits 8902 8906 +4
Misses 46 46
Partials 61 61 ☔ View full report in Codecov by Sentry. |
|
Currently, the export block is not concise enough when re-exporting '*' from an external module. So, I am wondering whether introducing a flag to control it, such as |
lukastaegert
left a comment
There was a problem hiding this comment.
Looks good to me, great work!
So, I am wondering whether introducing a flag to control it, such as reExportProtoFromExternal
Yes, I agree. This really is an edge case that of course we should somehow be able to handle, but it adds a lot of overhead to 99% of libraries without any benefit.
output. reExportProtoFromExternal sounds like a good name to me, but of course such a feature would need documentation, which would create some overhead. I think it should be enabled by default for maximum compatibility, but the docs should strongly suggest to disable it as you very likely will not need it.
Do you want to have a go within this PR? Otherwise I think we can also release it as it is and tackle this in a separate PR.
OK, I will do it within this PR. |
5101ee6 to
8fecd35
Compare
e283511 to
e417abe
Compare
lukastaegert
left a comment
There was a problem hiding this comment.
Great work!
The only thing left to do is to add the CLI option to the separate list of CLI options:
In cli/help.md:
--preserveSymlinks Do not follow symlinks when resolving files
+ --no-reexportProtoFromExternal Ignore `__proto__` in star re-exports
--no-sanitizeFileName Do not replace invalid characters in file names
This text is a suggestion. Here, if the default is "true", we would list the negated option but sort it by its non-negated name. Descriptions should fit within 80 characters.
And this list is duplicated (unchanged!) into docs/command-line-interface/index.md.
|
Ok, I get it. Thanks for your guidance! |
|
This PR has been released as part of rollup@4.11.0. You can test it via |
original issue resolved by rollup/rollup#5380
original issue resolved by rollup/rollup#5380
original issue resolved by rollup/rollup#5380
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
resolves #5377
Description