From 115b8a2308e4a74f6b94b54b01e2428fa3c60904 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Fri, 1 Nov 2024 18:51:07 +0800 Subject: [PATCH 1/9] todo --- .../src/compilation/entries.rs | 20 +- .../src/compilation/mod.rs | 16 ++ .../src/context_module_factory.rs | 63 +++--- .../rspack_binding_values/src/dependency.rs | 198 ++++++++++++------ .../src/dependency_block.rs | 178 ++++++++++++++++ crates/rspack_binding_values/src/lib.rs | 2 + crates/rspack_binding_values/src/module.rs | 65 +----- .../src/compiler/make/repair/factorize.rs | 7 +- .../src/compiler/make/repair/mod.rs | 7 +- .../make/repair/process_dependencies.rs | 1 + .../src/compiler/module_executor/entry.rs | 1 + 11 files changed, 393 insertions(+), 165 deletions(-) create mode 100644 crates/rspack_binding_values/src/dependency_block.rs diff --git a/crates/rspack_binding_values/src/compilation/entries.rs b/crates/rspack_binding_values/src/compilation/entries.rs index 03be0ca4ca4b..d94442d714c3 100644 --- a/crates/rspack_binding_values/src/compilation/entries.rs +++ b/crates/rspack_binding_values/src/compilation/entries.rs @@ -2,7 +2,9 @@ use napi_derive::napi; use rspack_core::{ChunkLoading, Compilation, EntryData, EntryOptions, EntryRuntime}; use rspack_napi::napi::bindgen_prelude::*; -use crate::{dependency::JsDependency, entry::JsEntryOptions, library::JsLibraryOptions}; +use crate::{ + dependency::JsDependency, entry::JsEntryOptions, library::JsLibraryOptions, JsDependencyWrapper, +}; #[napi] pub struct EntryOptionsDTO(EntryOptions); @@ -163,12 +165,12 @@ impl From for EntryData { dependencies: value .dependencies .into_iter() - .map(|dep| *dep.id()) + .map(|dep| dep.dependency_id) .collect::>(), include_dependencies: value .include_dependencies .into_iter() - .map(|dep| *dep.id()) + .map(|dep| dep.dependency_id) .collect::>(), options: value.options.into(), } @@ -183,8 +185,8 @@ pub struct EntryDataDTO { #[napi] impl EntryDataDTO { - #[napi(getter)] - pub fn dependencies(&'static self) -> Vec { + #[napi(getter, ts_return_type = "JsDependency[]")] + pub fn dependencies(&'static self) -> Vec { let module_graph = self.compilation.get_module_graph(); self .entry_data @@ -193,13 +195,13 @@ impl EntryDataDTO { .map(|dependency_id| { #[allow(clippy::unwrap_used)] let dep = module_graph.dependency_by_id(dependency_id).unwrap(); - JsDependency::new(dep) + JsDependencyWrapper::new(dep.as_ref(), self.compilation.id()) }) .collect::>() } - #[napi(getter)] - pub fn include_dependencies(&'static self) -> Vec { + #[napi(getter, ts_return_type = "JsDependency[]")] + pub fn include_dependencies(&'static self) -> Vec { let module_graph = self.compilation.get_module_graph(); self .entry_data @@ -208,7 +210,7 @@ impl EntryDataDTO { .map(|dependency_id| { #[allow(clippy::unwrap_used)] let dep = module_graph.dependency_by_id(dependency_id).unwrap(); - JsDependency::new(dep) + JsDependencyWrapper::new(dep.as_ref(), self.compilation.id()) }) .collect::>() } diff --git a/crates/rspack_binding_values/src/compilation/mod.rs b/crates/rspack_binding_values/src/compilation/mod.rs index 58e9935246ae..54a2810bfcdf 100644 --- a/crates/rspack_binding_values/src/compilation/mod.rs +++ b/crates/rspack_binding_values/src/compilation/mod.rs @@ -634,6 +634,22 @@ impl JsCompilationWrapper { Self(NonNull::new(compilation as *const Compilation as *mut Compilation).unwrap()) } + pub fn compilation_by_id(compilation_id: &CompilationId) -> Option<&mut Compilation> { + COMPILATION_INSTANCE_REFS.with(|ref_cell| { + let refs = ref_cell.borrow_mut(); + match refs.get(compilation_id) { + Some(r) => match r.from_napi_value() { + Ok(mut instance) => { + let compilation = unsafe { instance.0.as_mut() }; + Some(compilation) + } + Err(_) => None, + }, + None => None, + } + }) + } + pub fn cleanup_last_compilation(compilation_id: CompilationId) { COMPILATION_INSTANCE_REFS.with(|ref_cell| { let mut refs = ref_cell.borrow_mut(); diff --git a/crates/rspack_binding_values/src/context_module_factory.rs b/crates/rspack_binding_values/src/context_module_factory.rs index 3dbff18b36c2..3b3b51897834 100644 --- a/crates/rspack_binding_values/src/context_module_factory.rs +++ b/crates/rspack_binding_values/src/context_module_factory.rs @@ -2,9 +2,9 @@ use napi::bindgen_prelude::{ ClassInstance, Either, FromNapiValue, ToNapiValue, TypeName, ValidateNapiValue, }; use napi_derive::napi; -use rspack_core::{AfterResolveData, BeforeResolveData}; +use rspack_core::{AfterResolveData, BeforeResolveData, CompilationId}; -use crate::{JsDependencyMut, RawRegex}; +use crate::{JsDependencyWrapper, RawRegex}; #[napi] pub struct JsContextModuleFactoryBeforeResolveData(Box); @@ -111,43 +111,46 @@ pub type JsContextModuleFactoryBeforeResolveResult = Either; #[napi] -pub struct JsContextModuleFactoryAfterResolveData(Box); +pub struct JsContextModuleFactoryAfterResolveData { + compilation_id: CompilationId, + inner: Box, +} #[napi] impl JsContextModuleFactoryAfterResolveData { #[napi(getter)] pub fn resource(&self) -> &str { - self.0.resource.as_str() + self.inner.resource.as_str() } #[napi(setter)] pub fn set_resource(&mut self, resource: String) { - self.0.resource = resource.into(); + self.inner.resource = resource.into(); } #[napi(getter)] pub fn context(&self) -> &str { - &self.0.context + &self.inner.context } #[napi(setter)] pub fn set_context(&mut self, context: String) { - self.0.context = context; + self.inner.context = context; } #[napi(getter)] pub fn request(&self) -> &str { - &self.0.request + &self.inner.request } #[napi(setter)] pub fn set_request(&mut self, request: String) { - self.0.request = request; + self.inner.request = request; } #[napi(getter)] pub fn reg_exp(&self) -> Either { - match &self.0.reg_exp { + match &self.inner.reg_exp { Some(r) => Either::A(r.clone().into()), None => Either::B(()), } @@ -155,7 +158,7 @@ impl JsContextModuleFactoryAfterResolveData { #[napi(setter)] pub fn set_reg_exp(&mut self, raw_reg_exp: Either) { - self.0.reg_exp = match raw_reg_exp { + self.inner.reg_exp = match raw_reg_exp { Either::A(raw_reg_exp) => match raw_reg_exp.try_into() { Ok(reg_exp) => Some(reg_exp), Err(_) => None, @@ -166,34 +169,40 @@ impl JsContextModuleFactoryAfterResolveData { #[napi(getter)] pub fn recursive(&self) -> bool { - self.0.recursive + self.inner.recursive } #[napi(setter)] pub fn set_recursive(&mut self, recursive: bool) { - self.0.recursive = recursive; + self.inner.recursive = recursive; } - #[napi(getter)] - pub fn dependencies(&mut self) -> Vec { + #[napi(getter, ts_return_type = "JsDependency[]")] + pub fn dependencies(&self) -> Vec { self - .0 + .inner .dependencies - .iter_mut() - .map(JsDependencyMut::new) + .iter() + .map(|dep| JsDependencyWrapper::new(dep.as_ref(), self.compilation_id)) .collect::>() } } -pub struct JsContextModuleFactoryAfterResolveDataWrapper(Box); +pub struct JsContextModuleFactoryAfterResolveDataWrapper { + compilation_id: CompilationId, + inner: Box, +} impl JsContextModuleFactoryAfterResolveDataWrapper { - pub fn new(data: Box) -> Self { - JsContextModuleFactoryAfterResolveDataWrapper(data) + pub fn new(data: Box, compilation_id: CompilationId) -> Self { + JsContextModuleFactoryAfterResolveDataWrapper { + compilation_id, + inner: data, + } } pub fn take(self) -> Box { - self.0 + self.inner } } @@ -206,7 +215,10 @@ impl FromNapiValue for JsContextModuleFactoryAfterResolveDataWrapper { as FromNapiValue>::from_napi_value( env, napi_val, )?; - Ok(Self(instance.0.clone())) + Ok(Self { + compilation_id: instance.compilation_id, + inner: instance.inner.clone(), + }) } } @@ -215,7 +227,10 @@ impl ToNapiValue for JsContextModuleFactoryAfterResolveDataWrapper { env: napi::sys::napi_env, val: Self, ) -> napi::Result { - let js_val = JsContextModuleFactoryAfterResolveData(val.0); + let js_val = JsContextModuleFactoryAfterResolveData { + compilation_id: val.compilation_id, + inner: val.inner, + }; ToNapiValue::to_napi_value(env, js_val) } } diff --git a/crates/rspack_binding_values/src/dependency.rs b/crates/rspack_binding_values/src/dependency.rs index 1d85b7e7218b..f9dcefc098a1 100644 --- a/crates/rspack_binding_values/src/dependency.rs +++ b/crates/rspack_binding_values/src/dependency.rs @@ -1,108 +1,178 @@ +use std::{cell::RefCell, ptr::NonNull}; + +use napi::{ + bindgen_prelude::{ClassInstance, ToNapiValue}, + Env, +}; use napi_derive::napi; -use rspack_core::{BoxDependency, DependencyId}; +use rspack_core::{CompilationId, Dependency, DependencyId}; +use rspack_napi::OneShotRef; +use rustc_hash::FxHashMap as HashMap; + +use crate::JsCompilationWrapper; // JsDependency allows JS-side access to a Dependency instance that has already // been processed and stored in the Compilation. #[napi] -pub struct JsDependency(&'static BoxDependency); +pub struct JsDependency { + pub(crate) compilation_id: CompilationId, + pub(crate) dependency_id: DependencyId, + pub(crate) dependency: NonNull, +} impl JsDependency { - pub(crate) fn new(dependency: &BoxDependency) -> Self { - // SAFETY: - // The lifetime of the &mut BoxDependency reference is extended to 'static. - // Accessing fields and methods on the Rust object from the JS side after the Rust object's - // lifetime has ended is undefined behavior, which we currently disregard. - let dependency = - unsafe { std::mem::transmute::<&BoxDependency, &'static BoxDependency>(dependency) }; - Self(dependency) + fn as_ref(&mut self) -> napi::Result<&dyn Dependency> { + let dependency = unsafe { self.dependency.as_ref() }; + if *dependency.id() == self.dependency_id { + return Ok(dependency); + } + + if let Some(compilation) = JsCompilationWrapper::compilation_by_id(&self.compilation_id) { + let module_graph = compilation.get_module_graph(); + if let Some(dependency) = module_graph.dependency_by_id(&self.dependency_id) { + self.dependency = + NonNull::new(dependency.as_ref() as *const dyn Dependency as *mut dyn Dependency) + .unwrap(); + return Ok(unsafe { self.dependency.as_ref() }); + } + } + + Err(napi::Error::from_reason(format!( + "Unable to access dependency with id = {:?} now. The dependency have been removed on the Rust side.", + self.dependency_id + ))) } - pub(crate) fn id(&self) -> &DependencyId { - self.0.id() + fn as_mut(&mut self) -> napi::Result<&mut dyn Dependency> { + let dependency = unsafe { self.dependency.as_mut() }; + if *dependency.id() == self.dependency_id { + return Ok(dependency); + } + + Err(napi::Error::from_reason(format!( + "Unable to access dependency with id = {:?} now. The dependency have been removed on the Rust side.", + self.dependency_id + ))) } } #[napi] impl JsDependency { #[napi(getter)] - pub fn get_type(&self) -> &str { - self.0.dependency_type().as_str() + pub fn get_type(&mut self) -> napi::Result<&str> { + let dependency = self.as_ref()?; + + Ok(dependency.dependency_type().as_str()) } #[napi(getter)] - pub fn category(&self) -> &str { - self.0.category().as_str() + pub fn category(&mut self) -> napi::Result<&str> { + let dependency = self.as_ref()?; + + Ok(dependency.category().as_str()) } #[napi(getter)] - pub fn request(&self) -> napi::Either<&str, ()> { - match self.0.as_module_dependency() { + pub fn request(&mut self) -> napi::Result> { + let dependency = self.as_ref()?; + + Ok(match dependency.as_module_dependency() { Some(dep) => napi::Either::A(dep.request()), None => napi::Either::B(()), - } + }) } #[napi(getter)] - pub fn critical(&self) -> bool { - match self.0.as_context_dependency() { + pub fn critical(&mut self) -> napi::Result { + let dependency = self.as_ref()?; + + Ok(match dependency.as_context_dependency() { Some(dep) => dep.critical().is_some(), None => false, + }) + } + + #[napi(setter)] + pub fn set_critical(&mut self, val: bool) -> napi::Result<()> { + let dependency = self.as_mut()?; + + if let Some(dep) = dependency.as_context_dependency_mut() { + let critical = dep.critical_mut(); + if !val { + *critical = None; + } } + Ok(()) } } -// JsDependency represents a Dependency instance that is currently being processed. -// It is in the make stage and has not yet been added to the Compilation. -#[napi] -pub struct JsDependencyMut(&'static mut BoxDependency); - -impl JsDependencyMut { - pub(crate) fn new(dependency: &mut BoxDependency) -> Self { - // SAFETY: - // The lifetime of the &mut BoxDependency reference is extended to 'static. - // Accessing fields and methods on the Rust object from the JS side after the Rust object's - // lifetime has ended is undefined behavior, which we currently disregard. - let dependency = - unsafe { std::mem::transmute::<&mut BoxDependency, &'static mut BoxDependency>(dependency) }; - Self(dependency) - } +type DependencyInstanceRefs = HashMap>>; + +type DependencyInstanceRefsByCompilationId = + RefCell>; + +thread_local! { + static DEPENDENCY_INSTANCE_REFS: DependencyInstanceRefsByCompilationId = Default::default(); } -#[napi] -impl JsDependencyMut { - #[napi(getter)] - pub fn get_type(&self) -> &str { - self.0.dependency_type().as_str() - } +pub struct JsDependencyWrapper { + compilation_id: CompilationId, + dependency_id: DependencyId, + dependency: NonNull, +} - #[napi(getter)] - pub fn category(&self) -> &str { - self.0.category().as_str() - } +impl JsDependencyWrapper { + pub fn new(dependency: &dyn Dependency, compilation_id: CompilationId) -> Self { + let dependency_id = *dependency.id(); - #[napi(getter)] - pub fn request(&self) -> napi::Either<&str, ()> { - match self.0.as_module_dependency() { - Some(dep) => napi::Either::A(dep.request()), - None => napi::Either::B(()), + #[allow(clippy::unwrap_used)] + Self { + compilation_id, + dependency_id, + dependency: NonNull::new(dependency as *const dyn Dependency as *mut dyn Dependency).unwrap(), } } - #[napi(getter)] - pub fn critical(&self) -> bool { - match self.0.as_context_dependency() { - Some(dep) => dep.critical().is_some(), - None => false, - } + pub fn cleanup_last_compilation(compilation_id: CompilationId) { + DEPENDENCY_INSTANCE_REFS.with(|refs| { + let mut refs_by_compilation_id = refs.borrow_mut(); + refs_by_compilation_id.remove(&compilation_id) + }); } +} - #[napi(setter)] - pub fn set_critical(&mut self, val: bool) { - if let Some(dep) = self.0.as_context_dependency_mut() { - let critical = dep.critical_mut(); - if !val { - *critical = None; +impl ToNapiValue for JsDependencyWrapper { + unsafe fn to_napi_value( + env: napi::sys::napi_env, + val: Self, + ) -> napi::Result { + DEPENDENCY_INSTANCE_REFS.with(|refs| { + let mut refs_by_compilation_id = refs.borrow_mut(); + let entry = refs_by_compilation_id.entry(val.compilation_id); + let refs = match entry { + std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), + std::collections::hash_map::Entry::Vacant(entry) => { + let refs = HashMap::default(); + entry.insert(refs) + } + }; + + match refs.entry(val.dependency_id) { + std::collections::hash_map::Entry::Occupied(occupied_entry) => { + let r = occupied_entry.get(); + ToNapiValue::to_napi_value(env, r) + } + std::collections::hash_map::Entry::Vacant(vacant_entry) => { + let instance: ClassInstance = JsDependency { + compilation_id: val.compilation_id, + dependency_id: val.dependency_id, + dependency: val.dependency, + } + .into_instance(Env::from_raw(env))?; + let r = vacant_entry.insert(OneShotRef::new(env, instance)?); + ToNapiValue::to_napi_value(env, r) + } } - } + }) } } diff --git a/crates/rspack_binding_values/src/dependency_block.rs b/crates/rspack_binding_values/src/dependency_block.rs new file mode 100644 index 000000000000..4ecbceff9163 --- /dev/null +++ b/crates/rspack_binding_values/src/dependency_block.rs @@ -0,0 +1,178 @@ +use std::{cell::RefCell, ptr::NonNull, sync::Arc}; + +use napi_derive::napi; +use rspack_collections::IdentifierMap; +use rspack_core::{ + AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, Compilation, CompilationId, + DependenciesBlock, Module, ModuleGraph, ModuleIdentifier, RuntimeModuleStage, SourceType, +}; +use rspack_napi::{napi::bindgen_prelude::*, threadsafe_function::ThreadsafeFunction, OneShotRef}; +use rspack_plugin_runtime::RuntimeModuleFromJs; +use rspack_util::source_map::SourceMapKind; +use rustc_hash::FxHashMap as HashMap; + +use super::{JsCompatSource, ToJsCompatSource}; +use crate::{JsChunk, JsCodegenerationResults, JsCompilationWrapper, JsDependency, JsDependencyWrapper}; + +#[derive(Default)] +#[napi(object)] +pub struct JsFactoryMeta { + pub side_effect_free: Option, +} + +#[napi] +pub struct JsDependenciesBlock { + compilation_id: CompilationId, + block_id: AsyncDependenciesBlockIdentifier, + block: NonNull, +} + +impl JsDependenciesBlock { + pub fn new(block: &AsyncDependenciesBlock, compilation_id: CompilationId) -> Self { + #[allow(clippy::unwrap_used)] + Self { + compilation_id, + block_id: block.identifier(), + block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock).unwrap(), + } + } + + fn as_ref(&mut self) -> napi::Result<&AsyncDependenciesBlock> { + let block = unsafe { self.block.as_ref() }; + if block.identifier() == self.block_id { + return Ok(block); + } + + if let Some(compilation) = JsCompilationWrapper::compilation_by_id(&self.compilation_id) { + let module_graph = compilation.get_module_graph(); + if let Some(block) = module_graph.block_by_id(&self.block_id) { + self.block = + NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) + .unwrap(); + return Ok(unsafe { self.block.as_ref() }); + } + } + + Err(napi::Error::from_reason(format!( + "Unable to access block with id = {:?} now. The block have been removed on the Rust side.", + self.block_id + ))) + } + + fn as_mut(&mut self) -> napi::Result<&mut AsyncDependenciesBlock> { + let block = unsafe { self.block.as_mut() }; + if block.identifier() == self.block_id { + return Ok(block); + } + + Err(napi::Error::from_reason(format!( + "Unable to access block with id = {:?} now. The block have been removed on the Rust side.", + self.block_id + ))) + } +} + +#[napi] +impl JsDependenciesBlock { + #[napi(getter, ts_return_type = "JsDependency")] + pub fn dependencies(&self) -> Vec { + let compilation = unsafe { self.compilation.as_ref() }; + + let module_graph = compilation.get_module_graph(); + let block = self.block(&module_graph); + block + .get_dependencies() + .iter() + .filter_map(|dependency_id| { + module_graph + .dependency_by_id(dependency_id) + .map(|dep| JsDependencyWrapper::new(dep, self.compilation)) + }) + .collect::>() + } + + #[napi(getter)] + pub fn blocks(&self) -> Vec { + let compilation = unsafe { self.compilation.as_ref() }; + + let module_graph = compilation.get_module_graph(); + let block = self.block(&module_graph); + let blocks = block.get_blocks(); + blocks + .iter() + .cloned() + .map(|block_id| JsDependenciesBlock::new(block_id, self.compilation.as_ptr())) + .collect::>() + } +} + +type BlockInstanceRefs = + HashMap>>; + +type BlockInstanceRefsByCompilationId = RefCell>; + +thread_local! { + static BLOCK_INSTANCE_REFS: BlockInstanceRefsByCompilationId = Default::default(); +} + +pub struct JsDependenciesBlockWrapper { + compilation_id: CompilationId, + block_id: AsyncDependenciesBlockIdentifier, + block: NonNull, +} + +impl JsDependenciesBlockWrapper { + pub fn new(block: &dyn DependenciesBlock, compilation_id: CompilationId) -> Self { + let block_id = *block.id + + #[allow(clippy::unwrap_used)] + Self { + compilation_id, + dependency_id, + dependency: NonNull::new(dependency as *const dyn Dependency as *mut dyn Dependency).unwrap(), + } + } + + pub fn cleanup_last_compilation(compilation_id: CompilationId) { + BLOCK_INSTANCE_REFS.with(|refs| { + let mut refs_by_compilation_id = refs.borrow_mut(); + refs_by_compilation_id.remove(&compilation_id) + }); + } +} + +impl ToNapiValue for JsDependenciesBlockWrapper { + unsafe fn to_napi_value( + env: napi::sys::napi_env, + val: Self, + ) -> napi::Result { + BLOCK_INSTANCE_REFS.with(|refs| { + let mut refs_by_compilation_id = refs.borrow_mut(); + let entry = refs_by_compilation_id.entry(val.compilation_id); + let refs = match entry { + std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), + std::collections::hash_map::Entry::Vacant(entry) => { + let refs = HashMap::default(); + entry.insert(refs) + } + }; + + match refs.entry(val.dependency_id) { + std::collections::hash_map::Entry::Occupied(occupied_entry) => { + let r = occupied_entry.get(); + ToNapiValue::to_napi_value(env, r) + } + std::collections::hash_map::Entry::Vacant(vacant_entry) => { + let instance: ClassInstance = JsDependency { + compilation_id: val.compilation_id, + dependency_id: val.dependency_id, + dependency: val.dependency, + } + .into_instance(Env::from_raw(env))?; + let r = vacant_entry.insert(OneShotRef::new(env, instance)?); + ToNapiValue::to_napi_value(env, r) + } + } + }) + } +} diff --git a/crates/rspack_binding_values/src/lib.rs b/crates/rspack_binding_values/src/lib.rs index deeeea820328..c4c215331b06 100644 --- a/crates/rspack_binding_values/src/lib.rs +++ b/crates/rspack_binding_values/src/lib.rs @@ -8,6 +8,7 @@ mod codegen_result; mod compilation; mod context_module_factory; mod dependency; +mod dependency_block; mod filename; mod html; mod identifier; @@ -33,6 +34,7 @@ pub use codegen_result::*; pub use compilation::*; pub use context_module_factory::*; pub use dependency::*; +pub use dependency_block::*; pub use filename::*; pub use html::*; pub use module::*; diff --git a/crates/rspack_binding_values/src/module.rs b/crates/rspack_binding_values/src/module.rs index afcfb2110e81..a15dda4bb5a4 100644 --- a/crates/rspack_binding_values/src/module.rs +++ b/crates/rspack_binding_values/src/module.rs @@ -3,8 +3,7 @@ use std::{cell::RefCell, ptr::NonNull, sync::Arc}; use napi_derive::napi; use rspack_collections::IdentifierMap; use rspack_core::{ - AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, Compilation, CompilationId, - DependenciesBlock, Module, ModuleGraph, ModuleIdentifier, RuntimeModuleStage, SourceType, + Compilation, CompilationId, Module, ModuleIdentifier, RuntimeModuleStage, SourceType, }; use rspack_napi::{napi::bindgen_prelude::*, threadsafe_function::ThreadsafeFunction, OneShotRef}; use rspack_plugin_runtime::RuntimeModuleFromJs; @@ -12,7 +11,7 @@ use rspack_util::source_map::SourceMapKind; use rustc_hash::FxHashMap as HashMap; use super::{JsCompatSource, ToJsCompatSource}; -use crate::{JsChunk, JsCodegenerationResults, JsDependency}; +use crate::{JsChunk, JsCodegenerationResults, JsDependenciesBlock}; #[derive(Default)] #[napi(object)] @@ -20,65 +19,6 @@ pub struct JsFactoryMeta { pub side_effect_free: Option, } -#[napi] -pub struct JsDependenciesBlock { - block_id: AsyncDependenciesBlockIdentifier, - compilation: NonNull, -} - -impl JsDependenciesBlock { - pub fn new(block_id: AsyncDependenciesBlockIdentifier, compilation: *const Compilation) -> Self { - #[allow(clippy::unwrap_used)] - Self { - block_id, - compilation: NonNull::new(compilation as *mut Compilation).unwrap(), - } - } - - fn block<'a>(&self, module_graph: &'a ModuleGraph) -> &'a AsyncDependenciesBlock { - module_graph.block_by_id(&self.block_id).unwrap_or_else(|| { - panic!( - "Cannot find block with id = {:?}. It might have been removed on the Rust side.", - self.block_id - ) - }) - } -} - -#[napi] -impl JsDependenciesBlock { - #[napi(getter)] - pub fn dependencies(&self) -> Vec { - let compilation = unsafe { self.compilation.as_ref() }; - - let module_graph = compilation.get_module_graph(); - let block = self.block(&module_graph); - block - .get_dependencies() - .iter() - .filter_map(|dependency_id| { - module_graph - .dependency_by_id(dependency_id) - .map(JsDependency::new) - }) - .collect::>() - } - - #[napi(getter)] - pub fn blocks(&self) -> Vec { - let compilation = unsafe { self.compilation.as_ref() }; - - let module_graph = compilation.get_module_graph(); - let block = self.block(&module_graph); - let blocks = block.get_blocks(); - blocks - .iter() - .cloned() - .map(|block_id| JsDependenciesBlock::new(block_id, self.compilation.as_ptr())) - .collect::>() - } -} - #[napi] pub struct JsModule { identifier: ModuleIdentifier, @@ -342,7 +282,6 @@ unsafe impl Send for JsModuleWrapper {} impl JsModuleWrapper { pub fn new(module: &dyn Module, compilation: Option<&Compilation>) -> Self { - #[allow(clippy::not_unsafe_ptr_arg_deref)] let identifier = module.identifier(); #[allow(clippy::unwrap_used)] diff --git a/crates/rspack_core/src/compiler/make/repair/factorize.rs b/crates/rspack_core/src/compiler/make/repair/factorize.rs index a86e7ef11aac..9039c3cfd544 100644 --- a/crates/rspack_core/src/compiler/make/repair/factorize.rs +++ b/crates/rspack_core/src/compiler/make/repair/factorize.rs @@ -8,13 +8,14 @@ use super::{add::AddTask, MakeTaskContext}; use crate::{ module_graph::ModuleGraphModule, utils::task_loop::{Task, TaskResult, TaskType}, - BoxDependency, CompilerOptions, Context, ExportInfoData, ExportsInfoData, ModuleFactory, - ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, ModuleLayer, ModuleProfile, - Resolve, + BoxDependency, CompilationId, CompilerOptions, Context, ExportInfoData, ExportsInfoData, + ModuleFactory, ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, ModuleLayer, + ModuleProfile, Resolve, }; #[derive(Debug)] pub struct FactorizeTask { + pub compilation_id: CompilationId, pub module_factory: Arc, pub original_module_identifier: Option, pub original_module_source: Option, diff --git a/crates/rspack_core/src/compiler/make/repair/mod.rs b/crates/rspack_core/src/compiler/make/repair/mod.rs index 361a3625cf5c..5bf123d0d60c 100644 --- a/crates/rspack_core/src/compiler/make/repair/mod.rs +++ b/crates/rspack_core/src/compiler/make/repair/mod.rs @@ -14,12 +14,13 @@ use crate::{ module_graph::{ModuleGraph, ModuleGraphPartial}, old_cache::Cache as OldCache, utils::task_loop::{run_task_loop, Task}, - BuildDependency, Compilation, CompilerOptions, DependencyType, Module, ModuleFactory, - ModuleProfile, NormalModuleSource, ResolverFactory, SharedPluginDriver, + BuildDependency, Compilation, CompilationId, CompilerOptions, DependencyType, Module, + ModuleFactory, ModuleProfile, NormalModuleSource, ResolverFactory, SharedPluginDriver, }; pub struct MakeTaskContext { // compilation info + pub compilation_id: CompilationId, pub plugin_driver: SharedPluginDriver, pub buildtime_plugin_driver: SharedPluginDriver, pub fs: Arc, @@ -35,6 +36,7 @@ pub struct MakeTaskContext { impl MakeTaskContext { pub fn new(compilation: &Compilation, artifact: MakeArtifact) -> Self { Self { + compilation_id: compilation.id(), plugin_driver: compilation.plugin_driver.clone(), buildtime_plugin_driver: compilation.buildtime_plugin_driver.clone(), compiler_options: compilation.options.clone(), @@ -123,6 +125,7 @@ pub fn repair( } }); Some(Box::new(factorize::FactorizeTask { + compilation_id: compilation.id(), module_factory: compilation.get_dependency_factory(dependency), original_module_identifier: parent_module_identifier, original_module_source, diff --git a/crates/rspack_core/src/compiler/make/repair/process_dependencies.rs b/crates/rspack_core/src/compiler/make/repair/process_dependencies.rs index 818633f36323..896c5fec551e 100644 --- a/crates/rspack_core/src/compiler/make/repair/process_dependencies.rs +++ b/crates/rspack_core/src/compiler/make/repair/process_dependencies.rs @@ -96,6 +96,7 @@ impl Task for ProcessDependenciesTask { }) .clone(); res.push(Box::new(FactorizeTask { + compilation_id: context.compilation_id, module_factory, original_module_identifier: Some(module.identifier()), original_module_context: module.get_context(), diff --git a/crates/rspack_core/src/compiler/module_executor/entry.rs b/crates/rspack_core/src/compiler/module_executor/entry.rs index 8e97910d9383..d0802eb715b9 100644 --- a/crates/rspack_core/src/compiler/module_executor/entry.rs +++ b/crates/rspack_core/src/compiler/module_executor/entry.rs @@ -22,6 +22,7 @@ impl Task for EntryTask { module_graph.add_dependency(dep.clone()); Ok(vec![Box::new(FactorizeTask { + compilation_id: context.compilation_id, module_factory: context .dependency_factories .get(dep.dependency_type()) From 925e73d930ae1a8ede160e236269af77f3a84e88 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Sat, 2 Nov 2024 20:30:00 +0800 Subject: [PATCH 2/9] feat: JsDependenciesBlock --- .../src/dependency_block.rs | 108 +++++++++--------- 1 file changed, 56 insertions(+), 52 deletions(-) diff --git a/crates/rspack_binding_values/src/dependency_block.rs b/crates/rspack_binding_values/src/dependency_block.rs index 4ecbceff9163..fd8f163c6299 100644 --- a/crates/rspack_binding_values/src/dependency_block.rs +++ b/crates/rspack_binding_values/src/dependency_block.rs @@ -1,18 +1,14 @@ -use std::{cell::RefCell, ptr::NonNull, sync::Arc}; +use std::{cell::RefCell, ptr::NonNull}; use napi_derive::napi; -use rspack_collections::IdentifierMap; use rspack_core::{ - AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, Compilation, CompilationId, - DependenciesBlock, Module, ModuleGraph, ModuleIdentifier, RuntimeModuleStage, SourceType, + AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, CompilationId, DependenciesBlock, + ModuleGraph, }; -use rspack_napi::{napi::bindgen_prelude::*, threadsafe_function::ThreadsafeFunction, OneShotRef}; -use rspack_plugin_runtime::RuntimeModuleFromJs; -use rspack_util::source_map::SourceMapKind; +use rspack_napi::{napi::bindgen_prelude::*, OneShotRef}; use rustc_hash::FxHashMap as HashMap; -use super::{JsCompatSource, ToJsCompatSource}; -use crate::{JsChunk, JsCodegenerationResults, JsCompilationWrapper, JsDependency, JsDependencyWrapper}; +use crate::{JsCompilationWrapper, JsDependencyWrapper}; #[derive(Default)] #[napi(object)] @@ -31,25 +27,27 @@ impl JsDependenciesBlock { pub fn new(block: &AsyncDependenciesBlock, compilation_id: CompilationId) -> Self { #[allow(clippy::unwrap_used)] Self { - compilation_id, + compilation_id, block_id: block.identifier(), - block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock).unwrap(), + block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) + .unwrap(), } } - fn as_ref(&mut self) -> napi::Result<&AsyncDependenciesBlock> { - let block = unsafe { self.block.as_ref() }; - if block.identifier() == self.block_id { - return Ok(block); - } - + fn as_ref(&mut self) -> napi::Result<(&AsyncDependenciesBlock, ModuleGraph)> { if let Some(compilation) = JsCompilationWrapper::compilation_by_id(&self.compilation_id) { let module_graph = compilation.get_module_graph(); + + let block = unsafe { self.block.as_ref() }; + if block.identifier() == self.block_id { + return Ok((block, module_graph)); + } + if let Some(block) = module_graph.block_by_id(&self.block_id) { self.block = NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) .unwrap(); - return Ok(unsafe { self.block.as_ref() }); + return Ok((unsafe { self.block.as_ref() }, module_graph)); } } @@ -75,34 +73,39 @@ impl JsDependenciesBlock { #[napi] impl JsDependenciesBlock { #[napi(getter, ts_return_type = "JsDependency")] - pub fn dependencies(&self) -> Vec { - let compilation = unsafe { self.compilation.as_ref() }; - - let module_graph = compilation.get_module_graph(); - let block = self.block(&module_graph); - block - .get_dependencies() - .iter() - .filter_map(|dependency_id| { - module_graph - .dependency_by_id(dependency_id) - .map(|dep| JsDependencyWrapper::new(dep, self.compilation)) - }) - .collect::>() + pub fn dependencies(&mut self) -> Result> { + let compilation_id = self.compilation_id; + let (block, module_graph) = self.as_ref()?; + + Ok( + block + .get_dependencies() + .iter() + .filter_map(|dependency_id| { + module_graph + .dependency_by_id(dependency_id) + .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation_id)) + }) + .collect::>(), + ) } #[napi(getter)] - pub fn blocks(&self) -> Vec { - let compilation = unsafe { self.compilation.as_ref() }; - - let module_graph = compilation.get_module_graph(); - let block = self.block(&module_graph); - let blocks = block.get_blocks(); - blocks - .iter() - .cloned() - .map(|block_id| JsDependenciesBlock::new(block_id, self.compilation.as_ptr())) - .collect::>() + pub fn blocks(&mut self) -> Result> { + let compilation_id = self.compilation_id; + let (block, module_graph) = self.as_ref()?; + + Ok( + block + .get_blocks() + .iter() + .filter_map(|block_id| { + module_graph + .block_by_id(block_id) + .map(|block| JsDependenciesBlock::new(block, compilation_id)) + }) + .collect::>(), + ) } } @@ -118,18 +121,19 @@ thread_local! { pub struct JsDependenciesBlockWrapper { compilation_id: CompilationId, block_id: AsyncDependenciesBlockIdentifier, - block: NonNull, + block: NonNull, } impl JsDependenciesBlockWrapper { - pub fn new(block: &dyn DependenciesBlock, compilation_id: CompilationId) -> Self { - let block_id = *block.id + pub fn new(block: &AsyncDependenciesBlock, compilation_id: CompilationId) -> Self { + let block_id = block.identifier(); #[allow(clippy::unwrap_used)] Self { compilation_id, - dependency_id, - dependency: NonNull::new(dependency as *const dyn Dependency as *mut dyn Dependency).unwrap(), + block_id, + block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) + .unwrap(), } } @@ -157,16 +161,16 @@ impl ToNapiValue for JsDependenciesBlockWrapper { } }; - match refs.entry(val.dependency_id) { + match refs.entry(val.block_id) { std::collections::hash_map::Entry::Occupied(occupied_entry) => { let r = occupied_entry.get(); ToNapiValue::to_napi_value(env, r) } std::collections::hash_map::Entry::Vacant(vacant_entry) => { - let instance: ClassInstance = JsDependency { + let instance: ClassInstance = JsDependenciesBlock { compilation_id: val.compilation_id, - dependency_id: val.dependency_id, - dependency: val.dependency, + block_id: val.block_id, + block: val.block, } .into_instance(Env::from_raw(env))?; let r = vacant_entry.insert(OneShotRef::new(env, instance)?); From 98dff8f0e82f4a617fa64cd91962986a30b1ea06 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 4 Nov 2024 11:18:10 +0800 Subject: [PATCH 3/9] a --- .../rspack_binding_values/src/dependency.rs | 11 ++- .../src/dependency_block.rs | 78 +++++++------------ 2 files changed, 34 insertions(+), 55 deletions(-) diff --git a/crates/rspack_binding_values/src/dependency.rs b/crates/rspack_binding_values/src/dependency.rs index f9dcefc098a1..bf712182fc84 100644 --- a/crates/rspack_binding_values/src/dependency.rs +++ b/crates/rspack_binding_values/src/dependency.rs @@ -5,7 +5,7 @@ use napi::{ Env, }; use napi_derive::napi; -use rspack_core::{CompilationId, Dependency, DependencyId}; +use rspack_core::{Compilation, CompilationId, Dependency, DependencyId}; use rspack_napi::OneShotRef; use rustc_hash::FxHashMap as HashMap; @@ -116,20 +116,21 @@ thread_local! { } pub struct JsDependencyWrapper { - compilation_id: CompilationId, dependency_id: DependencyId, dependency: NonNull, + compilation: Option>, } impl JsDependencyWrapper { - pub fn new(dependency: &dyn Dependency, compilation_id: CompilationId) -> Self { + pub fn new(dependency: &dyn Dependency, compilation: Option<&Compilation>) -> Self { let dependency_id = *dependency.id(); #[allow(clippy::unwrap_used)] Self { - compilation_id, dependency_id, dependency: NonNull::new(dependency as *const dyn Dependency as *mut dyn Dependency).unwrap(), + compilation: compilation + .map(|c| NonNull::new(c as *const Compilation as *mut Compilation).unwrap()), } } @@ -147,6 +148,8 @@ impl ToNapiValue for JsDependencyWrapper { val: Self, ) -> napi::Result { DEPENDENCY_INSTANCE_REFS.with(|refs| { + let compilation = unsafe { val.compilation.as_ref() }; + let mut refs_by_compilation_id = refs.borrow_mut(); let entry = refs_by_compilation_id.entry(val.compilation_id); let refs = match entry { diff --git a/crates/rspack_binding_values/src/dependency_block.rs b/crates/rspack_binding_values/src/dependency_block.rs index fd8f163c6299..a9f8145cc193 100644 --- a/crates/rspack_binding_values/src/dependency_block.rs +++ b/crates/rspack_binding_values/src/dependency_block.rs @@ -2,13 +2,13 @@ use std::{cell::RefCell, ptr::NonNull}; use napi_derive::napi; use rspack_core::{ - AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, CompilationId, DependenciesBlock, - ModuleGraph, + AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, Compilation, CompilationId, + DependenciesBlock, ModuleGraph, }; use rspack_napi::{napi::bindgen_prelude::*, OneShotRef}; use rustc_hash::FxHashMap as HashMap; -use crate::{JsCompilationWrapper, JsDependencyWrapper}; +use crate::JsDependencyWrapper; #[derive(Default)] #[napi(object)] @@ -18,49 +18,26 @@ pub struct JsFactoryMeta { #[napi] pub struct JsDependenciesBlock { - compilation_id: CompilationId, block_id: AsyncDependenciesBlockIdentifier, block: NonNull, + compilation: NonNull, } impl JsDependenciesBlock { - pub fn new(block: &AsyncDependenciesBlock, compilation_id: CompilationId) -> Self { - #[allow(clippy::unwrap_used)] - Self { - compilation_id, - block_id: block.identifier(), - block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) - .unwrap(), - } - } - - fn as_ref(&mut self) -> napi::Result<(&AsyncDependenciesBlock, ModuleGraph)> { - if let Some(compilation) = JsCompilationWrapper::compilation_by_id(&self.compilation_id) { - let module_graph = compilation.get_module_graph(); - - let block = unsafe { self.block.as_ref() }; - if block.identifier() == self.block_id { - return Ok((block, module_graph)); - } + fn as_ref(&mut self) -> napi::Result<(&AsyncDependenciesBlock, &Compilation, ModuleGraph)> { + let compilation = unsafe { self.compilation.as_ref() }; + let module_graph = compilation.get_module_graph(); - if let Some(block) = module_graph.block_by_id(&self.block_id) { - self.block = - NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) - .unwrap(); - return Ok((unsafe { self.block.as_ref() }, module_graph)); - } + let block = unsafe { self.block.as_ref() }; + if block.identifier() == self.block_id { + return Ok((block, compilation, module_graph)); } - Err(napi::Error::from_reason(format!( - "Unable to access block with id = {:?} now. The block have been removed on the Rust side.", - self.block_id - ))) - } - - fn as_mut(&mut self) -> napi::Result<&mut AsyncDependenciesBlock> { - let block = unsafe { self.block.as_mut() }; - if block.identifier() == self.block_id { - return Ok(block); + if let Some(block) = module_graph.block_by_id(&self.block_id) { + self.block = + NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) + .unwrap(); + return Ok((unsafe { self.block.as_ref() }, compilation, module_graph)); } Err(napi::Error::from_reason(format!( @@ -74,8 +51,7 @@ impl JsDependenciesBlock { impl JsDependenciesBlock { #[napi(getter, ts_return_type = "JsDependency")] pub fn dependencies(&mut self) -> Result> { - let compilation_id = self.compilation_id; - let (block, module_graph) = self.as_ref()?; + let (block, compilation, module_graph) = self.as_ref()?; Ok( block @@ -84,16 +60,15 @@ impl JsDependenciesBlock { .filter_map(|dependency_id| { module_graph .dependency_by_id(dependency_id) - .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation_id)) + .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation)) }) .collect::>(), ) } - #[napi(getter)] - pub fn blocks(&mut self) -> Result> { - let compilation_id = self.compilation_id; - let (block, module_graph) = self.as_ref()?; + #[napi(getter, ts_return_type = "JsDependenciesBlock")] + pub fn blocks(&mut self) -> Result> { + let (block, compilation, module_graph) = self.as_ref()?; Ok( block @@ -102,7 +77,7 @@ impl JsDependenciesBlock { .filter_map(|block_id| { module_graph .block_by_id(block_id) - .map(|block| JsDependenciesBlock::new(block, compilation_id)) + .map(|block| JsDependenciesBlockWrapper::new(block, compilation)) }) .collect::>(), ) @@ -119,21 +94,21 @@ thread_local! { } pub struct JsDependenciesBlockWrapper { - compilation_id: CompilationId, block_id: AsyncDependenciesBlockIdentifier, block: NonNull, + compilation: NonNull, } impl JsDependenciesBlockWrapper { - pub fn new(block: &AsyncDependenciesBlock, compilation_id: CompilationId) -> Self { + pub fn new(block: &AsyncDependenciesBlock, compilation: &Compilation) -> Self { let block_id = block.identifier(); #[allow(clippy::unwrap_used)] Self { - compilation_id, block_id, block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) .unwrap(), + compilation: NonNull::new(compilation as *const Compilation as *mut Compilation).unwrap(), } } @@ -151,8 +126,9 @@ impl ToNapiValue for JsDependenciesBlockWrapper { val: Self, ) -> napi::Result { BLOCK_INSTANCE_REFS.with(|refs| { + let compilation = unsafe { val.compilation.as_ref() }; let mut refs_by_compilation_id = refs.borrow_mut(); - let entry = refs_by_compilation_id.entry(val.compilation_id); + let entry = refs_by_compilation_id.entry(compilation.id()); let refs = match entry { std::collections::hash_map::Entry::Occupied(entry) => entry.into_mut(), std::collections::hash_map::Entry::Vacant(entry) => { @@ -168,9 +144,9 @@ impl ToNapiValue for JsDependenciesBlockWrapper { } std::collections::hash_map::Entry::Vacant(vacant_entry) => { let instance: ClassInstance = JsDependenciesBlock { - compilation_id: val.compilation_id, block_id: val.block_id, block: val.block, + compilation: val.compilation, } .into_instance(Env::from_raw(env))?; let r = vacant_entry.insert(OneShotRef::new(env, instance)?); From 2929a00cd9eb4b835ad27b956ea4b91f042b9aef Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 12:37:54 +0800 Subject: [PATCH 4/9] fix --- .../src/compilation/entries.rs | 4 +- .../src/compilation/mod.rs | 16 ------ .../src/context_module_factory.rs | 54 +++++++------------ .../rspack_binding_values/src/dependency.rs | 52 +++++++++++------- .../src/dependency_block.rs | 25 ++++----- crates/rspack_binding_values/src/module.rs | 26 +++++---- .../rspack_core/src/context_module_factory.rs | 12 +++-- 7 files changed, 86 insertions(+), 103 deletions(-) diff --git a/crates/rspack_binding_values/src/compilation/entries.rs b/crates/rspack_binding_values/src/compilation/entries.rs index f6545148ef0c..5a13301c0909 100644 --- a/crates/rspack_binding_values/src/compilation/entries.rs +++ b/crates/rspack_binding_values/src/compilation/entries.rs @@ -195,7 +195,7 @@ impl EntryDataDTO { .map(|dependency_id| { #[allow(clippy::unwrap_used)] let dep = module_graph.dependency_by_id(dependency_id).unwrap(); - JsDependencyWrapper::new(dep.as_ref(), self.compilation.id()) + JsDependencyWrapper::new(dep.as_ref(), self.compilation.id(), Some(self.compilation)) }) .collect::>() } @@ -210,7 +210,7 @@ impl EntryDataDTO { .map(|dependency_id| { #[allow(clippy::unwrap_used)] let dep = module_graph.dependency_by_id(dependency_id).unwrap(); - JsDependencyWrapper::new(dep.as_ref(), self.compilation.id()) + JsDependencyWrapper::new(dep.as_ref(), self.compilation.id(), Some(self.compilation)) }) .collect::>() } diff --git a/crates/rspack_binding_values/src/compilation/mod.rs b/crates/rspack_binding_values/src/compilation/mod.rs index fbfc900f0207..6fa1338bffe9 100644 --- a/crates/rspack_binding_values/src/compilation/mod.rs +++ b/crates/rspack_binding_values/src/compilation/mod.rs @@ -725,22 +725,6 @@ impl JsCompilationWrapper { } } - pub fn compilation_by_id(compilation_id: &CompilationId) -> Option<&mut Compilation> { - COMPILATION_INSTANCE_REFS.with(|ref_cell| { - let refs = ref_cell.borrow_mut(); - match refs.get(compilation_id) { - Some(r) => match r.from_napi_value() { - Ok(mut instance) => { - let compilation = unsafe { instance.0.as_mut() }; - Some(compilation) - } - Err(_) => None, - }, - None => None, - } - }) - } - pub fn cleanup_last_compilation(compilation_id: CompilationId) { COMPILATION_INSTANCE_REFS.with(|ref_cell| { let mut refs = ref_cell.borrow_mut(); diff --git a/crates/rspack_binding_values/src/context_module_factory.rs b/crates/rspack_binding_values/src/context_module_factory.rs index 938b719600f1..28f4ecb8660b 100644 --- a/crates/rspack_binding_values/src/context_module_factory.rs +++ b/crates/rspack_binding_values/src/context_module_factory.rs @@ -2,9 +2,10 @@ use napi::bindgen_prelude::{ ClassInstance, Either, FromNapiValue, ToNapiValue, TypeName, ValidateNapiValue, }; use napi_derive::napi; -use rspack_core::{AfterResolveData, BeforeResolveData, CompilationId}; +use rspack_core::{AfterResolveData, BeforeResolveData}; +use rspack_regex::RspackRegex; -use crate::{JsDependencyWrapper, RawRegex}; +use crate::JsDependencyWrapper; #[napi] pub struct JsContextModuleFactoryBeforeResolveData(Box); @@ -108,41 +109,38 @@ pub type JsContextModuleFactoryBeforeResolveResult = Either; #[napi] -pub struct JsContextModuleFactoryAfterResolveData { - compilation_id: CompilationId, - inner: Box, -} +pub struct JsContextModuleFactoryAfterResolveData(Box); #[napi] impl JsContextModuleFactoryAfterResolveData { #[napi(getter)] pub fn resource(&self) -> &str { - self.inner.resource.as_str() + self.0.resource.as_str() } #[napi(setter)] pub fn set_resource(&mut self, resource: String) { - self.inner.resource = resource.into(); + self.0.resource = resource.into(); } #[napi(getter)] pub fn context(&self) -> &str { - &self.inner.context + &self.0.context } #[napi(setter)] pub fn set_context(&mut self, context: String) { - self.inner.context = context; + self.0.context = context; } #[napi(getter)] pub fn request(&self) -> &str { - &self.inner.request + &self.0.request } #[napi(setter)] pub fn set_request(&mut self, request: String) { - self.inner.request = request; + self.0.request = request; } #[napi(getter, ts_return_type = "RegExp | undefined")] @@ -163,40 +161,34 @@ impl JsContextModuleFactoryAfterResolveData { #[napi(getter)] pub fn recursive(&self) -> bool { - self.inner.recursive + self.0.recursive } #[napi(setter)] pub fn set_recursive(&mut self, recursive: bool) { - self.inner.recursive = recursive; + self.0.recursive = recursive; } #[napi(getter, ts_return_type = "JsDependency[]")] pub fn dependencies(&self) -> Vec { self - .inner + .0 .dependencies .iter() - .map(|dep| JsDependencyWrapper::new(dep.as_ref(), self.compilation_id)) + .map(|dep| JsDependencyWrapper::new(dep.as_ref(), self.0.compilation_id, None)) .collect::>() } } -pub struct JsContextModuleFactoryAfterResolveDataWrapper { - compilation_id: CompilationId, - inner: Box, -} +pub struct JsContextModuleFactoryAfterResolveDataWrapper(Box); impl JsContextModuleFactoryAfterResolveDataWrapper { - pub fn new(data: Box, compilation_id: CompilationId) -> Self { - JsContextModuleFactoryAfterResolveDataWrapper { - compilation_id, - inner: data, - } + pub fn new(data: Box) -> Self { + JsContextModuleFactoryAfterResolveDataWrapper(data) } pub fn take(self) -> Box { - self.inner + self.0 } } @@ -209,10 +201,7 @@ impl FromNapiValue for JsContextModuleFactoryAfterResolveDataWrapper { as FromNapiValue>::from_napi_value( env, napi_val, )?; - Ok(Self { - compilation_id: instance.compilation_id, - inner: instance.inner.clone(), - }) + Ok(Self(instance.0.clone())) } } @@ -221,10 +210,7 @@ impl ToNapiValue for JsContextModuleFactoryAfterResolveDataWrapper { env: napi::sys::napi_env, val: Self, ) -> napi::Result { - let js_val = JsContextModuleFactoryAfterResolveData { - compilation_id: val.compilation_id, - inner: val.inner, - }; + let js_val = JsContextModuleFactoryAfterResolveData(val.0); ToNapiValue::to_napi_value(env, js_val) } } diff --git a/crates/rspack_binding_values/src/dependency.rs b/crates/rspack_binding_values/src/dependency.rs index bf712182fc84..bb4d73cb891d 100644 --- a/crates/rspack_binding_values/src/dependency.rs +++ b/crates/rspack_binding_values/src/dependency.rs @@ -1,38 +1,41 @@ use std::{cell::RefCell, ptr::NonNull}; -use napi::{ - bindgen_prelude::{ClassInstance, ToNapiValue}, - Env, -}; +use napi::bindgen_prelude::ToNapiValue; use napi_derive::napi; use rspack_core::{Compilation, CompilationId, Dependency, DependencyId}; use rspack_napi::OneShotRef; use rustc_hash::FxHashMap as HashMap; -use crate::JsCompilationWrapper; - // JsDependency allows JS-side access to a Dependency instance that has already // been processed and stored in the Compilation. #[napi] pub struct JsDependency { - pub(crate) compilation_id: CompilationId, + pub(crate) compilation: Option>, pub(crate) dependency_id: DependencyId, pub(crate) dependency: NonNull, } impl JsDependency { + fn attach(&mut self, compilation: NonNull) { + if self.compilation.is_none() { + self.compilation = Some(compilation); + } + } + fn as_ref(&mut self) -> napi::Result<&dyn Dependency> { let dependency = unsafe { self.dependency.as_ref() }; if *dependency.id() == self.dependency_id { return Ok(dependency); } - if let Some(compilation) = JsCompilationWrapper::compilation_by_id(&self.compilation_id) { + if let Some(compilation) = self.compilation { + let compilation = unsafe { compilation.as_ref() }; let module_graph = compilation.get_module_graph(); if let Some(dependency) = module_graph.dependency_by_id(&self.dependency_id) { - self.dependency = - NonNull::new(dependency.as_ref() as *const dyn Dependency as *mut dyn Dependency) - .unwrap(); + self.dependency = { + #[allow(clippy::unwrap_used)] + NonNull::new(dependency.as_ref() as *const dyn Dependency as *mut dyn Dependency).unwrap() + }; return Ok(unsafe { self.dependency.as_ref() }); } } @@ -106,7 +109,7 @@ impl JsDependency { } } -type DependencyInstanceRefs = HashMap>>; +type DependencyInstanceRefs = HashMap>; type DependencyInstanceRefsByCompilationId = RefCell>; @@ -118,17 +121,23 @@ thread_local! { pub struct JsDependencyWrapper { dependency_id: DependencyId, dependency: NonNull, + compilation_id: CompilationId, compilation: Option>, } impl JsDependencyWrapper { - pub fn new(dependency: &dyn Dependency, compilation: Option<&Compilation>) -> Self { + pub fn new( + dependency: &dyn Dependency, + compilation_id: CompilationId, + compilation: Option<&Compilation>, + ) -> Self { let dependency_id = *dependency.id(); #[allow(clippy::unwrap_used)] Self { dependency_id, dependency: NonNull::new(dependency as *const dyn Dependency as *mut dyn Dependency).unwrap(), + compilation_id, compilation: compilation .map(|c| NonNull::new(c as *const Compilation as *mut Compilation).unwrap()), } @@ -148,8 +157,6 @@ impl ToNapiValue for JsDependencyWrapper { val: Self, ) -> napi::Result { DEPENDENCY_INSTANCE_REFS.with(|refs| { - let compilation = unsafe { val.compilation.as_ref() }; - let mut refs_by_compilation_id = refs.borrow_mut(); let entry = refs_by_compilation_id.entry(val.compilation_id); let refs = match entry { @@ -163,16 +170,21 @@ impl ToNapiValue for JsDependencyWrapper { match refs.entry(val.dependency_id) { std::collections::hash_map::Entry::Occupied(occupied_entry) => { let r = occupied_entry.get(); + let instance = r.from_napi_mut_ref()?; + if let Some(compilation) = val.compilation { + instance.attach(compilation); + } else { + instance.dependency = val.dependency; + } ToNapiValue::to_napi_value(env, r) } std::collections::hash_map::Entry::Vacant(vacant_entry) => { - let instance: ClassInstance = JsDependency { - compilation_id: val.compilation_id, + let js_dependency = JsDependency { + compilation: val.compilation, dependency_id: val.dependency_id, dependency: val.dependency, - } - .into_instance(Env::from_raw(env))?; - let r = vacant_entry.insert(OneShotRef::new(env, instance)?); + }; + let r = vacant_entry.insert(OneShotRef::new(env, js_dependency)?); ToNapiValue::to_napi_value(env, r) } } diff --git a/crates/rspack_binding_values/src/dependency_block.rs b/crates/rspack_binding_values/src/dependency_block.rs index a9f8145cc193..6d9f343988a5 100644 --- a/crates/rspack_binding_values/src/dependency_block.rs +++ b/crates/rspack_binding_values/src/dependency_block.rs @@ -10,12 +10,6 @@ use rustc_hash::FxHashMap as HashMap; use crate::JsDependencyWrapper; -#[derive(Default)] -#[napi(object)] -pub struct JsFactoryMeta { - pub side_effect_free: Option, -} - #[napi] pub struct JsDependenciesBlock { block_id: AsyncDependenciesBlockIdentifier, @@ -34,9 +28,10 @@ impl JsDependenciesBlock { } if let Some(block) = module_graph.block_by_id(&self.block_id) { - self.block = - NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) - .unwrap(); + self.block = { + #[allow(clippy::unwrap_used)] + NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock).unwrap() + }; return Ok((unsafe { self.block.as_ref() }, compilation, module_graph)); } @@ -60,7 +55,7 @@ impl JsDependenciesBlock { .filter_map(|dependency_id| { module_graph .dependency_by_id(dependency_id) - .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation)) + .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation.id(), Some(compilation))) }) .collect::>(), ) @@ -84,8 +79,7 @@ impl JsDependenciesBlock { } } -type BlockInstanceRefs = - HashMap>>; +type BlockInstanceRefs = HashMap>; type BlockInstanceRefsByCompilationId = RefCell>; @@ -143,13 +137,12 @@ impl ToNapiValue for JsDependenciesBlockWrapper { ToNapiValue::to_napi_value(env, r) } std::collections::hash_map::Entry::Vacant(vacant_entry) => { - let instance: ClassInstance = JsDependenciesBlock { + let js_block = JsDependenciesBlock { block_id: val.block_id, block: val.block, compilation: val.compilation, - } - .into_instance(Env::from_raw(env))?; - let r = vacant_entry.insert(OneShotRef::new(env, instance)?); + }; + let r = vacant_entry.insert(OneShotRef::new(env, js_block)?); ToNapiValue::to_napi_value(env, r) } } diff --git a/crates/rspack_binding_values/src/module.rs b/crates/rspack_binding_values/src/module.rs index 758ca24b49ca..f2fd15865ebe 100644 --- a/crates/rspack_binding_values/src/module.rs +++ b/crates/rspack_binding_values/src/module.rs @@ -11,7 +11,7 @@ use rspack_util::source_map::SourceMapKind; use rustc_hash::FxHashMap as HashMap; use super::{JsCompatSource, ToJsCompatSource}; -use crate::{JsChunk, JsCodegenerationResults, JsDependenciesBlock}; +use crate::{JsChunk, JsCodegenerationResults, JsDependenciesBlockWrapper, JsDependencyWrapper}; #[derive(Default)] #[napi(object)] @@ -195,17 +195,22 @@ impl JsModule { }) } - #[napi(getter)] - pub fn blocks(&mut self) -> napi::Result> { + #[napi(getter, ts_return_type = "JsDependenciesBlock")] + pub fn blocks(&mut self) -> napi::Result> { Ok(match self.compilation { Some(compilation) => { + let compilation = unsafe { compilation.as_ref() }; + let module_graph = compilation.get_module_graph(); let module = self.as_ref()?; let blocks = module.get_blocks(); blocks .iter() - .cloned() - .map(|block_id| JsDependenciesBlock::new(block_id, compilation.as_ptr())) + .filter_map(|block_id| { + module_graph + .block_by_id(block_id) + .map(|block| JsDependenciesBlockWrapper::new(block, compilation)) + }) .collect::>() } None => { @@ -214,8 +219,8 @@ impl JsModule { }) } - #[napi(getter)] - pub fn dependencies(&mut self) -> napi::Result> { + #[napi(getter, ts_return_type = "JsDependency")] + pub fn dependencies(&mut self) -> napi::Result> { Ok(match self.compilation { Some(compilation) => { let compilation = unsafe { compilation.as_ref() }; @@ -225,9 +230,10 @@ impl JsModule { dependencies .iter() .filter_map(|dependency_id| { - module_graph - .dependency_by_id(dependency_id) - .map(JsDependency::new) + module_graph.dependency_by_id(dependency_id).map(|dep| { + let compilation = unsafe { self.compilation.map(|c| c.as_ref()) }; + JsDependencyWrapper::new(dep.as_ref(), self.compilation_id, compilation) + }) }) .collect::>() } diff --git a/crates/rspack_core/src/context_module_factory.rs b/crates/rspack_core/src/context_module_factory.rs index a17ffe9f18b9..f2e2ae777cf9 100644 --- a/crates/rspack_core/src/context_module_factory.rs +++ b/crates/rspack_core/src/context_module_factory.rs @@ -10,11 +10,11 @@ use swc_core::common::util::take::Take; use tracing::instrument; use crate::{ - resolve, BoxDependency, ContextElementDependency, ContextModule, ContextModuleOptions, - DependencyCategory, DependencyId, DependencyType, ErrorSpan, ModuleExt, ModuleFactory, - ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, RawModule, ResolveArgs, - ResolveContextModuleDependencies, ResolveInnerOptions, ResolveOptionsWithDependencyType, - ResolveResult, Resolver, ResolverFactory, SharedPluginDriver, + resolve, BoxDependency, CompilationId, ContextElementDependency, ContextModule, + ContextModuleOptions, DependencyCategory, DependencyId, DependencyType, ErrorSpan, ModuleExt, + ModuleFactory, ModuleFactoryCreateData, ModuleFactoryResult, ModuleIdentifier, RawModule, + ResolveArgs, ResolveContextModuleDependencies, ResolveInnerOptions, + ResolveOptionsWithDependencyType, ResolveResult, Resolver, ResolverFactory, SharedPluginDriver, }; #[derive(Debug)] @@ -50,6 +50,7 @@ pub enum AfterResolveResult { #[derive(Derivative)] #[derivative(Debug, Clone)] pub struct AfterResolveData { + pub compilation_id: CompilationId, pub resource: Utf8PathBuf, pub context: String, pub dependencies: Vec, @@ -339,6 +340,7 @@ impl ContextModuleFactory { ) -> Result> { let context_options = &context_module_options.context_options; let after_resolve_data = AfterResolveData { + compilation_id: data.compilation_id, resource: context_module_options.resource.clone(), context: context_options.context.clone(), dependencies: data.dependencies.clone(), From 08e433612c78cdd6be01939914e335794596f16b Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 14:10:00 +0800 Subject: [PATCH 5/9] fix --- crates/rspack_binding_values/src/module.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/rspack_binding_values/src/module.rs b/crates/rspack_binding_values/src/module.rs index f2fd15865ebe..e490f0d6e2e0 100644 --- a/crates/rspack_binding_values/src/module.rs +++ b/crates/rspack_binding_values/src/module.rs @@ -195,7 +195,7 @@ impl JsModule { }) } - #[napi(getter, ts_return_type = "JsDependenciesBlock")] + #[napi(getter, ts_return_type = "JsDependenciesBlock[]")] pub fn blocks(&mut self) -> napi::Result> { Ok(match self.compilation { Some(compilation) => { @@ -219,7 +219,7 @@ impl JsModule { }) } - #[napi(getter, ts_return_type = "JsDependency")] + #[napi(getter, ts_return_type = "JsDependency[]")] pub fn dependencies(&mut self) -> napi::Result> { Ok(match self.compilation { Some(compilation) => { From 14674a04474ab0942e131b6f7c921797c473c7b5 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 19:35:16 +0800 Subject: [PATCH 6/9] fix --- .../rspack_binding_values/src/dependency.rs | 35 +++++------ .../src/dependency_block.rs | 60 ++++++------------- crates/rspack_binding_values/src/module.rs | 35 +++++------ 3 files changed, 47 insertions(+), 83 deletions(-) diff --git a/crates/rspack_binding_values/src/dependency.rs b/crates/rspack_binding_values/src/dependency.rs index bb4d73cb891d..ed29338a429a 100644 --- a/crates/rspack_binding_values/src/dependency.rs +++ b/crates/rspack_binding_values/src/dependency.rs @@ -23,11 +23,6 @@ impl JsDependency { } fn as_ref(&mut self) -> napi::Result<&dyn Dependency> { - let dependency = unsafe { self.dependency.as_ref() }; - if *dependency.id() == self.dependency_id { - return Ok(dependency); - } - if let Some(compilation) = self.compilation { let compilation = unsafe { compilation.as_ref() }; let module_graph = compilation.get_module_graph(); @@ -36,26 +31,26 @@ impl JsDependency { #[allow(clippy::unwrap_used)] NonNull::new(dependency.as_ref() as *const dyn Dependency as *mut dyn Dependency).unwrap() }; - return Ok(unsafe { self.dependency.as_ref() }); + Ok(unsafe { self.dependency.as_ref() }) + } else { + Err(napi::Error::from_reason(format!( + "Unable to access dependency with id = {:?} now. The dependency have been removed on the Rust side.", + self.dependency_id + ))) } + } else { + // SAFETY: + // We need to make users aware in the documentation that values obtained within the JS hook callback should not be used outside the scope of the callback. + // We do not guarantee that the memory pointed to by the pointer remains valid when used outside the scope. + Ok(unsafe { self.dependency.as_ref() }) } - - Err(napi::Error::from_reason(format!( - "Unable to access dependency with id = {:?} now. The dependency have been removed on the Rust side.", - self.dependency_id - ))) } fn as_mut(&mut self) -> napi::Result<&mut dyn Dependency> { - let dependency = unsafe { self.dependency.as_mut() }; - if *dependency.id() == self.dependency_id { - return Ok(dependency); - } - - Err(napi::Error::from_reason(format!( - "Unable to access dependency with id = {:?} now. The dependency have been removed on the Rust side.", - self.dependency_id - ))) + // SAFETY: + // We need to make users aware in the documentation that values obtained within the JS hook callback should not be used outside the scope of the callback. + // We do not guarantee that the memory pointed to by the pointer remains valid when used outside the scope. + Ok(unsafe { self.dependency.as_mut() }) } } diff --git a/crates/rspack_binding_values/src/dependency_block.rs b/crates/rspack_binding_values/src/dependency_block.rs index 6d9f343988a5..3103ef750268 100644 --- a/crates/rspack_binding_values/src/dependency_block.rs +++ b/crates/rspack_binding_values/src/dependency_block.rs @@ -3,7 +3,7 @@ use std::{cell::RefCell, ptr::NonNull}; use napi_derive::napi; use rspack_core::{ AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, Compilation, CompilationId, - DependenciesBlock, ModuleGraph, + DependenciesBlock, }; use rspack_napi::{napi::bindgen_prelude::*, OneShotRef}; use rustc_hash::FxHashMap as HashMap; @@ -13,42 +13,16 @@ use crate::JsDependencyWrapper; #[napi] pub struct JsDependenciesBlock { block_id: AsyncDependenciesBlockIdentifier, - block: NonNull, compilation: NonNull, } +#[napi] impl JsDependenciesBlock { - fn as_ref(&mut self) -> napi::Result<(&AsyncDependenciesBlock, &Compilation, ModuleGraph)> { + #[napi(getter, ts_return_type = "JsDependency[]")] + pub fn dependencies(&mut self) -> Vec { let compilation = unsafe { self.compilation.as_ref() }; let module_graph = compilation.get_module_graph(); - - let block = unsafe { self.block.as_ref() }; - if block.identifier() == self.block_id { - return Ok((block, compilation, module_graph)); - } - if let Some(block) = module_graph.block_by_id(&self.block_id) { - self.block = { - #[allow(clippy::unwrap_used)] - NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock).unwrap() - }; - return Ok((unsafe { self.block.as_ref() }, compilation, module_graph)); - } - - Err(napi::Error::from_reason(format!( - "Unable to access block with id = {:?} now. The block have been removed on the Rust side.", - self.block_id - ))) - } -} - -#[napi] -impl JsDependenciesBlock { - #[napi(getter, ts_return_type = "JsDependency")] - pub fn dependencies(&mut self) -> Result> { - let (block, compilation, module_graph) = self.as_ref()?; - - Ok( block .get_dependencies() .iter() @@ -57,15 +31,17 @@ impl JsDependenciesBlock { .dependency_by_id(dependency_id) .map(|dep| JsDependencyWrapper::new(dep.as_ref(), compilation.id(), Some(compilation))) }) - .collect::>(), - ) + .collect::>() + } else { + vec![] + } } - #[napi(getter, ts_return_type = "JsDependenciesBlock")] - pub fn blocks(&mut self) -> Result> { - let (block, compilation, module_graph) = self.as_ref()?; - - Ok( + #[napi(getter, ts_return_type = "JsDependenciesBlock[]")] + pub fn blocks(&mut self) -> Vec { + let compilation = unsafe { self.compilation.as_ref() }; + let module_graph = compilation.get_module_graph(); + if let Some(block) = module_graph.block_by_id(&self.block_id) { block .get_blocks() .iter() @@ -74,8 +50,10 @@ impl JsDependenciesBlock { .block_by_id(block_id) .map(|block| JsDependenciesBlockWrapper::new(block, compilation)) }) - .collect::>(), - ) + .collect::>() + } else { + vec![] + } } } @@ -89,7 +67,6 @@ thread_local! { pub struct JsDependenciesBlockWrapper { block_id: AsyncDependenciesBlockIdentifier, - block: NonNull, compilation: NonNull, } @@ -100,8 +77,6 @@ impl JsDependenciesBlockWrapper { #[allow(clippy::unwrap_used)] Self { block_id, - block: NonNull::new(block as *const AsyncDependenciesBlock as *mut AsyncDependenciesBlock) - .unwrap(), compilation: NonNull::new(compilation as *const Compilation as *mut Compilation).unwrap(), } } @@ -139,7 +114,6 @@ impl ToNapiValue for JsDependenciesBlockWrapper { std::collections::hash_map::Entry::Vacant(vacant_entry) => { let js_block = JsDependenciesBlock { block_id: val.block_id, - block: val.block, compilation: val.compilation, }; let r = vacant_entry.insert(OneShotRef::new(env, js_block)?); diff --git a/crates/rspack_binding_values/src/module.rs b/crates/rspack_binding_values/src/module.rs index 8da971ef5fec..a5b1c230b669 100644 --- a/crates/rspack_binding_values/src/module.rs +++ b/crates/rspack_binding_values/src/module.rs @@ -36,11 +36,6 @@ impl JsModule { } fn as_ref(&mut self) -> napi::Result<&'static dyn Module> { - let module = unsafe { self.module.as_ref() }; - if module.identifier() == self.identifier { - return Ok(module); - } - if let Some(compilation) = self.compilation { let compilation = unsafe { compilation.as_ref() }; if let Some(module) = compilation.module_by_identifier(&self.identifier) { @@ -49,26 +44,26 @@ impl JsModule { #[allow(clippy::unwrap_used)] NonNull::new(module as *const dyn Module as *mut dyn Module).unwrap() }; - return Ok(module); + Ok(module) + } else { + Err(napi::Error::from_reason(format!( + "Unable to access module with id = {} now. The module have been removed on the Rust side.", + self.identifier + ))) } + } else { + // SAFETY: + // We need to make users aware in the documentation that values obtained within the JS hook callback should not be used outside the scope of the callback. + // We do not guarantee that the memory pointed to by the pointer remains valid when used outside the scope. + Ok(unsafe { self.module.as_ref() }) } - - Err(napi::Error::from_reason(format!( - "Unable to access module with id = {} now. The module have been removed on the Rust side.", - self.identifier - ))) } fn as_mut(&mut self) -> napi::Result<&'static mut dyn Module> { - let module = unsafe { self.module.as_mut() }; - if module.identifier() == self.identifier { - return Ok(module); - } - - Err(napi::Error::from_reason(format!( - "Unable to access module with id = {} now. The module have been removed on the Rust side.", - self.identifier - ))) + // SAFETY: + // We need to make users aware in the documentation that values obtained within the JS hook callback should not be used outside the scope of the callback. + // We do not guarantee that the memory pointed to by the pointer remains valid when used outside the scope. + Ok(unsafe { self.module.as_mut() }) } } From 5b2e260708a7edeedc690017dff566ed21c97c55 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 21:01:14 +0800 Subject: [PATCH 7/9] fix --- crates/node_binding/binding.d.ts | 21 +++++++-------------- packages/rspack/src/Dependency.ts | 10 ++++------ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/crates/node_binding/binding.d.ts b/crates/node_binding/binding.d.ts index e9014f096205..bac59f0cc4e1 100644 --- a/crates/node_binding/binding.d.ts +++ b/crates/node_binding/binding.d.ts @@ -20,8 +20,8 @@ export declare class ExternalObject { } } export declare class EntryDataDto { - get dependencies(): Array - get includeDependencies(): Array + get dependencies(): JsDependency[] + get includeDependencies(): JsDependency[] get options(): EntryOptionsDto } export type EntryDataDTO = EntryDataDto @@ -108,7 +108,7 @@ export declare class JsContextModuleFactoryAfterResolveData { set regExp(rawRegExp: RegExp | undefined) get recursive(): boolean set recursive(recursive: boolean) - get dependencies(): Array + get dependencies(): JsDependency[] } export declare class JsContextModuleFactoryBeforeResolveData { @@ -138,8 +138,8 @@ export declare class JsDependencies { } export declare class JsDependenciesBlock { - get dependencies(): Array - get blocks(): Array + get dependencies(): JsDependency[] + get blocks(): JsDependenciesBlock[] } export declare class JsDependency { @@ -147,13 +147,6 @@ export declare class JsDependency { get category(): string get request(): string | undefined get critical(): boolean -} - -export declare class JsDependencyMut { - get type(): string - get category(): string - get request(): string | undefined - get critical(): boolean set critical(val: boolean) } @@ -181,8 +174,8 @@ export declare class JsModule { get factoryMeta(): JsFactoryMeta | undefined get type(): string get layer(): string | undefined - get blocks(): Array - get dependencies(): Array + get blocks(): JsDependenciesBlock[] + get dependencies(): JsDependency[] size(ty?: string | undefined | null): number get modules(): JsModule[] | undefined get useSourceMap(): boolean diff --git a/packages/rspack/src/Dependency.ts b/packages/rspack/src/Dependency.ts index 49e3eb5b3eff..ac871704f165 100644 --- a/packages/rspack/src/Dependency.ts +++ b/packages/rspack/src/Dependency.ts @@ -1,4 +1,4 @@ -import { type JsDependency, JsDependencyMut } from "@rspack/binding"; +import { type JsDependency } from "@rspack/binding"; export class Dependency { declare readonly type: string; @@ -6,11 +6,11 @@ export class Dependency { declare readonly request: string | undefined; declare critical: boolean; - static __from_binding(binding: JsDependencyMut | JsDependency): Dependency { + static __from_binding(binding: JsDependency): Dependency { return new Dependency(binding); } - private constructor(binding: JsDependencyMut | JsDependency) { + private constructor(binding: JsDependency) { Object.defineProperties(this, { type: { enumerable: true, @@ -36,9 +36,7 @@ export class Dependency { return binding.critical; }, set(val: boolean) { - if (binding instanceof JsDependencyMut) { - binding.critical = val; - } + binding.critical = val; } } }); From b3737c1a23c052c02836cb8dbd5872e22562c288 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 21:03:44 +0800 Subject: [PATCH 8/9] fix --- .../compilation/rebuild-module/rspack.config.js | 7 ++++--- packages/rspack/etc/core.api.md | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js b/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js index e738c8c11006..d7af394cb9b1 100644 --- a/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js +++ b/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js @@ -27,9 +27,10 @@ class Plugin { }); }); - expect( - newModule.originalSource().source().includes("a = 2") - ).toBe(true); + console.log('newModule.originalSource().source()', newModule.originalSource()) + // expect( + // newModule.originalSource().source().includes("a = 2") + // ).toBe(true); } }); }); diff --git a/packages/rspack/etc/core.api.md b/packages/rspack/etc/core.api.md index 12685109bed2..e352d3e77900 100644 --- a/packages/rspack/etc/core.api.md +++ b/packages/rspack/etc/core.api.md @@ -44,7 +44,6 @@ import type { JsContextModuleFactoryBeforeResolveData } from '@rspack/binding'; import type { JsCreateData } from '@rspack/binding'; import type { JsDependenciesBlock } from '@rspack/binding'; import { JsDependency } from '@rspack/binding'; -import { JsDependencyMut } from '@rspack/binding'; import type { JsFactoryMeta } from '@rspack/binding'; import { JsHtmlPluginTag } from '@rspack/binding'; import { JsLibraryOptions } from '@rspack/binding'; @@ -1319,7 +1318,7 @@ class DependenciesBlock { // @public (undocumented) class Dependency { // (undocumented) - static __from_binding(binding: JsDependencyMut | JsDependency): Dependency; + static __from_binding(binding: JsDependency): Dependency; // (undocumented) readonly category: string; // (undocumented) From 4162479639435f4d9f296d5f97f7ab52229d83f3 Mon Sep 17 00:00:00 2001 From: Cong-Cong Date: Mon, 11 Nov 2024 21:33:49 +0800 Subject: [PATCH 9/9] fix --- crates/rspack_binding_values/src/dependency.rs | 14 +++----------- crates/rspack_binding_values/src/module.rs | 13 ++----------- .../compilation/rebuild-module/rspack.config.js | 7 +++---- packages/rspack/etc/core.api.md | 2 +- packages/rspack/src/Dependency.ts | 2 +- 5 files changed, 10 insertions(+), 28 deletions(-) diff --git a/crates/rspack_binding_values/src/dependency.rs b/crates/rspack_binding_values/src/dependency.rs index ed29338a429a..dfb297d4718b 100644 --- a/crates/rspack_binding_values/src/dependency.rs +++ b/crates/rspack_binding_values/src/dependency.rs @@ -16,12 +16,6 @@ pub struct JsDependency { } impl JsDependency { - fn attach(&mut self, compilation: NonNull) { - if self.compilation.is_none() { - self.compilation = Some(compilation); - } - } - fn as_ref(&mut self) -> napi::Result<&dyn Dependency> { if let Some(compilation) = self.compilation { let compilation = unsafe { compilation.as_ref() }; @@ -166,11 +160,9 @@ impl ToNapiValue for JsDependencyWrapper { std::collections::hash_map::Entry::Occupied(occupied_entry) => { let r = occupied_entry.get(); let instance = r.from_napi_mut_ref()?; - if let Some(compilation) = val.compilation { - instance.attach(compilation); - } else { - instance.dependency = val.dependency; - } + instance.compilation = val.compilation; + instance.dependency = val.dependency; + ToNapiValue::to_napi_value(env, r) } std::collections::hash_map::Entry::Vacant(vacant_entry) => { diff --git a/crates/rspack_binding_values/src/module.rs b/crates/rspack_binding_values/src/module.rs index a5b1c230b669..33c9c1990104 100644 --- a/crates/rspack_binding_values/src/module.rs +++ b/crates/rspack_binding_values/src/module.rs @@ -29,12 +29,6 @@ pub struct JsModule { } impl JsModule { - fn attach(&mut self, compilation: NonNull) { - if self.compilation.is_none() { - self.compilation = Some(compilation); - } - } - fn as_ref(&mut self) -> napi::Result<&'static dyn Module> { if let Some(compilation) = self.compilation { let compilation = unsafe { compilation.as_ref() }; @@ -359,11 +353,8 @@ impl ToNapiValue for JsModuleWrapper { std::collections::hash_map::Entry::Occupied(entry) => { let r = entry.get(); let instance = r.from_napi_mut_ref()?; - if let Some(compilation) = val.compilation { - instance.attach(compilation); - } else { - instance.module = val.module; - } + instance.compilation = val.compilation; + instance.module = val.module; ToNapiValue::to_napi_value(env, r) } std::collections::hash_map::Entry::Vacant(entry) => { diff --git a/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js b/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js index d7af394cb9b1..e738c8c11006 100644 --- a/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js +++ b/packages/rspack-test-tools/tests/configCases/compilation/rebuild-module/rspack.config.js @@ -27,10 +27,9 @@ class Plugin { }); }); - console.log('newModule.originalSource().source()', newModule.originalSource()) - // expect( - // newModule.originalSource().source().includes("a = 2") - // ).toBe(true); + expect( + newModule.originalSource().source().includes("a = 2") + ).toBe(true); } }); }); diff --git a/packages/rspack/etc/core.api.md b/packages/rspack/etc/core.api.md index e352d3e77900..39765f4625d1 100644 --- a/packages/rspack/etc/core.api.md +++ b/packages/rspack/etc/core.api.md @@ -43,7 +43,7 @@ import type { JsContextModuleFactoryAfterResolveData } from '@rspack/binding'; import type { JsContextModuleFactoryBeforeResolveData } from '@rspack/binding'; import type { JsCreateData } from '@rspack/binding'; import type { JsDependenciesBlock } from '@rspack/binding'; -import { JsDependency } from '@rspack/binding'; +import type { JsDependency } from '@rspack/binding'; import type { JsFactoryMeta } from '@rspack/binding'; import { JsHtmlPluginTag } from '@rspack/binding'; import { JsLibraryOptions } from '@rspack/binding'; diff --git a/packages/rspack/src/Dependency.ts b/packages/rspack/src/Dependency.ts index ac871704f165..8030b7cd2034 100644 --- a/packages/rspack/src/Dependency.ts +++ b/packages/rspack/src/Dependency.ts @@ -1,4 +1,4 @@ -import { type JsDependency } from "@rspack/binding"; +import type { JsDependency } from "@rspack/binding"; export class Dependency { declare readonly type: string;