fix: resolve compilation errors in consume shared plugin #11621
Conversation
- Add public get_options() method to ConsumeSharedModule for accessing private options field - Fix type mismatches between Ustr and Identifier in module graph operations - Add proper mutability declarations for module graph mutations - Collapse nested if statements to follow clippy style guidelines - Implement finishModules hook to copy build metadata from fallback modules
✅ Deploy Preview for rspack canceled.
|
github-actions
bot
added
the
release: bug fix
There was a problem hiding this comment.
Pull Request Overview
This PR fixes compilation errors in the rspack_plugin_mf crate by resolving type mismatches and private field access issues in the consume shared plugin functionality.
- Adds a new hook for processing ConsumeSharedModule instances during compilation to copy metadata from fallback modules
- Introduces a public API method to access ConsumeSharedModule options that were previously private
- Updates imports to include necessary types for module graph operations
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/rspack_plugin_mf/src/sharing/consume_shared_plugin.rs | Adds finish_modules hook implementation and updates imports to support module graph operations |
| crates/rspack_plugin_mf/src/sharing/consume_shared_module.rs | Adds public getter method for accessing ConsumeOptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Only process ConsumeSharedModule instances with fallback dependencies | ||
| if let Some(consume_shared) = module.as_any().downcast_ref::<ConsumeSharedModule>() { | ||
| // Check if this module has a fallback import | ||
| if consume_shared.get_options().import.is_some() { |
There was a problem hiding this comment.
The condition checks for import.is_some() but the logic processes modules regardless of whether they have fallback dependencies. Consider adding validation to ensure the module actually has fallback dependencies before adding it to the processing list.
Copilot uses AI. Check for mistakes.
| for module_id in consume_shared_modules { | ||
| let fallback_module_id = { | ||
| let module_graph = compilation.get_module_graph(); |
There was a problem hiding this comment.
The module graph is accessed multiple times within the loop (lines 424, 475, 487). Consider restructuring to minimize repeated access to the module graph for better performance.
| for module_id in consume_shared_modules { | |
| let fallback_module_id = { | |
| let module_graph = compilation.get_module_graph(); | |
| let module_graph = compilation.get_module_graph(); | |
| for module_id in consume_shared_modules { | |
| let fallback_module_id = { |
Copilot uses AI. Check for mistakes.
📦 Binary Size-limit
❌ Size increased by 7.75KB from 47.45MB to 47.45MB (⬆️0.02%) |
- Add test configuration for nested consume shared module scenarios - Include package dependencies and ESM/CJS module test setup
CodSpeed Performance ReportMerging #11621 will not alter performanceComparing Summary
|
…raph access; clippy cleanups - Process only modules with fallback import - Compute fallback meta with single graph borrow - Emit warning when fallback not found instead of defaulting - Collapse nested ifs, use Option::map per clippy
Summary