fix: loader intermediate data should be overridden between loaders (w…

…eb-infra-dev#7846)

* fix: init

* test: add

.vscode/settings.json

@@ -42,5 +42,6 @@
     "tokio",
     "ukey",
     "Ukey"
-  ]
+  ],
+  "rust-analyzer.checkOnSave": false
 }

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::<ThreadsafeJsValueRef<Unknown>>() {
+async fn additional_data(&self, additional_data: &mut Option<&mut AdditionalData>) -> Result<()> {
+  if !additional_data
+    .as_ref()
+    .is_some_and(|data| data.contains::<ThreadsafeJsValueRef<Unknown>>())
+  {
     return Ok(());
   }
-  let (tx, rx) = oneshot::channel::<AdditionalData>();
-  let mut old_data = std::mem::take(additional_data);
-  self.js_callback.call(Box::new(move |env| {
-    if let Some(data) = old_data.get::<ThreadsafeJsValueRef<Unknown>>()
-      && 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<rspack_plugin_extract_css::CssExtractJsonData> = 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::<AdditionalData>();
+    self.js_callback.call(Box::new(move |env| {
+      if let Some(data) = old_data
+        .get::<ThreadsafeJsValueRef<Unknown>>()
+        .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<rspack_plugin_extract_css::CssExtractJsonData> = 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(())
 }
 

crates/rspack_binding_options/src/plugins/js_loader/context.rs

@@ -94,17 +94,17 @@ impl TryFrom<&mut LoaderContext<RunnerContext>> 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::<ThreadsafeJsValueRef<Unknown>>()
+        .additional_data()
+        .and_then(|data| data.get::<ThreadsafeJsValueRef<Unknown>>())
         .cloned(),
       source_map: cx
-        .source_map
-        .clone()
+        .source_map()
+        .cloned()
         .map(|v| v.to_json())
         .transpose()
         .map_err(|e| error!(e.to_string()))?

crates/rspack_binding_options/src/plugins/js_loader/resolver.rs

@@ -75,6 +75,8 @@ pub fn get_builtin_loader(builtin: &str, options: Option<&str>) -> Result<BoxLoa
       rspack_loader_preact_refresh::PreactRefreshLoader::default().with_identifier(builtin.into()),
     ));
   }
+
+  // TODO: should be compiled with a different cfg
   if builtin.starts_with(rspack_loader_testing::SIMPLE_ASYNC_LOADER_IDENTIFIER) {
     return Ok(Arc::new(rspack_loader_testing::SimpleAsyncLoader));
   }
@@ -84,6 +86,12 @@ pub fn get_builtin_loader(builtin: &str, options: Option<&str>) -> Result<BoxLoa
   if builtin.starts_with(rspack_loader_testing::PITCHING_LOADER_IDENTIFIER) {
     return Ok(Arc::new(rspack_loader_testing::PitchingLoader));
   }
+  if builtin.starts_with(rspack_loader_testing::PASS_THROUGH_LOADER_IDENTIFIER) {
+    return Ok(Arc::new(rspack_loader_testing::PassthroughLoader));
+  }
+  if builtin.starts_with(rspack_loader_testing::NO_PASS_THROUGH_LOADER_IDENTIFIER) {
+    return Ok(Arc::new(rspack_loader_testing::NoPassthroughLoader));
+  }
   unreachable!("Unexpected builtin loader: {builtin}")
 }
 

crates/rspack_binding_options/src/plugins/js_loader/scheduler.rs

@@ -1,7 +1,7 @@
 use napi::Either;
 use rspack_core::{
-  LoaderContext, NormalModuleLoaderShouldYield, NormalModuleLoaderStartYielding, RunnerContext,
-  BUILTIN_LOADER_PREFIX,
+  AdditionalData, LoaderContext, NormalModuleLoaderShouldYield, NormalModuleLoaderStartYielding,
+  RunnerContext, BUILTIN_LOADER_PREFIX,
 };
 use rspack_error::{error, Result};
 use rspack_hook::plugin_hook;
@@ -46,9 +46,6 @@ pub(crate) fn merge_loader_context(
   to: &mut LoaderContext<RunnerContext>,
   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::<Vec<u8>>::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();

crates/rspack_core/src/normal_module.rs

@@ -76,7 +76,7 @@ define_hook!(NormalModuleLoader: SyncSeries(loader_context: &mut LoaderContext<R
 define_hook!(NormalModuleLoaderShouldYield: SyncSeriesBail(loader_context: &LoaderContext<RunnerContext>) -> bool);
 define_hook!(NormalModuleLoaderStartYielding: AsyncSeries(loader_context: &mut LoaderContext<RunnerContext>));
 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 {
@@ -410,14 +410,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 {
@@ -448,7 +445,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);
 

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<AdditionalData>,
   pub build_info: &'a mut BuildInfo,
   pub build_meta: &'a mut BuildMeta,
 }

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(())

crates/rspack_loader_preact_refresh/src/lib.rs

@@ -27,11 +27,13 @@ impl PreactRefreshLoader {
 #[async_trait::async_trait]
 impl Loader<RunnerContext> for PreactRefreshLoader {
   async fn run(&self, loader_context: &mut LoaderContext<RunnerContext>) -> 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(())
   }
 }

crates/rspack_loader_react_refresh/src/lib.rs

@@ -27,7 +27,7 @@ impl ReactRefreshLoader {
 #[async_trait::async_trait]
 impl Loader<RunnerContext> for ReactRefreshLoader {
   async fn run(&self, loader_context: &mut LoaderContext<RunnerContext>) -> 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(())
   }
 }