-
-
Notifications
You must be signed in to change notification settings - Fork 757
fix: use default runtime to export default interop and keep empty import for externals #12530
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
Conversation
…ort for externals
✅ Deploy Preview for rspack canceled.
|
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.
Pull request overview
This PR fixes interop handling for default exports from CommonJS modules and preserves empty import statements for externals. The changes ensure that when exporting a default from a CJS module, the appropriate runtime helper is invoked, and side-effect imports (like import 'fs') are not removed during optimization.
- Adds logic to call
default_exported()runtime when the symbol being exported is the default access symbol for interop - Preserves empty import statements (side-effect imports) for external modules during the optimization pass
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/rspack_plugin_esm_library/src/link.rs |
Implements the core fix: adds condition to invoke default_exported() for default access symbols (lines 1067-1070) and skips removal of empty imports for externals (lines 1597-1600) |
tests/rspack-test/esmOutputCases/interop/export-cjs-default/index.js |
Test case for default export interop without type:module |
tests/rspack-test/esmOutputCases/interop/export-cjs-default/foo.cjs |
Test fixture that exports a primitive value (42) via module.exports |
tests/rspack-test/esmOutputCases/interop/export-cjs-default/__snapshots__/esm.snap.txt |
Expected output showing __webpack_require__.n() and function invocation for interop |
tests/rspack-test/esmOutputCases/interop/export-cjs-default-type-module/package.json |
Package config specifying type:module |
tests/rspack-test/esmOutputCases/interop/export-cjs-default-type-module/index.js |
Test case for default export interop with type:module |
tests/rspack-test/esmOutputCases/interop/export-cjs-default-type-module/foo.cjs |
Test fixture (same as non-type-module version) |
tests/rspack-test/esmOutputCases/interop/export-cjs-default-type-module/__snapshots__/esm.snap.txt |
Expected output without interop runtime (direct export) |
tests/rspack-test/esmOutputCases/externals/empty-import/rspack.config.js |
Config marking 'fs' as external with module type |
tests/rspack-test/esmOutputCases/externals/empty-import/index.js |
Test case with side-effect import of external |
tests/rspack-test/esmOutputCases/externals/empty-import/__snapshots__/esm.snap.txt |
Expected output preserving the empty import statement |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/rspack-test/esmOutputCases/interop/export-cjs-default-type-module/index.js
Show resolved
Hide resolved
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
📦 Binary Size-limit
❌ Size increased by 384bytes from 47.87MB to 47.87MB (⬆️0.00%) |
CodSpeed Performance ReportMerging #12530 will not alter performanceComparing Summary
Footnotes
|
Summary
When export interop default, should invoke the default access runtime.
Keep empty external import
Related links
Checklist