diff --git a/.vscode/settings.json b/.vscode/settings.json index 6b5e375a759..3410b4eeb0b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -42,5 +42,6 @@ "tokio", "ukey", "Ukey" - ] + ], + "rust-analyzer.checkOnSave": false } diff --git a/crates/rspack_binding_options/src/plugins/css_extract_additional_data.rs b/crates/rspack_binding_options/src/plugins/css_extract_additional_data.rs index 55a81a368c5..6063984837c 100644 --- a/crates/rspack_binding_options/src/plugins/css_extract_additional_data.rs +++ b/crates/rspack_binding_options/src/plugins/css_extract_additional_data.rs @@ -31,36 +31,43 @@ impl Debug for CssExtractRspackAdditionalDataPlugin { } #[plugin_hook(NormalModuleAdditionalData for CssExtractRspackAdditionalDataPlugin)] -async fn additional_data(&self, additional_data: &mut AdditionalData) -> Result<()> { - if !additional_data.contains::>() { +async fn additional_data(&self, additional_data: &mut Option<&mut AdditionalData>) -> Result<()> { + if !additional_data + .as_ref() + .is_some_and(|data| data.contains::>()) + { return Ok(()); } - let (tx, rx) = oneshot::channel::(); - let mut old_data = std::mem::take(additional_data); - self.js_callback.call(Box::new(move |env| { - if let Some(data) = old_data.get::>() - && let Ok(data) = data.get(env) - && let Ok(data) = data.coerce_to_object() - && let Ok(Some(data)) = data.get::<_, String>("css-extract-rspack-plugin") - { - let data_list: Vec = data - .split("__RSPACK_CSS_EXTRACT_SEP__") - .map(|info| { - serde_json::from_str(info) - .unwrap_or_else(|e| panic!("failed to parse CssExtractJsonData: {}", e)) - }) - .collect(); + if let Some(mut old_data) = additional_data.as_mut().map(|data| std::mem::take(*data)) { + let (tx, rx) = oneshot::channel::(); + self.js_callback.call(Box::new(move |env| { + if let Some(data) = old_data + .get::>() + .and_then(|data| data.get(env).ok()) + .and_then(|data| data.coerce_to_object().ok()) + .and_then(|data| data.get::<_, String>("css-extract-rspack-plugin").ok()) + .flatten() + { + let data_list: Vec = data + .split("__RSPACK_CSS_EXTRACT_SEP__") + .map(|info| { + serde_json::from_str(info) + .unwrap_or_else(|e| panic!("failed to parse CssExtractJsonData: {}", e)) + }) + .collect(); - old_data.insert(CssExtractJsonDataList(data_list)); - }; - tx.send(old_data) - .expect("should send `additional_data` for `CssExtractRspackAdditionalDataPlugin`"); - })); - let new_data = rx - .await - .expect("should receive `additional_data` for `CssExtractRspackAdditionalDataPlugin`"); - // ignore the default value here - let _ = std::mem::replace(additional_data, new_data); + old_data.insert(CssExtractJsonDataList(data_list)); + }; + tx.send(old_data) + .expect("should send `additional_data` for `CssExtractRspackAdditionalDataPlugin`"); + })); + let new_data = rx + .await + .expect("should receive `additional_data` for `CssExtractRspackAdditionalDataPlugin`"); + if let Some(data) = additional_data.as_mut() { + let _ = std::mem::replace(*data, new_data); + } + } Ok(()) } diff --git a/crates/rspack_binding_options/src/plugins/js_loader/context.rs b/crates/rspack_binding_options/src/plugins/js_loader/context.rs index 7f948a0c8f2..4315ca839a8 100644 --- a/crates/rspack_binding_options/src/plugins/js_loader/context.rs +++ b/crates/rspack_binding_options/src/plugins/js_loader/context.rs @@ -94,17 +94,17 @@ impl TryFrom<&mut LoaderContext> for JsLoaderContext { .to_js_module() .expect("CompilerModuleContext::to_js_module should not fail."), hot: cx.hot, - content: match &cx.content { + content: match cx.content() { Some(c) => Either::B(c.to_owned().into_bytes().into()), None => Either::A(Null), }, additional_data: cx - .additional_data - .get::>() + .additional_data() + .and_then(|data| data.get::>()) .cloned(), source_map: cx - .source_map - .clone() + .source_map() + .cloned() .map(|v| v.to_json()) .transpose() .map_err(|e| error!(e.to_string()))? diff --git a/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs b/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs index be0a6e2c002..7c055038f4e 100644 --- a/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs +++ b/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs @@ -63,6 +63,8 @@ pub fn get_builtin_loader(builtin: &str, options: Option<&str>) -> Result) -> Result, mut from: JsLoaderContext, ) -> Result<()> { - if let Some(data) = &from.additional_data { - to.additional_data.insert(data.clone()); - } to.cacheable = from.cacheable; to.file_dependencies = from.file_dependencies.into_iter().map(Into::into).collect(); to.context_dependencies = from @@ -66,16 +63,23 @@ pub(crate) fn merge_loader_context( .into_iter() .map(Into::into) .collect(); - to.content = match from.content { + + let content = match from.content { Either::A(_) => None, Either::B(c) => Some(rspack_core::Content::from(Into::>::into(c))), }; - to.source_map = from + let source_map = from .source_map .as_ref() .map(|s| rspack_core::rspack_sources::SourceMap::from_slice(s)) .transpose() .map_err(|e| error!(e.to_string()))?; + let additional_data = from.additional_data.take().map(|data| { + let mut additional = AdditionalData::default(); + additional.insert(data); + additional + }); + to.__finish_with((content, source_map, additional_data)); // update loader status to.loader_items = to @@ -90,6 +94,8 @@ pub(crate) fn merge_loader_context( to.set_pitch_executed() } to.set_data(from.data); + // JS loader should always be considered as finished + to.set_finish_called(); to }) .collect(); diff --git a/crates/rspack_core/src/normal_module.rs b/crates/rspack_core/src/normal_module.rs index b292f1196ce..d58bde1b827 100644 --- a/crates/rspack_core/src/normal_module.rs +++ b/crates/rspack_core/src/normal_module.rs @@ -85,7 +85,7 @@ define_hook!(NormalModuleLoader: SyncSeries(loader_context: &mut LoaderContext) -> bool); define_hook!(NormalModuleLoaderStartYielding: AsyncSeries(loader_context: &mut LoaderContext)); define_hook!(NormalModuleBeforeLoaders: SyncSeries(module: &mut NormalModule)); -define_hook!(NormalModuleAdditionalData: AsyncSeries(additional_data: &mut AdditionalData)); +define_hook!(NormalModuleAdditionalData: AsyncSeries(additional_data: &mut Option<&mut AdditionalData>)); #[derive(Debug, Default)] pub struct NormalModuleHooks { @@ -419,14 +419,11 @@ impl Module for NormalModule { current_loader: Default::default(), }); - let additional_data = AdditionalData::default(); - let loader_result = run_loaders( self.loaders.clone(), self.resource_data.clone(), Some(plugin.clone()), build_context.runner_context, - additional_data, ) .await; let (mut loader_result, ds) = match loader_result { @@ -457,7 +454,7 @@ impl Module for NormalModule { .plugin_driver .normal_module_hooks .additional_data - .call(&mut loader_result.additional_data) + .call(&mut loader_result.additional_data.as_mut()) .await?; self.add_diagnostics(ds); diff --git a/crates/rspack_core/src/parser_and_generator.rs b/crates/rspack_core/src/parser_and_generator.rs index 271999144ec..809d4bb97bf 100644 --- a/crates/rspack_core/src/parser_and_generator.rs +++ b/crates/rspack_core/src/parser_and_generator.rs @@ -32,7 +32,7 @@ pub struct ParseContext<'a> { pub loaders: &'a [BoxLoader], pub resource_data: &'a ResourceData, pub compiler_options: &'a CompilerOptions, - pub additional_data: AdditionalData, + pub additional_data: Option, pub build_info: &'a mut BuildInfo, pub build_meta: &'a mut BuildMeta, } diff --git a/crates/rspack_loader_lightningcss/src/lib.rs b/crates/rspack_loader_lightningcss/src/lib.rs index 9aec6700a3c..49960a725b2 100644 --- a/crates/rspack_loader_lightningcss/src/lib.rs +++ b/crates/rspack_loader_lightningcss/src/lib.rs @@ -49,7 +49,7 @@ impl LightningCssLoader { let filename = resource_path.as_str().to_string(); - let Some(content) = std::mem::take(&mut loader_context.content) else { + let Some(content) = loader_context.take_content() else { return Ok(()); }; @@ -131,8 +131,7 @@ impl LightningCssLoader { let enable_sourcemap = loader_context.context.module_source_map_kind.enabled(); let mut source_map = loader_context - .source_map - .as_ref() + .source_map() .map(|input_source_map| -> Result<_> { let mut sm = parcel_sourcemap::SourceMap::new( input_source_map @@ -180,15 +179,17 @@ impl LightningCssLoader { }) .map_err(|_| rspack_error::error!("failed to generate css"))?; - loader_context.content = Some(rspack_core::Content::String(content.code)); - if enable_sourcemap { let source_map = source_map .to_json(None) .map_err(|e| rspack_error::error!(e.to_string()))?; - loader_context.source_map = - Some(SourceMap::from_json(&source_map).expect("should be able to generate source-map")); + loader_context.finish_with(( + content.code, + SourceMap::from_json(&source_map).expect("should be able to generate source-map"), + )); + } else { + loader_context.finish_with(content.code); } Ok(()) diff --git a/crates/rspack_loader_preact_refresh/src/lib.rs b/crates/rspack_loader_preact_refresh/src/lib.rs index eb77a20f8ff..8c06dba9e33 100644 --- a/crates/rspack_loader_preact_refresh/src/lib.rs +++ b/crates/rspack_loader_preact_refresh/src/lib.rs @@ -27,11 +27,13 @@ impl PreactRefreshLoader { #[async_trait::async_trait] impl Loader for PreactRefreshLoader { async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { - let content = std::mem::take(&mut loader_context.content).expect("Content should be available"); + let content = loader_context + .take_content() + .expect("Content should be available"); let mut source = content.try_into_string()?; source += "\n"; source += include_str!("runtime.js"); - loader_context.content = Some(source.into()); + loader_context.finish_with(source); Ok(()) } } diff --git a/crates/rspack_loader_react_refresh/src/lib.rs b/crates/rspack_loader_react_refresh/src/lib.rs index ba85db67918..8f92ec12bd0 100644 --- a/crates/rspack_loader_react_refresh/src/lib.rs +++ b/crates/rspack_loader_react_refresh/src/lib.rs @@ -27,7 +27,7 @@ impl ReactRefreshLoader { #[async_trait::async_trait] impl Loader for ReactRefreshLoader { async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { - let Some(content) = std::mem::take(&mut loader_context.content) else { + let Some(content) = loader_context.take_content() else { return Ok(()); }; let mut source = content.try_into_string()?; @@ -42,7 +42,7 @@ Promise.resolve().then(function() { $ReactRefreshRuntime$.refresh(__webpack_module__.id, __webpack_module__.hot); }); "#; - loader_context.content = Some(source.into()); + loader_context.finish_with(source); Ok(()) } } diff --git a/crates/rspack_loader_runner/src/context.rs b/crates/rspack_loader_runner/src/context.rs index 162c2539299..573693c250a 100644 --- a/crates/rspack_loader_runner/src/context.rs +++ b/crates/rspack_loader_runner/src/context.rs @@ -37,14 +37,14 @@ impl State { pub struct LoaderContext { pub hot: bool, pub resource_data: Arc, - - pub content: Option, #[derivative(Debug = "ignore")] pub context: Context, - pub source_map: Option, - pub additional_data: AdditionalData, - pub cacheable: bool, + pub(crate) content: Option, + pub(crate) source_map: Option, + pub(crate) additional_data: Option, + + pub cacheable: bool, pub file_dependencies: HashSet, pub context_dependencies: HashSet, pub missing_dependencies: HashSet, @@ -118,8 +118,196 @@ impl LoaderContext { self.resource_data.resource_fragment.as_deref() } + pub fn content(&self) -> Option<&Content> { + self.content.as_ref() + } + + pub fn source_map(&self) -> Option<&SourceMap> { + self.source_map.as_ref() + } + + pub fn additional_data(&self) -> Option<&AdditionalData> { + self.additional_data.as_ref() + } + + pub fn take_content(&mut self) -> Option { + self.content.take() + } + + pub fn take_source_map(&mut self) -> Option { + self.source_map.take() + } + + pub fn take_additional_data(&mut self) -> Option { + self.additional_data.take() + } + + pub fn take_all(&mut self) -> (Option, Option, Option) { + ( + self.content.take(), + self.source_map.take(), + self.additional_data.take(), + ) + } + + pub fn finish_with(&mut self, patch: impl Into) { + self.__finish_with(patch); + self.current_loader().set_finish_called(); + } + + pub fn finish_with_empty(&mut self) { + self.content = None; + self.source_map = None; + self.additional_data = None; + self.current_loader().set_finish_called(); + } + #[inline] pub fn state(&self) -> State { self.state } + + #[doc(hidden)] + pub fn __finish_with(&mut self, patch: impl Into) { + let patch = patch.into(); + self.content = patch.content; + self.source_map = patch.source_map; + self.additional_data = patch.additional_data; + } +} + +pub struct LoaderPatch { + pub(crate) content: Option, + pub(crate) source_map: Option, + pub(crate) additional_data: Option, +} + +impl From for LoaderPatch +where + T: Into, +{ + fn from(content: T) -> Self { + Self { + content: Some(content.into()), + source_map: None, + additional_data: None, + } + } +} + +impl From<(T, SourceMap)> for LoaderPatch +where + T: Into, +{ + fn from(value: (T, SourceMap)) -> Self { + Self { + content: Some(value.0.into()), + source_map: Some(value.1), + additional_data: None, + } + } +} + +impl From<(T, Option)> for LoaderPatch +where + T: Into, +{ + fn from(value: (T, Option)) -> Self { + Self { + content: Some(value.0.into()), + source_map: value.1, + additional_data: None, + } + } +} + +impl From<(T, SourceMap, AdditionalData)> for LoaderPatch +where + T: Into, +{ + fn from(value: (T, SourceMap, AdditionalData)) -> Self { + Self { + content: Some(value.0.into()), + source_map: Some(value.1), + additional_data: Some(value.2), + } + } +} + +impl From<(T, Option, Option)> for LoaderPatch +where + T: Into, +{ + fn from(value: (T, Option, Option)) -> Self { + Self { + content: Some(value.0.into()), + source_map: value.1, + additional_data: value.2, + } + } +} + +impl From> for LoaderPatch +where + T: Into, +{ + fn from(content: Option) -> Self { + Self { + content: content.map(|c| c.into()), + source_map: None, + additional_data: None, + } + } +} + +impl From<(Option, SourceMap)> for LoaderPatch +where + T: Into, +{ + fn from(value: (Option, SourceMap)) -> Self { + Self { + content: value.0.map(|c| c.into()), + source_map: Some(value.1), + additional_data: None, + } + } +} + +impl From<(Option, Option)> for LoaderPatch +where + T: Into, +{ + fn from(value: (Option, Option)) -> Self { + Self { + content: value.0.map(|c| c.into()), + source_map: value.1, + additional_data: None, + } + } +} + +impl From<(Option, SourceMap, AdditionalData)> for LoaderPatch +where + T: Into, +{ + fn from(value: (Option, SourceMap, AdditionalData)) -> Self { + Self { + content: value.0.map(|c| c.into()), + source_map: Some(value.1), + additional_data: Some(value.2), + } + } +} + +impl From<(Option, Option, Option)> for LoaderPatch +where + T: Into, +{ + fn from(value: (Option, Option, Option)) -> Self { + Self { + content: value.0.map(|c| c.into()), + source_map: value.1, + additional_data: value.2, + } + } } diff --git a/crates/rspack_loader_runner/src/loader.rs b/crates/rspack_loader_runner/src/loader.rs index 749d7de29c2..787a3d1b2eb 100644 --- a/crates/rspack_loader_runner/src/loader.rs +++ b/crates/rspack_loader_runner/src/loader.rs @@ -39,6 +39,14 @@ pub struct LoaderItem { r#type: String, pitch_executed: AtomicBool, normal_executed: AtomicBool, + /// Whether loader was called with [LoaderContext::finish_with]. + /// + /// Indicates that the loader has finished its work, + /// otherwise loader runner will reset [`LoaderContext::content`], [`LoaderContext::source_map`], [`LoaderContext::additional_data`]. + /// + /// This flag is used to align with webpack's behavior: + /// If nothing is modified in the loader, the loader will reset the content, source map, and additional data. + finish_called: AtomicBool, } impl LoaderItem { @@ -62,11 +70,13 @@ impl LoaderItem { } #[inline] + #[doc(hidden)] pub fn set_data(&mut self, data: serde_json::Value) { self.data = data; } #[inline] + #[doc(hidden)] pub fn pitch_executed(&self) -> bool { self.pitch_executed.load(Ordering::Relaxed) } @@ -77,14 +87,28 @@ impl LoaderItem { } #[inline] + #[doc(hidden)] + pub fn finish_called(&self) -> bool { + self.finish_called.load(Ordering::Relaxed) + } + + #[inline] + #[doc(hidden)] pub fn set_pitch_executed(&self) { self.pitch_executed.store(true, Ordering::Relaxed) } #[inline] + #[doc(hidden)] pub fn set_normal_executed(&self) { self.normal_executed.store(true, Ordering::Relaxed) } + + #[inline] + #[doc(hidden)] + pub fn set_finish_called(&self) { + self.finish_called.store(true, Ordering::Relaxed) + } } impl Display for LoaderItem { @@ -141,9 +165,14 @@ impl Identifiable for LoaderItem { } #[async_trait] -pub trait Loader: Identifiable + Send + Sync { - async fn run(&self, _loader_context: &mut LoaderContext) -> Result<()> { - // noop +pub trait Loader: Identifiable + Send + Sync +where + Context: Send, +{ + async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { + // If loader does not implement normal stage, + // it should inherit the result from the previous loader. + loader_context.current_loader().set_finish_called(); Ok(()) } async fn pitch(&self, _loader_context: &mut LoaderContext) -> Result<()> { @@ -177,6 +206,7 @@ impl From>> for LoaderItem { r#type: r#type.to_string(), pitch_executed: AtomicBool::new(false), normal_executed: AtomicBool::new(false), + finish_called: AtomicBool::new(false), }; } let ident = loader.identifier(); @@ -195,6 +225,7 @@ impl From>> for LoaderItem { r#type: String::default(), pitch_executed: AtomicBool::new(false), normal_executed: AtomicBool::new(false), + finish_called: AtomicBool::new(false), } } } diff --git a/crates/rspack_loader_runner/src/runner.rs b/crates/rspack_loader_runner/src/runner.rs index d096b30538d..83e55a53894 100644 --- a/crates/rspack_loader_runner/src/runner.rs +++ b/crates/rspack_loader_runner/src/runner.rs @@ -62,7 +62,6 @@ async fn create_loader_context( resource_data: Arc, plugin: Option>>, context: Context, - additional_data: AdditionalData, ) -> Result> { let mut file_dependencies: HashSet = Default::default(); if let Some(resource_path) = &resource_data.resource_path @@ -81,7 +80,7 @@ async fn create_loader_context( content: None, context, source_map: None, - additional_data, + additional_data: None, state: State::Init, loader_index: 0, loader_items, @@ -102,15 +101,13 @@ pub async fn run_loaders( resource_data: Arc, plugins: Option>>, context: Context, - additional_data: AdditionalData, ) -> Result> { let loaders = loaders .into_iter() .map(|i| i.into()) .collect::>>(); - let mut cx = - create_loader_context(loaders, resource_data, plugins, context, additional_data).await?; + let mut cx = create_loader_context(loaders, resource_data, plugins, context).await?; loop { match cx.state { @@ -173,6 +170,12 @@ pub async fn run_loaders( cx.current_loader().set_normal_executed(); let loader = cx.current_loader().loader().clone(); loader.run(&mut cx).await?; + if !cx.current_loader().finish_called() { + // If nothing is returned from this loader, + // we set everything to [None] and move to the next loader. + // This mocks the behavior of webpack loader-runner. + cx.finish_with_empty(); + } } State::Finished => break, } @@ -190,7 +193,7 @@ pub struct LoaderResult { pub build_dependencies: HashSet, pub content: Content, pub source_map: Option, - pub additional_data: AdditionalData, + pub additional_data: Option, } impl TryFrom> for TWithDiagnosticArray { @@ -230,7 +233,7 @@ mod test { use rspack_error::Result; use super::{run_loaders, Loader, LoaderContext, ResourceData}; - use crate::{content::Content, plugin::LoaderRunnerPlugin}; + use crate::{content::Content, plugin::LoaderRunnerPlugin, AdditionalData}; struct TestContentPlugin; @@ -404,15 +407,16 @@ mod test { encoded_content: None, }); - run_loaders( + // Ignore error: Final loader didn't return a Buffer or String + assert!(run_loaders( vec![p1, p2, c1, c2], rs.clone(), Some(Arc::new(TestContentPlugin)), (), - Default::default(), ) .await - .unwrap(); + .err() + .is_some()); IDENTS.with(|i| assert_eq!(*i.borrow(), &["pitch1", "pitch2", "normal2", "normal1"])); IDENTS.with(|i| i.borrow_mut().clear()); @@ -420,15 +424,16 @@ mod test { let p2 = Arc::new(PitchNormal) as Arc>; let p3 = Arc::new(PitchNormal2) as Arc>; - run_loaders( + // Ignore error: Final loader didn't return a Buffer or String + assert!(run_loaders( vec![p1, p2, p3], rs.clone(), Some(Arc::new(TestContentPlugin)), (), - Default::default(), ) .await - .unwrap(); + .err() + .is_some()); IDENTS.with(|i| { // should not execute p3, as p2 pitched successfully. assert!(!i.borrow().contains(&"pitch-normal-normal-2".to_string())); @@ -456,8 +461,14 @@ mod test { #[async_trait::async_trait] impl Loader<()> for Normal { async fn run(&self, loader_context: &mut LoaderContext<()>) -> Result<()> { - let data = loader_context.additional_data.get::<&str>().unwrap(); + let data = loader_context + .additional_data + .as_ref() + .unwrap() + .get::<&str>() + .unwrap(); assert_eq!(*data, "additional-data"); + loader_context.finish_with(("".to_string(), None, None)); Ok(()) } } @@ -473,7 +484,9 @@ mod test { #[async_trait::async_trait] impl Loader<()> for Normal2 { async fn run(&self, loader_context: &mut LoaderContext<()>) -> Result<()> { - loader_context.additional_data.insert("additional-data"); + let mut additional_data: AdditionalData = Default::default(); + additional_data.insert("additional-data"); + loader_context.finish_with(("".to_string(), None, Some(additional_data))); Ok(()) } } @@ -496,9 +509,71 @@ mod test { rs, Some(Arc::new(TestContentPlugin)), (), - Default::default(), ) .await .unwrap(); } + + #[tokio::test] + async fn should_override_data_if_finish_with_is_not_called() { + struct Normal; + + impl Identifiable for Normal { + fn identifier(&self) -> Identifier { + "/rspack/normal-loader1".into() + } + } + + #[async_trait::async_trait] + impl Loader<()> for Normal { + async fn run(&self, loader_context: &mut LoaderContext<()>) -> Result<()> { + assert!(loader_context.content.is_some()); + // Does not call `LoaderContext::finish_with` + Ok(()) + } + } + + let rs = Arc::new(ResourceData { + scheme: OnceCell::new(), + resource: "/rspack/main.js?abc=123#efg".to_owned(), + resource_description: None, + resource_fragment: None, + resource_query: None, + resource_path: Default::default(), + mimetype: None, + parameters: None, + encoding: None, + encoded_content: None, + }); + + struct Normal2; + + impl Identifiable for Normal2 { + fn identifier(&self) -> Identifier { + "/rspack/normal-loader2".into() + } + } + + #[async_trait::async_trait] + impl Loader<()> for Normal2 { + async fn run(&self, loader_context: &mut LoaderContext<()>) -> Result<()> { + let (content, source_map, additional_data) = loader_context.take_all(); + assert!(content.is_none()); + assert!(source_map.is_none()); + assert!(additional_data.is_none()); + Ok(()) + } + } + + // Ignore error: Final loader didn't return a Buffer or String + assert!(run_loaders( + vec![Arc::new(Normal2), Arc::new(Normal)], + rs, + Some(Arc::new(TestContentPlugin)), + (), + ) + .await + .err() + .is_some()); + } } diff --git a/crates/rspack_loader_swc/src/lib.rs b/crates/rspack_loader_swc/src/lib.rs index bb644f0d538..4f471c106eb 100644 --- a/crates/rspack_loader_swc/src/lib.rs +++ b/crates/rspack_loader_swc/src/lib.rs @@ -48,7 +48,7 @@ impl SwcLoader { .resource_path() .map(|p| p.to_path_buf()) .unwrap_or_default(); - let Some(content) = std::mem::take(&mut loader_context.content) else { + let Some(content) = loader_context.take_content() else { return Ok(()); }; @@ -64,7 +64,7 @@ impl SwcLoader { .transform .merge(MergingOption::from(Some(transform))); } - if let Some(pre_source_map) = loader_context.source_map.clone() { + if let Some(pre_source_map) = loader_context.source_map().cloned() { if let Ok(source_map) = pre_source_map.to_json() { swc_options.config.input_source_map = Some(InputSourceMap::Str(source_map)) } @@ -133,12 +133,11 @@ impl SwcLoader { let ast = c.into_js_ast(program); let TransformOutput { code, map } = ast::stringify(&ast, codegen_options)?; - loader_context.content = Some(code.into()); let map = map .map(|m| SourceMap::from_json(&m)) .transpose() .map_err(|e| error!(e.to_string()))?; - loader_context.source_map = map; + loader_context.finish_with((code, map)); Ok(()) } diff --git a/crates/rspack_loader_testing/src/lib.rs b/crates/rspack_loader_testing/src/lib.rs index f4cd8101dfd..3c0c55d0e02 100644 --- a/crates/rspack_loader_testing/src/lib.rs +++ b/crates/rspack_loader_testing/src/lib.rs @@ -10,11 +10,11 @@ pub struct SimpleLoader; #[async_trait] impl Loader for SimpleLoader { async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { - let Some(content) = loader_context.content.take() else { + let Some(content) = loader_context.take_content() else { return Ok(()); }; let export = format!("{}-simple", content.try_into_string()?); - loader_context.content = Some(format!("module.exports = {}", json!(export)).into()); + loader_context.finish_with(format!("module.exports = {}", json!(export))); Ok(()) } } @@ -29,10 +29,10 @@ pub struct SimpleAsyncLoader; #[async_trait] impl Loader for SimpleAsyncLoader { async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { - let Some(content) = loader_context.content.take() else { + let Some(content) = loader_context.take_content() else { return Ok(()); }; - loader_context.content = Some(format!("{}-async-simple", content.try_into_string()?).into()); + loader_context.finish_with(format!("{}-async-simple", content.try_into_string()?)); Ok(()) } } @@ -47,15 +47,14 @@ pub struct PitchingLoader; #[async_trait] impl Loader for PitchingLoader { async fn pitch(&self, loader_context: &mut LoaderContext) -> Result<()> { - loader_context.content = Some( + loader_context.finish_with( [ loader_context .remaining_request() .display_with_suffix(loader_context.resource()), loader_context.previous_request().to_string(), ] - .join(":") - .into(), + .join(":"), ); Ok(()) } @@ -66,3 +65,35 @@ impl Identifiable for PitchingLoader { } } pub const PITCHING_LOADER_IDENTIFIER: &str = "builtin:test-pitching-loader"; + +pub struct PassthroughLoader; +#[async_trait] +impl Loader for PassthroughLoader { + async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { + let patch_data = loader_context.take_all(); + loader_context.finish_with(patch_data); + Ok(()) + } +} +impl Identifiable for PassthroughLoader { + fn identifier(&self) -> Identifier { + PASS_THROUGH_LOADER_IDENTIFIER.into() + } +} +pub const PASS_THROUGH_LOADER_IDENTIFIER: &str = "builtin:test-passthrough-loader"; + +pub struct NoPassthroughLoader; +#[async_trait] +impl Loader for NoPassthroughLoader { + async fn run(&self, loader_context: &mut LoaderContext) -> Result<()> { + let (content, _, _) = loader_context.take_all(); + loader_context.finish_with(content); + Ok(()) + } +} +impl Identifiable for NoPassthroughLoader { + fn identifier(&self) -> Identifier { + NO_PASS_THROUGH_LOADER_IDENTIFIER.into() + } +} +pub const NO_PASS_THROUGH_LOADER_IDENTIFIER: &str = "builtin:test-no-passthrough-loader"; diff --git a/crates/rspack_plugin_extract_css/src/parser_plugin.rs b/crates/rspack_plugin_extract_css/src/parser_plugin.rs index 8d6e5e1c568..0a1b497591e 100644 --- a/crates/rspack_plugin_extract_css/src/parser_plugin.rs +++ b/crates/rspack_plugin_extract_css/src/parser_plugin.rs @@ -28,7 +28,10 @@ pub struct PluginCssExtractParserPlugin { impl JavascriptParserPlugin for PluginCssExtractParserPlugin { fn finish(&self, parser: &mut JavascriptParser) -> Option { - let deps = if let Some(additional_data) = parser.additional_data.get::() + let deps = if let Some(additional_data) = parser + .additional_data + .as_ref() + .and_then(|data| data.get::()) { if let Some(deps) = self.cache.get(additional_data) { deps.clone() diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs index d4842c278ab..090270c2926 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs @@ -56,7 +56,7 @@ pub fn scan_dependencies( semicolons: &mut FxHashSet, unresolved_mark: Mark, parser_plugins: &mut Vec, - additional_data: AdditionalData, + additional_data: Option, ) -> Result>> { let mut parser = JavascriptParser::new( source_map, diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs index 8404ecc3384..379cb801f23 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs @@ -212,7 +212,7 @@ pub struct JavascriptParser<'parser> { #[allow(clippy::vec_box)] pub(crate) blocks: Vec>, // TODO: remove `additional_data` once we have builtin:css-extract-loader - pub additional_data: AdditionalData, + pub additional_data: Option, pub(crate) comments: Option<&'parser dyn Comments>, pub(crate) worker_index: u32, pub(crate) build_meta: &'parser mut BuildMeta, @@ -265,7 +265,7 @@ impl<'parser> JavascriptParser<'parser> { semicolons: &'parser mut FxHashSet, unresolved_mark: Mark, parser_plugins: &'parser mut Vec, - additional_data: AdditionalData, + additional_data: Option, ) -> Self { let warning_diagnostics: Vec> = Vec::with_capacity(4); let errors = Vec::with_capacity(4); diff --git a/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.css b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.css new file mode 100644 index 00000000000..cd1bf79c24f --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.css @@ -0,0 +1,4 @@ +.foo { + user-select: none; + font-size: 32px; +} diff --git a/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.js b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.js new file mode 100644 index 00000000000..e4dcfca4d13 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/index.js @@ -0,0 +1,24 @@ +import './index.css' + +const fs = __non_webpack_require__("node:fs"); +const path = __non_webpack_require__("node:path"); + +it("should transform CSS and add prefixes correctly", () => { + const css = fs.readFileSync( + path.resolve(__dirname, "./bundle0.css"), + "utf-8" + ); + + expect(css.includes('-ms-user-select: none;')).toBeTruthy(); + expect(css.includes('user-select: none;')).toBeTruthy(); +}); + +it("should perform px to rem transformation", () => { + const css = fs.readFileSync( + path.resolve(__dirname, "./bundle0.css"), + "utf-8" + ); + + expect(css.includes('px')).toBeFalsy(); + expect(css.includes('rem')).toBeTruthy(); +}); diff --git a/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/rspack.config.js b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/rspack.config.js new file mode 100644 index 00000000000..12c98b1c362 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/builtin-lightningcss-loader/wth-css-extract-postcss/rspack.config.js @@ -0,0 +1,46 @@ +const rspack = require("@rspack/core"); + +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + target: 'web', + node: false, + module: { + rules: [ + { + test: /\.css$/, + use: [ + rspack.CssExtractRspackPlugin.loader, + "css-loader", + { + loader: "builtin:lightningcss-loader", + /** @type {import("@rspack/core").LightningcssLoaderOptions} */ + options: { + targets: [ + 'Edge >= 12' + ] + } + }, + { + loader: "postcss-loader", + options: { + postcssOptions: { + plugins: [ + "postcss-pxtorem" + ] + } + } + } + ], + type: "javascript/auto" + } + ] + }, + plugins: [ + new rspack.CssExtractRspackPlugin({ + filename: 'bundle0.css' + }) + ], + experiments: { + css: true + } +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/a.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/a.js new file mode 100644 index 00000000000..6cd1d0075d4 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/a.js @@ -0,0 +1 @@ +module.exports = "a"; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/index.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/index.js new file mode 100644 index 00000000000..75cdb63c65d --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/index.js @@ -0,0 +1,5 @@ +it("should not passthrough additional data if builtin loader didn't reuse additional data", () => { + let result = require("./a"); + expect(Object.keys(result)).not.toContain("a"); + expect(Object.keys(result)).toContain("b"); +}); diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-1.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-1.js new file mode 100644 index 00000000000..9d88459441c --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-1.js @@ -0,0 +1,5 @@ +module.exports = function (content, sourceMap, additionalData) { + this.callback(null, content, null, { + a: "a" + }); +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-2.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-2.js new file mode 100644 index 00000000000..50d47afe5eb --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/loader-2.js @@ -0,0 +1,10 @@ +module.exports = function (content, sourceMap, additionalData) { + this.callback( + null, + `module.exports = ${JSON.stringify({ + ...additionalData, + b: "b" + })}`, + null + ); +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/rspack.config.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/rspack.config.js new file mode 100644 index 00000000000..b3a4b38a2bc --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-no-passthrough/rspack.config.js @@ -0,0 +1,16 @@ +const path = require("path"); + +/** + * @type {import('@rspack/core').RspackOptions} + */ +module.exports = { + context: __dirname, + module: { + rules: [ + { + test: path.join(__dirname, "a.js"), + use: [{ loader: "./loader-2.js" }, { loader: "builtin:test-no-passthrough-loader" }, { loader: "./loader-1.js" }] + } + ] + } +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/a.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/a.js new file mode 100644 index 00000000000..6cd1d0075d4 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/a.js @@ -0,0 +1 @@ +module.exports = "a"; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/index.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/index.js new file mode 100644 index 00000000000..d2b47aacafe --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/index.js @@ -0,0 +1,7 @@ +it("should pass additional data between loaders if builtin loader passes through its additional data", () => { + let result = require("./a"); + expect(result).toEqual({ + a: "a", + b: "b" + }); +}); diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-1.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-1.js new file mode 100644 index 00000000000..9d88459441c --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-1.js @@ -0,0 +1,5 @@ +module.exports = function (content, sourceMap, additionalData) { + this.callback(null, content, null, { + a: "a" + }); +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-2.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-2.js new file mode 100644 index 00000000000..50d47afe5eb --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/loader-2.js @@ -0,0 +1,10 @@ +module.exports = function (content, sourceMap, additionalData) { + this.callback( + null, + `module.exports = ${JSON.stringify({ + ...additionalData, + b: "b" + })}`, + null + ); +}; diff --git a/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/rspack.config.js b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/rspack.config.js new file mode 100644 index 00000000000..9e66fb89655 --- /dev/null +++ b/packages/rspack-test-tools/tests/configCases/loader/additional-data-passthrough/rspack.config.js @@ -0,0 +1,16 @@ +const path = require("path"); + +/** + * @type {import('@rspack/core').RspackOptions} + */ +module.exports = { + context: __dirname, + module: { + rules: [ + { + test: path.join(__dirname, "a.js"), + use: [{ loader: "./loader-2.js" }, { loader: "builtin:test-passthrough-loader" }, { loader: "./loader-1.js" }] + } + ] + } +}; diff --git a/scripts/debug/launch.mjs b/scripts/debug/launch.mjs index fb0dd7eb63b..7d5c8cc4a6c 100644 --- a/scripts/debug/launch.mjs +++ b/scripts/debug/launch.mjs @@ -33,7 +33,7 @@ export async function launchJestWithArgs(additionalArgs) { "--expose-gc", "--max-old-space-size=8192", "--experimental-vm-modules", - "${workspaceFolder}/node_modules/.bin/jest", + "${workspaceFolder}/node_modules/jest-cli/bin/jest", "--runInBand", "--logHeapUsage" ];