Skip to content

Commit

Permalink
fix: remove critical in dep
Browse files Browse the repository at this point in the history
  • Loading branch information
SyMind committed Sep 18, 2024
1 parent 53f83b0 commit 3ee789a
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 67 deletions.
2 changes: 1 addition & 1 deletion crates/node_binding/binding.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ export interface JsContextModuleFactoryAfterResolveData {

export interface JsContextModuleFactoryBeforeResolveData {
context: string
request?: string
request: string
regExp?: RawRegex
recursive: boolean
}
Expand Down
6 changes: 4 additions & 2 deletions crates/node_binding/src/plugins/interceptor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
Expand Down Expand Up @@ -1572,6 +1572,8 @@ impl ContextModuleFactoryAfterResolve for ContextModuleFactoryAfterResolveTap {
None => None,
},
recursive: d.recursive,
// TODO: fix
critical: true,
};
Ok(AfterResolveResult::Data(Box::new(data)))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_binding_values/src/context_module_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::RawRegex;
#[napi(object)]
pub struct JsContextModuleFactoryBeforeResolveData {
pub context: String,
pub request: Option<String>,
pub request: String,
pub reg_exp: Option<RawRegex>,
pub recursive: bool,
}
Expand Down
98 changes: 65 additions & 33 deletions crates/rspack_core/src/context_module_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<BeforeResolveData>),
}

#[derive(Clone)]
#[derive(Debug)]
pub struct BeforeResolveData {
// context_info
// resolve_options
pub context: String,
pub request: Option<String>,
pub request: String,
// assertions
pub dependencies: Vec<DependencyId>,
// dependencies
// dependency_type
// file_dependencies
// missing_dependencies
Expand All @@ -35,6 +35,10 @@ pub struct BeforeResolveData {
// cacheable
pub recursive: bool,
pub reg_exp: Option<RspackRegex>,
// 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)]
Expand All @@ -43,7 +47,7 @@ pub enum AfterResolveResult {
Data(Box<AfterResolveData>),
}

#[derive(Clone)]
#[derive(Debug, Clone)]
pub struct AfterResolveData {
pub resource: Utf8PathBuf,
pub context: String,
Expand All @@ -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);
Expand All @@ -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<ModuleFactoryResult> {
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)
}
}

Expand All @@ -120,19 +134,18 @@ impl ContextModuleFactory {
async fn before_resolve(
&self,
data: &mut ModuleFactoryCreateData,
) -> Result<Option<ModuleFactoryResult>> {
) -> Result<BeforeResolveResult> {
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
Expand All @@ -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))
}
}
}
Expand All @@ -163,15 +178,16 @@ impl ContextModuleFactory {
async fn resolve(
&self,
data: &mut ModuleFactoryCreateData,
before_resolve_data: Box<BeforeResolveData>,
) -> Result<(ModuleFactoryResult, Option<ContextModuleOptions>)> {
let plugin_driver = &self.plugin_driver;
let dependency = data.dependencies[0]
.as_context_dependency()
.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();
Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -291,7 +311,8 @@ impl ContextModuleFactory {

async fn after_resolve(
&self,
context_module_options: &mut ContextModuleOptions,
mut context_module_options: ContextModuleOptions,
dependencies: &mut Vec<BoxDependency>,

Check failure on line 315 in crates/rspack_core/src/context_module_factory.rs

View workflow job for this annotation

GitHub Actions / Rust check

writing `&mut Vec` instead of `&mut [_]` involves a new object where a slice will do
) -> Result<Option<ModuleFactoryResult>> {
let context_options = &context_module_options.context_options;
let after_resolve_data = AfterResolveData {
Expand All @@ -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
Expand All @@ -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))))
}
}
Expand Down
49 changes: 25 additions & 24 deletions crates/rspack_plugin_context_replacement/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -37,25 +38,28 @@ impl ContextReplacementPlugin {

#[plugin_hook(ContextModuleFactoryBeforeResolve for ContextReplacementPlugin)]
async fn cmf_before_resolve(&self, mut result: BeforeResolveResult) -> Result<BeforeResolveResult> {
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)
Expand All @@ -70,11 +74,7 @@ async fn cmf_after_resolve(&self, mut result: AfterResolveResult) -> Result<Afte
{
data.resource = new_content_resource.clone().into();
} else {
// result.resource = join(
// /** @type {InputFileSystem} */ (compiler.inputFileSystem),
// result.resource,
// newContentResource
// );
data.resource = data.resource.join(Utf8PathBuf::from(new_content_resource));
}
}
if let Some(new_content_recursive) = self.new_content_recursive {
Expand All @@ -86,9 +86,10 @@ async fn cmf_after_resolve(&self, mut result: AfterResolveResult) -> Result<Afte
// if let Some(new_content_callback) = &self.new_content_callback {
// // new_content_callback(&mut result).await?;
// } else {
// // for (const d of result.dependencies) {
// // if (d.critical) d.critical = false;
// // }
// for (const d of result.dependencies) {
// if (d.critical) d.critical = false;
// }
data.critical = false;
// }
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_plugin_ignore/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ async fn cmf_before_resolve(&self, data: BeforeResolveResult) -> Result<BeforeRe
match data {
BeforeResolveResult::Ignored => 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))
Expand Down
Original file line number Diff line number Diff line change
@@ -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};
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = () => {return false}
module.exports = () => { return false }
Original file line number Diff line number Diff line change
@@ -1 +1 @@
module.exports = () => {return false}
module.exports = () => { return false }

0 comments on commit 3ee789a

Please sign in to comment.