New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: separately export __proto__
for compatibility with CJS Transpiler Re-exports
#5380
Conversation
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/5377 Notice: 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
resolves #5377
Description