-
-
Notifications
You must be signed in to change notification settings - Fork 746
fix: should use exported name of the imported module #12423
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
✅ 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 an issue with ESM module re-exports where the wrong name was being used for concatenated modules. The fix ensures that when re-exporting from a concatenated module, the code first looks into the export_map to find the exported name, then retrieves the internal name. The key change is to use the export info of the referenced module instead of the current module when determining the used name for re-exports.
Key changes:
- Updated logic to correctly resolve export names through the export_map for concatenated modules
- Changed to use referenced module's export info instead of current module's export info when determining used names
- Added test case for re-exporting a renamed export (
lib_local as lib4)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/rspack_plugin_esm_library/src/link.rs | Core fix: Updated re-export logic to use ref module's export info and properly resolve names through export_map for concatenated modules |
| tests/rspack-test/esmOutputCases/re-exports/deep-re-exports-esm/lib3.js | Added test case: exports local variable lib_local as renamed export lib4 |
| tests/rspack-test/esmOutputCases/re-exports/deep-re-exports-esm/index.js | Updated test assertions to verify the new lib4 export |
| tests/rspack-test/esmOutputCases/re-exports/deep-re-exports-esm/snapshots/esm.snap.txt | Updated snapshot showing correct output with lib_local as lib4 in exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
0312ed3 to
35ac638
Compare
Rsdoctor Bundle Diff AnalysisFound 5 project(s) in monorepo. 📁 react-10kPath:
📦 Download Diff Report: react-10k Bundle Diff 📁 react-1kPath:
📦 Download Diff Report: react-1k Bundle Diff 📁 romePath:
📦 Download Diff Report: rome Bundle Diff 📁 react-5kPath:
📦 Download Diff Report: react-5k Bundle Diff 📁 ui-componentsPath:
📦 Download Diff Report: ui-components Bundle Diff Generated by Rsdoctor GitHub Action |
📦 Binary Size-limit
❌ Size increased by 640bytes from 48.21MB to 48.21MB (⬆️0.00%) |
CodSpeed Performance ReportMerging #12423 will not alter performanceComparing Summary
|
Summary
related issue: web-infra-dev/rsbuild#6746
Should first look into the
export_mapfor concatenated modules, then find the internal name of it. Use export_info of ref module instead of current moduleRelated links
Checklist
Note
Resolve normal re-exports via the referenced module’s export info/export_map (including externals and concatenated modules), and add a test covering deep re-exports with a new lib4 export.
export_namefromget_target(item.export_info)and use the referenced module’s exports info to compute the used name (instead of the current module).export_nameasidswhen re-exporting from external modules.internal_nameby mappingexport_namethroughexport_mapbefore lookup.lib4, adjusting snapshot and assertions.Written by Cursor Bugbot for commit 35ac638. This will update automatically on new commits. Configure here.