From 3ee789a6a990bce9a8618039edab36a861379be6 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Wed, 18 Sep 2024 17:02:07 +0800 Subject: [PATCH] fix: remove critical in dep --- crates/node_binding/binding.d.ts | 2 +- .../node_binding/src/plugins/interceptor.rs | 6 +- .../src/context_module_factory.rs | 2 +- .../rspack_core/src/context_module_factory.rs | 98 ++++++++++++------- .../src/lib.rs | 49 +++++----- crates/rspack_plugin_ignore/src/lib.rs | 2 +- .../common_js_imports_parse_plugin.rs | 4 +- .../System.import/test.filter.js | 1 - .../context-replacement/a/test.filter.js | 2 +- .../context-replacement/b/test.filter.js | 2 +- 10 files changed, 101 insertions(+), 67 deletions(-) delete mode 100644 tests/webpack-test/configCases/context-replacement/System.import/test.filter.js diff --git a/crates/node_binding/binding.d.ts b/crates/node_binding/binding.d.ts index bf0e8e5e0577..6760667b5023 100644 --- a/crates/node_binding/binding.d.ts +++ b/crates/node_binding/binding.d.ts @@ -476,7 +476,7 @@ export interface JsContextModuleFactoryAfterResolveData { export interface JsContextModuleFactoryBeforeResolveData { context: string - request?: string + request: string regExp?: RawRegex recursive: boolean } diff --git a/crates/node_binding/src/plugins/interceptor.rs b/crates/node_binding/src/plugins/interceptor.rs index cba2abe61ff9..b04a07e5d799 100644 --- a/crates/node_binding/src/plugins/interceptor.rs +++ b/crates/node_binding/src/plugins/interceptor.rs @@ -1530,8 +1530,8 @@ impl ContextModuleFactoryBeforeResolve for ContextModuleFactoryBeforeResolveTap request: d.request, reg_exp, recursive: d.recursive, - // TODO - dependencies: vec![], + // TODO: fix + critical: true, }; Ok(BeforeResolveResult::Data(Box::new(data))) } @@ -1572,6 +1572,8 @@ impl ContextModuleFactoryAfterResolve for ContextModuleFactoryAfterResolveTap { None => None, }, recursive: d.recursive, + // TODO: fix + critical: true, }; Ok(AfterResolveResult::Data(Box::new(data))) } diff --git a/crates/rspack_binding_values/src/context_module_factory.rs b/crates/rspack_binding_values/src/context_module_factory.rs index 1e05ae18ad6b..ed9beb1848e6 100644 --- a/crates/rspack_binding_values/src/context_module_factory.rs +++ b/crates/rspack_binding_values/src/context_module_factory.rs @@ -6,7 +6,7 @@ use crate::RawRegex; #[napi(object)] pub struct JsContextModuleFactoryBeforeResolveData { pub context: String, - pub request: Option, + pub request: String, pub reg_exp: Option, pub recursive: bool, } diff --git a/crates/rspack_core/src/context_module_factory.rs b/crates/rspack_core/src/context_module_factory.rs index b5000d3cc5e8..e5c16824801c 100644 --- a/crates/rspack_core/src/context_module_factory.rs +++ b/crates/rspack_core/src/context_module_factory.rs @@ -7,26 +7,26 @@ use rspack_regex::RspackRegex; use tracing::instrument; use crate::{ - resolve, ContextModule, ContextModuleOptions, DependencyCategory, DependencyId, ErrorSpan, + resolve, BoxDependency, ContextModule, ContextModuleOptions, DependencyCategory, ErrorSpan, ModuleExt, ModuleFactory, ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, RawModule, ResolveArgs, ResolveOptionsWithDependencyType, ResolveResult, Resolver, ResolverFactory, SharedPluginDriver, }; -#[derive(Clone)] +#[derive(Debug)] pub enum BeforeResolveResult { Ignored, Data(Box), } -#[derive(Clone)] +#[derive(Debug)] pub struct BeforeResolveData { // context_info // resolve_options pub context: String, - pub request: Option, + pub request: String, // assertions - pub dependencies: Vec, + // dependencies // dependency_type // file_dependencies // missing_dependencies @@ -35,6 +35,10 @@ pub struct BeforeResolveData { // cacheable pub recursive: bool, pub reg_exp: Option, + // FIX: This field is used by ContextReplacementPlugin to ignore errors collected during the build phase of Context modules. + // In Webpack, the ContextModuleFactory's beforeResolve hook directly traverses dependencies in context and modifies the ContextDependency's critical field. + // Since Rspack currently has difficulty passing the dependencies field, an additional field is used to indicate whether to ignore the collected errors. + pub critical: bool, } #[derive(Clone)] @@ -43,7 +47,7 @@ pub enum AfterResolveResult { Data(Box), } -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct AfterResolveData { pub resource: Utf8PathBuf, pub context: String, @@ -66,6 +70,11 @@ pub struct AfterResolveData { // type_prefix: String, // category: String, // referenced_exports + + // FIX: This field is used by ContextReplacementPlugin to ignore errors collected during the build phase of Context modules. + // In Webpack, the ContextModuleFactory's beforeResolve hook directly traverses dependencies in context and modifies the ContextDependency's critical field. + // Since Rspack currently has difficulty passing the dependencies field, an additional field is used to indicate whether to ignore the collected errors. + pub critical: bool, } define_hook!(ContextModuleFactoryBeforeResolve: AsyncSeriesWaterfall(data: BeforeResolveResult) -> BeforeResolveResult); @@ -88,19 +97,24 @@ pub struct ContextModuleFactory { impl ModuleFactory for ContextModuleFactory { #[instrument(name = "context_module_factory:create", skip_all)] async fn create(&self, data: &mut ModuleFactoryCreateData) -> Result { - if let Some(before_resolve_result) = self.before_resolve(data).await? { - return Ok(before_resolve_result); - } + match self.before_resolve(data).await? { + BeforeResolveResult::Ignored => return Ok(ModuleFactoryResult::default()), + BeforeResolveResult::Data(before_resolve_result) => { + let (factorize_result, context_module_options) = + self.resolve(data, before_resolve_result).await?; - let (factorize_result, mut context_module_options) = self.resolve(data).await?; + if let Some(context_module_options) = context_module_options { + if let Some(factorize_result) = self + .after_resolve(context_module_options, &mut data.dependencies) + .await? + { + return Ok(factorize_result); + } + } - if let Some(context_module_options) = context_module_options.as_mut() { - if let Some(factorize_result) = self.after_resolve(context_module_options).await? { - return Ok(factorize_result); + Ok(factorize_result) } } - - Ok(factorize_result) } } @@ -120,19 +134,18 @@ impl ContextModuleFactory { async fn before_resolve( &self, data: &mut ModuleFactoryCreateData, - ) -> Result> { + ) -> Result { let dependency = data.dependencies[0] - .as_context_dependency() + .as_context_dependency_mut() .expect("should be context dependency"); - let dependency_options = dependency.options(); let before_resolve_data = BeforeResolveData { context: data.context.to_string(), - request: data.request().map(|r| r.to_string()), + request: dependency.request().to_string(), recursive: dependency_options.recursive, reg_exp: dependency_options.reg_exp.clone(), - dependencies: vec![*dependency.id()], + critical: true, }; match self @@ -142,10 +155,12 @@ impl ContextModuleFactory { .call(BeforeResolveResult::Data(Box::new(before_resolve_data))) .await? { - BeforeResolveResult::Ignored => Ok(Some(ModuleFactoryResult::default())), - BeforeResolveResult::Data(d) => { - data.context = d.context.into(); - Ok(None) + BeforeResolveResult::Ignored => Ok(BeforeResolveResult::Ignored), + BeforeResolveResult::Data(result) => { + if !result.critical { + *dependency.critical_mut() = None; + } + Ok(BeforeResolveResult::Data(result)) } } } @@ -163,6 +178,7 @@ impl ContextModuleFactory { async fn resolve( &self, data: &mut ModuleFactoryCreateData, + before_resolve_data: Box, ) -> Result<(ModuleFactoryResult, Option)> { let plugin_driver = &self.plugin_driver; let dependency = data.dependencies[0] @@ -170,8 +186,8 @@ impl ContextModuleFactory { .expect("should be context dependency"); let mut file_dependencies = Default::default(); let mut missing_dependencies = Default::default(); - // let context_dependencies = Default::default(); - let request = dependency.request(); + + let request = before_resolve_data.request; let (loader_request, specifier) = match request.rfind('!') { Some(idx) => { let mut loaders_prefix = String::new(); @@ -190,7 +206,7 @@ impl ContextModuleFactory { } else { loaders_request.split('!').collect() }; - let resource = &request[idx + 1..]; + let resource = request[idx + 1..].to_string(); let mut loader_result = Vec::with_capacity(loaders.len()); let loader_resolver = self.get_loader_resolver(); @@ -226,10 +242,10 @@ impl ContextModuleFactory { }; let resolve_args = ResolveArgs { - context: data.context.clone(), + context: before_resolve_data.context.into(), importer: data.issuer_identifier.as_ref(), issuer: data.issuer.as_deref(), - specifier, + specifier: specifier.as_str(), dependency_type: dependency.dependency_type(), dependency_category: dependency.category(), span: dependency @@ -246,6 +262,10 @@ impl ContextModuleFactory { let (module, context_module_options) = match resource_data { Ok(ResolveResult::Resource(resource)) => { + let mut dependency_options = dependency.options().clone(); + dependency_options.recursive = before_resolve_data.recursive; + dependency_options.reg_exp = before_resolve_data.reg_exp.clone(); + let options = ContextModuleOptions { addon: loader_request.to_string(), resource: resource.path, @@ -291,7 +311,8 @@ impl ContextModuleFactory { async fn after_resolve( &self, - context_module_options: &mut ContextModuleOptions, + mut context_module_options: ContextModuleOptions, + dependencies: &mut Vec, ) -> Result> { let context_options = &context_module_options.context_options; let after_resolve_data = AfterResolveData { @@ -300,6 +321,7 @@ impl ContextModuleFactory { request: context_options.request.clone(), reg_exp: context_options.reg_exp.clone(), recursive: context_options.recursive, + critical: true, }; match self @@ -310,14 +332,24 @@ impl ContextModuleFactory { .await? { AfterResolveResult::Ignored => Ok(Some(ModuleFactoryResult::default())), - AfterResolveResult::Data(d) => { - context_module_options.resource = d.resource; - context_module_options.context_options.reg_exp = d.reg_exp; + AfterResolveResult::Data(result) => { + context_module_options.resource = result.resource; + context_module_options.context_options.context = result.context; + context_module_options.context_options.reg_exp = result.reg_exp; + context_module_options.context_options.recursive = result.recursive; + + let dependency = dependencies[0] + .as_context_dependency_mut() + .expect("should be context dependency"); + if !result.critical { + *dependency.critical_mut() = None; + } let module = ContextModule::new( context_module_options.clone(), self.resolver_factory.clone(), ); + Ok(Some(ModuleFactoryResult::new_with_module(Box::new(module)))) } } diff --git a/crates/rspack_plugin_context_replacement/src/lib.rs b/crates/rspack_plugin_context_replacement/src/lib.rs index 7f7fe9ea0cb1..2eb7a7569776 100644 --- a/crates/rspack_plugin_context_replacement/src/lib.rs +++ b/crates/rspack_plugin_context_replacement/src/lib.rs @@ -5,6 +5,7 @@ use rspack_core::{ }; use rspack_error::Result; use rspack_hook::{plugin, plugin_hook}; +use rspack_paths::Utf8PathBuf; use rspack_regex::RspackRegex; pub struct ContextReplacementPluginOptions { @@ -37,25 +38,28 @@ impl ContextReplacementPlugin { #[plugin_hook(ContextModuleFactoryBeforeResolve for ContextReplacementPlugin)] async fn cmf_before_resolve(&self, mut result: BeforeResolveResult) -> Result { + println!("cmf_before_resolve = {:?}", &result); if let BeforeResolveResult::Data(data) = &mut result { - if let Some(request) = &data.request { - if self.resource_reg_exp.test(request) { - data.request = self.new_content_resource.clone(); - } - if let Some(new_content_recursive) = self.new_content_recursive { - data.recursive = new_content_recursive; - } - if let Some(new_content_reg_exp) = &self.new_content_reg_exp { - data.reg_exp = Some(new_content_reg_exp.clone()); + println!("request = {:?} self = {:#?}", &data.request, self); + if self.resource_reg_exp.test(&data.request) { + if let Some(new_content_resource) = &self.new_content_resource { + data.request = new_content_resource.clone(); } - // if let Some(new_content_callback) = &self.new_content_after_resolve_callback { - // // new_content_callback(&mut result).await?; - // } else { - // // for (const d of result.dependencies) { - // // if (d.critical) d.critical = false; - // // } - // } } + if let Some(new_content_recursive) = self.new_content_recursive { + data.recursive = new_content_recursive; + } + if let Some(new_content_reg_exp) = &self.new_content_reg_exp { + data.reg_exp = Some(new_content_reg_exp.clone()); + } + // if let Some(new_content_callback) = &self.new_content_after_resolve_callback { + // // new_content_callback(&mut result).await?; + // } else { + // for (const d of result.dependencies) { + // if (d.critical) d.critical = false; + // } + data.critical = false; + // } } Ok(result) @@ -70,11 +74,7 @@ async fn cmf_after_resolve(&self, mut result: AfterResolveResult) -> Result Result Result Ok(BeforeResolveResult::Ignored), BeforeResolveResult::Data(d) => { - if let Some(false) = self.check_ignore(d.request.as_deref(), &d.context).await { + if let Some(false) = self.check_ignore(Some(&d.request), &d.context).await { Ok(BeforeResolveResult::Ignored) } else { Ok(BeforeResolveResult::Data(d)) diff --git a/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs b/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs index 82f2d87df601..22ac0c289b5a 100644 --- a/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs +++ b/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs @@ -1,6 +1,6 @@ use rspack_core::{ - ConstDependency, ContextDependency, ContextMode, DependencyCategory, RealDependencyLocation, - SpanExt, + ConstDependency, ContextDependency, ContextMode, Dependency, DependencyCategory, + RealDependencyLocation, SpanExt, }; use rspack_core::{ContextNameSpaceObject, ContextOptions}; use rspack_error::{DiagnosticExt, Severity}; diff --git a/tests/webpack-test/configCases/context-replacement/System.import/test.filter.js b/tests/webpack-test/configCases/context-replacement/System.import/test.filter.js deleted file mode 100644 index 3be456dcd23c..000000000000 --- a/tests/webpack-test/configCases/context-replacement/System.import/test.filter.js +++ /dev/null @@ -1 +0,0 @@ -module.exports = () => {return false} \ No newline at end of file diff --git a/tests/webpack-test/configCases/context-replacement/a/test.filter.js b/tests/webpack-test/configCases/context-replacement/a/test.filter.js index 3be456dcd23c..7910c8152ef4 100644 --- a/tests/webpack-test/configCases/context-replacement/a/test.filter.js +++ b/tests/webpack-test/configCases/context-replacement/a/test.filter.js @@ -1 +1 @@ -module.exports = () => {return false} \ No newline at end of file +module.exports = () => { return false } \ No newline at end of file diff --git a/tests/webpack-test/configCases/context-replacement/b/test.filter.js b/tests/webpack-test/configCases/context-replacement/b/test.filter.js index 3be456dcd23c..7910c8152ef4 100644 --- a/tests/webpack-test/configCases/context-replacement/b/test.filter.js +++ b/tests/webpack-test/configCases/context-replacement/b/test.filter.js @@ -1 +1 @@ -module.exports = () => {return false} \ No newline at end of file +module.exports = () => { return false } \ No newline at end of file