-
-
Notifications
You must be signed in to change notification settings - Fork 724
fix: should render default exports for cjs entry and json entry #11860
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for rspack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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 the rendering of default exports for CommonJS (CJS) entries and JSON entries in module library output. The fix ensures that when using module library types, both CJS modules and JSON files generate appropriate default exports alongside named exports.
Key changes:
- Added export type detection to determine when to render default exports
- Refactored export rendering to support both named and default export patterns
- Added comprehensive test coverage for both module and modern-module library types with JSON and CJS entries
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
crates/rspack_plugin_library/src/module_library_plugin.rs |
Updated export rendering logic to handle different export types and generate default exports when needed |
crates/rspack_plugin_library/src/modern_module_library_plugin.rs |
Added export type detection and refactored export rendering functions to support both named and default exports |
tests/rspack-test/configCases/library/module-json-entry/* |
Added test case for module library type with JSON entry |
tests/rspack-test/configCases/library/modern-module-json-entry/* |
Added test case for modern-module library type with JSON entry |
tests/rspack-test/configCases/library/modern-module-cjs-entry/* |
Added test case for modern-module library type with CJS entry |
Comments suppressed due to low confidence (1)
crates/rspack_plugin_library/src/module_library_plugin.rs:1
- [nitpick] Missing space after '=' operator. Should be 'module.exports = {' for consistent formatting.
use std::hash::Hash;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
source.add(RawStringSource::from(exports_string)); | ||
} else if matches!(exports_type, ExportsType::Dynamic) { | ||
// compat for css entry | ||
render_as_default_export(&exports); |
Copilot
AI
Oct 13, 2025
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.
The result of render_as_default_export(&exports)
is not being used. This function returns a String that should be added to the source, but the return value is being discarded.
render_as_default_export(&exports); | |
source.add(RawStringSource::from(render_as_default_export(&exports))); |
Copilot uses AI. Check for mistakes.
source.add(RawStringSource::from(exports_string)); | ||
} else if matches!(exports_type, ExportsType::Dynamic) { | ||
// compat for css entry | ||
render_as_default_export(&exports); |
Copilot
AI
Oct 13, 2025
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.
The result of render_as_default_export(&exports)
is not being used. This function returns a String that should be added to the source, but the return value is being discarded.
render_as_default_export(&exports); | |
source.add(RawStringSource::from(render_as_default_export(&exports))); |
Copilot uses AI. Check for mistakes.
} else { | ||
local.clone() |
Copilot
AI
Oct 13, 2025
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.
When there's no exported name, the local variable name is used directly in the object literal, but this could create invalid JavaScript if the local name contains special characters or is a reserved word. Consider using property shorthand syntax or proper escaping.
Copilot uses AI. Check for mistakes.
📦 Binary Size-limit
🎉 Size decreased by 278.88KB from 47.89MB to 47.62MB (⬇️0.57%) |
CodSpeed Performance ReportMerging #11860 will not alter performanceComparing Summary
|
Summary
Render cjs entry and default export for json entry module
See tests for detail
Related links
Checklist