-
-
Notifications
You must be signed in to change notification settings - Fork 787
Populate buildMeta and buildInfo on ConsumeSharedPlugin using fallbacks #11621
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
Open
ScriptedAlchemy
wants to merge
7
commits into
main
Choose a base branch
from
fix/consume-shared-plugin-compilation-errors
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
565d5c2
fix: resolve compilation errors in consume shared plugin
ScriptedAlchemy 312a1a2
test: add consume-nested test case for container-1-5
ScriptedAlchemy e5b6597
Merge branch 'main' into fix/consume-shared-plugin-compilation-errors
ScriptedAlchemy a3187a5
Merge branch 'main' into fix/consume-shared-plugin-compilation-errors
ScriptedAlchemy ae398d9
fix(plugin_mf): validate fallback, warn when missing, reduce module_g…
ScriptedAlchemy a01c551
Merge branch 'main' into fix/consume-shared-plugin-compilation-errors
ScriptedAlchemy fd7e174
Merge branch 'main' into fix/consume-shared-plugin-compilation-errors
ScriptedAlchemy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,10 +9,10 @@ use regex::Regex; | |||||||||||||
use rspack_cacheable::cacheable; | ||||||||||||||
use rspack_core::{ | ||||||||||||||
BoxModule, ChunkUkey, Compilation, CompilationAdditionalTreeRuntimeRequirements, | ||||||||||||||
CompilationParams, CompilerThisCompilation, Context, DependencyCategory, DependencyType, | ||||||||||||||
ModuleExt, ModuleFactoryCreateData, NormalModuleCreateData, NormalModuleFactoryCreateModule, | ||||||||||||||
NormalModuleFactoryFactorize, Plugin, ResolveOptionsWithDependencyType, ResolveResult, Resolver, | ||||||||||||||
RuntimeGlobals, | ||||||||||||||
CompilationFinishModules, CompilationParams, CompilerThisCompilation, Context, DependenciesBlock, | ||||||||||||||
DependencyCategory, DependencyType, ModuleExt, ModuleFactoryCreateData, NormalModuleCreateData, | ||||||||||||||
NormalModuleFactoryCreateModule, NormalModuleFactoryFactorize, Plugin, | ||||||||||||||
ResolveOptionsWithDependencyType, ResolveResult, Resolver, RuntimeGlobals, | ||||||||||||||
}; | ||||||||||||||
use rspack_error::{Diagnostic, Result, error}; | ||||||||||||||
use rspack_fs::ReadableFileSystem; | ||||||||||||||
|
@@ -397,6 +397,106 @@ async fn this_compilation( | |||||||||||||
Ok(()) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#[plugin_hook(CompilationFinishModules for ConsumeSharedPlugin, stage = 10)] | ||||||||||||||
async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> { | ||||||||||||||
// Add finishModules hook to copy buildMeta/buildInfo from fallback modules before webpack's export analysis | ||||||||||||||
// This follows webpack's pattern used by FlagDependencyExportsPlugin and InferAsyncModulesPlugin | ||||||||||||||
// We use finishModules with high priority stage to ensure buildMeta is available before other plugins process exports | ||||||||||||||
// Based on webpack's Compilation.js: finishModules (line 2833) runs before seal (line 2920) | ||||||||||||||
|
||||||||||||||
let module_graph = compilation.get_module_graph(); | ||||||||||||||
|
||||||||||||||
// Iterate through all modules to find ConsumeShared modules | ||||||||||||||
let mut consume_shared_modules = Vec::new(); | ||||||||||||||
for (module_id, module) in module_graph.modules() { | ||||||||||||||
// 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() { | ||||||||||||||
consume_shared_modules.push(module_id); | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
// Process each ConsumeShared module | ||||||||||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||||||||
if let Some(module) = module_graph.module_by_identifier(&module_id) { | ||||||||||||||
if let Some(consume_shared) = module.as_any().downcast_ref::<ConsumeSharedModule>() { | ||||||||||||||
// Find the fallback dependency | ||||||||||||||
let mut fallback_id = None; | ||||||||||||||
|
||||||||||||||
if consume_shared.get_options().eager { | ||||||||||||||
// For eager mode, get the fallback directly from dependencies | ||||||||||||||
for dep_id in module.get_dependencies() { | ||||||||||||||
if let Some(dep) = module_graph.dependency_by_id(dep_id) | ||||||||||||||
&& matches!(dep.dependency_type(), DependencyType::ConsumeSharedFallback) | ||||||||||||||
{ | ||||||||||||||
fallback_id = module_graph | ||||||||||||||
.module_identifier_by_dependency_id(dep_id) | ||||||||||||||
.copied(); | ||||||||||||||
break; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} else { | ||||||||||||||
// For async mode, get it from the async dependencies block | ||||||||||||||
for block_id in module.get_blocks() { | ||||||||||||||
if let Some(block) = module_graph.block_by_id(block_id) { | ||||||||||||||
for dep_id in block.get_dependencies() { | ||||||||||||||
if let Some(dep) = module_graph.dependency_by_id(dep_id) | ||||||||||||||
&& matches!(dep.dependency_type(), DependencyType::ConsumeSharedFallback) | ||||||||||||||
{ | ||||||||||||||
fallback_id = module_graph | ||||||||||||||
.module_identifier_by_dependency_id(dep_id) | ||||||||||||||
.copied(); | ||||||||||||||
break; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
if fallback_id.is_some() { | ||||||||||||||
break; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
fallback_id | ||||||||||||||
} else { | ||||||||||||||
None | ||||||||||||||
} | ||||||||||||||
} else { | ||||||||||||||
None | ||||||||||||||
} | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
// Copy metadata from fallback to ConsumeShared module | ||||||||||||||
if let Some(fallback_id) = fallback_module_id { | ||||||||||||||
let (fallback_meta, fallback_info) = { | ||||||||||||||
let module_graph = compilation.get_module_graph(); | ||||||||||||||
if let Some(fallback_module) = module_graph.module_by_identifier(&fallback_id) { | ||||||||||||||
( | ||||||||||||||
fallback_module.build_meta().clone(), | ||||||||||||||
fallback_module.build_info().clone(), | ||||||||||||||
) | ||||||||||||||
} else { | ||||||||||||||
(Default::default(), Default::default()) | ||||||||||||||
} | ||||||||||||||
}; | ||||||||||||||
ScriptedAlchemy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
// Update the ConsumeShared module with fallback's metadata | ||||||||||||||
let mut module_graph_mut = compilation.get_module_graph_mut(); | ||||||||||||||
if let Some(consume_module) = module_graph_mut.module_by_identifier_mut(&module_id) { | ||||||||||||||
// Copy buildMeta and buildInfo following webpack's DelegatedModule pattern: this.buildMeta = { ...delegateData.buildMeta }; | ||||||||||||||
// This ensures ConsumeSharedModule inherits ESM/CJS detection (exportsType) and other optimization metadata | ||||||||||||||
*consume_module.build_meta_mut() = fallback_meta; | ||||||||||||||
*consume_module.build_info_mut() = fallback_info; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
Ok(()) | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#[plugin_hook(NormalModuleFactoryFactorize for ConsumeSharedPlugin)] | ||||||||||||||
async fn factorize(&self, data: &mut ModuleFactoryCreateData) -> Result<Option<BoxModule>> { | ||||||||||||||
let dep = data.dependencies[0] | ||||||||||||||
|
@@ -512,6 +612,10 @@ impl Plugin for ConsumeSharedPlugin { | |||||||||||||
.compilation_hooks | ||||||||||||||
.additional_tree_runtime_requirements | ||||||||||||||
.tap(additional_tree_runtime_requirements::new(self)); | ||||||||||||||
ctx | ||||||||||||||
.compilation_hooks | ||||||||||||||
.finish_modules | ||||||||||||||
.tap(finish_modules::new(self)); | ||||||||||||||
Ok(()) | ||||||||||||||
} | ||||||||||||||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 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.