diff --git a/crates/rspack_binding_api/src/module.rs b/crates/rspack_binding_api/src/module.rs index 02bbf043f14b..501ad789674e 100644 --- a/crates/rspack_binding_api/src/module.rs +++ b/crates/rspack_binding_api/src/module.rs @@ -9,8 +9,8 @@ use rspack_core::{ FactoryMeta, LibIdentOptions, Module as _, ModuleIdentifier, RuntimeModuleStage, SourceType, }; use rspack_napi::{ - OneShotInstanceRef, WeakRef, napi::bindgen_prelude::*, string::JsStringExt, - threadsafe_function::ThreadsafeFunction, + OneShotInstanceRef, Property, WeakRef, define_properties, napi::bindgen_prelude::*, + string::JsStringExt, threadsafe_function::ThreadsafeFunction, }; use rspack_plugin_runtime::RuntimeModuleFromJs; use rspack_util::source_map::SourceMapKind; @@ -227,44 +227,44 @@ impl Module { properties.clear(); properties.push( Property::new() - .with_utf8_name("type")? + .with_utf8_name(c"type")? .with_value(&env.create_string(module.module_type().as_str())?), ); properties.push( Property::new() - .with_utf8_name("context")? + .with_utf8_name(c"context")? .with_getter(context_getter), ); properties.push( Property::new() - .with_utf8_name("layer")? + .with_utf8_name(c"layer")? .with_getter(layer_getter), ); properties.push( Property::new() - .with_utf8_name("useSourceMap")? + .with_utf8_name(c"useSourceMap")? .with_getter(use_source_map_getter), ); properties.push( Property::new() - .with_utf8_name("useSimpleSourceMap")? + .with_utf8_name(c"useSimpleSourceMap")? .with_getter(use_simple_source_map_getter), ); properties.push( Property::new() - .with_utf8_name("factoryMeta")? + .with_utf8_name(c"factoryMeta")? .with_getter(factory_meta_getter) .with_setter(factory_meta_setter), ); properties.push( Property::new() - .with_utf8_name("buildInfo")? + .with_utf8_name(c"buildInfo")? .with_getter(build_info_getter) .with_setter(build_info_setter), ); properties.push( Property::new() - .with_utf8_name("buildMeta")? + .with_utf8_name(c"buildMeta")? .with_value(&Object::new(env)?), ); MODULE_IDENTIFIER_SYMBOL.with(|once_cell| { @@ -280,7 +280,7 @@ impl Module { Ok::<(), napi::Error>(()) })?; - object.define_properties(&properties) + define_properties(env, &mut object, &properties) })?; Ok(instance) diff --git a/crates/rspack_binding_api/src/modules/external_module.rs b/crates/rspack_binding_api/src/modules/external_module.rs index 2c3883ee5d0e..1aaa185c0107 100644 --- a/crates/rspack_binding_api/src/modules/external_module.rs +++ b/crates/rspack_binding_api/src/modules/external_module.rs @@ -22,8 +22,8 @@ impl ExternalModule { properties.clear(); properties.push( - napi::Property::new() - .with_utf8_name("userRequest")? + rspack_napi::Property::new() + .with_utf8_name(c"userRequest")? .with_value(&user_request), ); Self::new_inherited(self, env, &mut properties) diff --git a/crates/rspack_binding_api/src/modules/macros.rs b/crates/rspack_binding_api/src/modules/macros.rs index 4f51d70d1d4b..192bbfa5e47d 100644 --- a/crates/rspack_binding_api/src/modules/macros.rs +++ b/crates/rspack_binding_api/src/modules/macros.rs @@ -5,7 +5,7 @@ macro_rules! impl_module_methods { fn new_inherited<'a>( self, env: &'a napi::Env, - properties: &mut Vec, + properties: &mut Vec, ) -> napi::Result> { use napi::bindgen_prelude::{JavaScriptClassExt, JsObjectValue, JsValue}; @@ -186,59 +186,59 @@ macro_rules! impl_module_methods { } properties.push( - napi::Property::new() - .with_utf8_name("type")? + rspack_napi::Property::new() + .with_utf8_name(c"type")? .with_value(&env.create_string(module.module_type().as_str())?), ); properties.push( - napi::Property::new() - .with_utf8_name("context")? + rspack_napi::Property::new() + .with_utf8_name(c"context")? .with_getter(context_getter) ); properties.push( - napi::Property::new() - .with_utf8_name("layer")? + rspack_napi::Property::new() + .with_utf8_name(c"layer")? .with_getter(layer_getter) ); properties.push( - napi::Property::new() - .with_utf8_name("useSourceMap")? + rspack_napi::Property::new() + .with_utf8_name(c"useSourceMap")? .with_getter(use_source_map_getter) ); properties.push( - napi::Property::new() - .with_utf8_name("useSimpleSourceMap")? + rspack_napi::Property::new() + .with_utf8_name(c"useSimpleSourceMap")? .with_getter(use_simple_source_map_getter), ); properties.push( - napi::Property::new() - .with_utf8_name("factoryMeta")? + rspack_napi::Property::new() + .with_utf8_name(c"factoryMeta")? .with_getter(factory_meta_getter) .with_setter(factory_meta_setter), ); properties.push( - napi::Property::new() - .with_utf8_name("buildInfo")? + rspack_napi::Property::new() + .with_utf8_name(c"buildInfo")? .with_getter(build_info_getter) .with_setter(build_info_setter) ); properties.push( - napi::Property::new() - .with_utf8_name("buildMeta")? + rspack_napi::Property::new() + .with_utf8_name(c"buildMeta")? .with_value(&napi::bindgen_prelude::Object::new(env)?) ); $crate::module::MODULE_IDENTIFIER_SYMBOL.with(|once_cell| { let identifier = env.create_string(module.identifier().as_str())?; let symbol = once_cell.get().unwrap(); properties.push( - napi::bindgen_prelude::Property::new() + rspack_napi::Property::new() .with_name(env, symbol)? .with_value(&identifier) .with_property_attributes(napi::bindgen_prelude::PropertyAttributes::Configurable), ); Ok::<(), napi::Error>(()) })?; - object.define_properties(properties)?; + rspack_napi::define_properties(env, &mut object, properties)?; Ok(instance) } diff --git a/crates/rspack_binding_api/src/modules/normal_module.rs b/crates/rspack_binding_api/src/modules/normal_module.rs index 792b472a3afd..318797ae412b 100644 --- a/crates/rspack_binding_api/src/modules/normal_module.rs +++ b/crates/rspack_binding_api/src/modules/normal_module.rs @@ -11,6 +11,46 @@ use crate::{ resource_data::ReadonlyResourceDataWrapper, }; +#[js_function] +pub fn match_resource_getter(ctx: CallContext<'_>) -> napi::Result> { + let this = ctx.this_unchecked::(); + let env = ctx.env.raw(); + let wrapped_value = unsafe { NormalModule::from_napi_mut_ref(env, this.raw())? }; + + let (_, module) = wrapped_value.as_ref()?; + Ok(match module.match_resource() { + Some(match_resource) => Either::A(&match_resource.resource), + None => Either::B(()), + }) +} + +#[js_function(1)] +pub fn match_resource_setter(ctx: CallContext) -> napi::Result<()> { + let this = ctx.this_unchecked::(); + let env = ctx.env.raw(); + let wrapped_value = unsafe { NormalModule::from_napi_mut_ref(env, this.raw())? }; + + let val = ctx.get::>(0)?; + match val { + Either::A(val) => { + let module = wrapped_value.as_mut()?; + let ResourceParsedData { + path, + query, + fragment, + } = parse_resource(&val).expect("Should parse resource"); + *module.match_resource_mut() = Some( + ResourceData::new(val) + .path(path) + .query_optional(query) + .fragment_optional(fragment), + ); + } + Either::B(_) => {} + } + Ok(()) +} + #[napi] #[repr(C)] pub struct NormalModule { @@ -50,83 +90,43 @@ impl NormalModule { )? }); - #[js_function] - pub fn match_resource_getter(ctx: CallContext<'_>) -> napi::Result> { - let this = ctx.this_unchecked::(); - let env = ctx.env.raw(); - let wrapped_value = unsafe { NormalModule::from_napi_mut_ref(env, this.raw())? }; - - let (_, module) = wrapped_value.as_ref()?; - Ok(match module.match_resource() { - Some(match_resource) => Either::A(&match_resource.resource), - None => Either::B(()), - }) - } - - #[js_function(1)] - pub fn match_resource_setter(ctx: CallContext) -> napi::Result<()> { - let this = ctx.this_unchecked::(); - let env = ctx.env.raw(); - let wrapped_value = unsafe { NormalModule::from_napi_mut_ref(env, this.raw())? }; - - let val = ctx.get::>(0)?; - match val { - Either::A(val) => { - let module = wrapped_value.as_mut()?; - let ResourceParsedData { - path, - query, - fragment, - } = parse_resource(&val).expect("Should parse resource"); - *module.match_resource_mut() = Some( - ResourceData::new(val) - .path(path) - .query_optional(query) - .fragment_optional(fragment), - ); - } - Either::B(_) => {} - } - Ok(()) - } - MODULE_PROPERTIES_BUFFER.with(|ref_cell| { let mut properties = ref_cell.borrow_mut(); properties.clear(); properties.push( - napi::Property::new() - .with_utf8_name("resource")? + rspack_napi::Property::new() + .with_utf8_name(c"resource")? .with_value(&resource), ); properties.push( - napi::Property::new() - .with_utf8_name("request")? + rspack_napi::Property::new() + .with_utf8_name(c"request")? .with_value(&request), ); properties.push( - napi::Property::new() - .with_utf8_name("userRequest")? + rspack_napi::Property::new() + .with_utf8_name(c"userRequest")? .with_value(&user_request), ); properties.push( - napi::Property::new() - .with_utf8_name("rawRequest")? + rspack_napi::Property::new() + .with_utf8_name(c"rawRequest")? .with_value(&raw_request), ); properties.push( - napi::Property::new() - .with_utf8_name("resourceResolveData")? + rspack_napi::Property::new() + .with_utf8_name(c"resourceResolveData")? .with_value(&resource_resolve_data), ); properties.push( - napi::Property::new() - .with_utf8_name("loaders")? + rspack_napi::Property::new() + .with_utf8_name(c"loaders")? .with_value(&loaders), ); properties.push( - napi::Property::new() - .with_utf8_name("matchResource")? + rspack_napi::Property::new() + .with_utf8_name(c"matchResource")? .with_getter(match_resource_getter) .with_setter(match_resource_setter), ); diff --git a/crates/rspack_binding_api/src/resource_data.rs b/crates/rspack_binding_api/src/resource_data.rs index b23dcdd30aa2..ce19aa7dcdc3 100644 --- a/crates/rspack_binding_api/src/resource_data.rs +++ b/crates/rspack_binding_api/src/resource_data.rs @@ -4,11 +4,12 @@ use std::{ }; use napi::{ - Env, JsValue, Property, + Env, JsValue, bindgen_prelude::{JavaScriptClassExt, JsObjectValue, ToNapiValue}, sys::napi_value, }; use napi_derive::napi; +use rspack_napi::{Property, define_properties}; // Converting the descriptionFileData property to a JSObject may become a performance bottleneck. // Additionally, descriptionFileData and descriptionFilePath are rarely used, so they are exposed via getter methods and only converted to JSObject when accessed. @@ -85,31 +86,31 @@ impl ToNapiValue for ReadonlyResourceDataWrapper { properties.clear(); properties.push( Property::new() - .with_utf8_name("resource")? + .with_utf8_name(c"resource")? .with_value(&env_wrapper.create_string(&resource_data.resource)?), ); if let Some(path) = &resource_data.resource_path { properties.push( Property::new() - .with_utf8_name("path")? + .with_utf8_name(c"path")? .with_value(&env_wrapper.create_string(path.as_str())?), ); } if let Some(query) = &resource_data.resource_query { properties.push( Property::new() - .with_utf8_name("query")? + .with_utf8_name(c"query")? .with_value(&env_wrapper.create_string(query)?), ); } if let Some(fragment) = &resource_data.resource_fragment { properties.push( Property::new() - .with_utf8_name("fragment")? + .with_utf8_name(c"fragment")? .with_value(&env_wrapper.create_string(fragment)?), ); } - object.define_properties(&properties) + define_properties(&env_wrapper, &mut object, &properties) })?; Ok(object.raw()) diff --git a/crates/rspack_napi/src/lib.rs b/crates/rspack_napi/src/lib.rs index 848f4f2b8dc5..388160324100 100644 --- a/crates/rspack_napi/src/lib.rs +++ b/crates/rspack_napi/src/lib.rs @@ -2,6 +2,7 @@ mod ext; mod js_values; +mod object; mod utils; mod errors; @@ -22,4 +23,5 @@ pub use js_values::{ weak_ref::*, }; pub use napi; +pub use object::*; pub use utils::*; diff --git a/crates/rspack_napi/src/object.rs b/crates/rspack_napi/src/object.rs new file mode 100644 index 000000000000..e128ed950945 --- /dev/null +++ b/crates/rspack_napi/src/object.rs @@ -0,0 +1,128 @@ +use std::{ffi::CStr, ptr}; + +use napi::{ + Callback, Env, JsValue, PropertyAttributes, Result, + bindgen_prelude::{Object, ToNapiValue, check_status}, + sys, +}; + +#[derive(Clone)] +pub struct Property { + utf8_name: Option<&'static CStr>, + name: sys::napi_value, + getter: sys::napi_callback, + setter: sys::napi_callback, + method: sys::napi_callback, + attrs: PropertyAttributes, + value: sys::napi_value, +} + +impl Default for Property { + fn default() -> Self { + Property { + utf8_name: Default::default(), + name: ptr::null_mut(), + getter: Default::default(), + setter: Default::default(), + method: Default::default(), + attrs: Default::default(), + value: ptr::null_mut(), + } + } +} + +impl Property { + pub fn new() -> Self { + Default::default() + } + + #[inline] + pub fn with_utf8_name(mut self, name: &'static CStr) -> Result { + self.utf8_name = Some(name); + Ok(self) + } + + #[inline] + pub fn with_name(mut self, env: &Env, name: T) -> Result { + self.name = unsafe { T::to_napi_value(env.raw(), name)? }; + Ok(self) + } + + #[inline] + pub fn with_method(mut self, callback: Callback) -> Self { + self.method = Some(callback); + self + } + + #[inline] + pub fn with_getter(mut self, callback: Callback) -> Self { + self.getter = Some(callback); + self + } + + #[inline] + pub fn with_setter(mut self, callback: Callback) -> Self { + self.setter = Some(callback); + self + } + + #[inline] + pub fn with_property_attributes(mut self, attributes: PropertyAttributes) -> Self { + self.attrs = attributes; + self + } + + #[inline] + pub fn with_value<'env, T: JsValue<'env>>(mut self, value: &T) -> Self { + self.value = T::raw(value); + self + } + + #[inline] + pub fn with_napi_value(mut self, env: &Env, value: T) -> Result { + self.value = unsafe { T::to_napi_value(env.raw(), value)? }; + Ok(self) + } + + #[inline] + pub(crate) fn raw(&self) -> sys::napi_property_descriptor { + sys::napi_property_descriptor { + utf8name: match self.utf8_name { + Some(name) => name.as_ptr(), + None => ptr::null(), + }, + name: self.name, + method: self.method, + getter: self.getter, + setter: self.setter, + value: self.value, + attributes: self.attrs.into(), + data: ptr::null_mut(), + } + } +} + +/// Performance-optimized property definition that bypasses NAPI's closure overhead. +/// +/// Unlike the standard NAPI `define_properties`, this implementation: +/// - Does not support getter/setter closures +/// - Avoids closure detection and `napi_add_finalizer` calls +/// - Provides significant performance improvements for bulk property definitions +/// +/// Use this when defining multiple properties with static values or function pointers, +/// rather than closures that capture environment variables. +pub fn define_properties(env: &Env, object: &mut Object, properties: &[Property]) -> Result<()> { + let properties_iter = properties.iter().map(|property| property.raw()); + let env = env.raw(); + + check_status!(unsafe { + sys::napi_define_properties( + env, + object.value().value, + properties.len(), + properties_iter + .collect::>() + .as_ptr(), + ) + }) +}