Skip to content

Commit

Permalink
fix: should not panic when passing test option to SourceMapDevToolP…
Browse files Browse the repository at this point in the history
…lugin (#8136)

fix: panic when passing `test` option to SourceMapDevToolPlugin
  • Loading branch information
inottn authored Oct 16, 2024
1 parent 5fe525b commit dcfcea0
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 7 deletions.
7 changes: 4 additions & 3 deletions crates/rspack_binding_options/src/options/raw_devtool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rspack_plugin_devtool::{
Append, EvalDevToolModulePluginOptions, ModuleFilenameTemplate, ModuleFilenameTemplateFnCtx,
SourceMapDevToolPluginOptions, TestFn,
};
use tokio::runtime::Handle;

type RawAppend = Either3<String, bool, ThreadsafeFunction<RawPathData, String>>;

Expand Down Expand Up @@ -95,8 +94,10 @@ fn normalize_raw_module_filename_template(
}

fn normalize_raw_test(raw: ThreadsafeFunction<String, bool>) -> TestFn {
let handle = Handle::current();
Box::new(move |ctx| handle.block_on(raw.call(ctx)))
Box::new(move |ctx| {
let raw = raw.clone();
Box::pin(async move { raw.call(ctx).await })
})
}

#[napi(object, object_to_js = false)]
Expand Down
14 changes: 10 additions & 4 deletions crates/rspack_plugin_devtool/src/source_map_dev_tool_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ pub enum Append {
Disabled,
}

pub type TestFn = Box<dyn Fn(String) -> Result<bool> + Sync + Send>;
pub type TestFn = Box<dyn Fn(String) -> BoxFuture<'static, Result<bool>> + Sync + Send>;

#[derive(Derivative)]
#[derivative(Debug)]
Expand Down Expand Up @@ -173,11 +173,11 @@ impl SourceMapDevToolPlugin {
let output_options = &compilation.options.output;
let map_options = MapOptions::new(self.columns);

let mut mapped_sources = raw_assets
let futures = raw_assets
.into_par_iter()
.filter_map(|(file, asset)| {
.map(|(file, asset)| async {
let is_match = match &self.test {
Some(test) => match test(file.to_owned()) {
Some(test) => match test(file.to_owned()).await {
Ok(val) => val,
Err(e) => return Some(Err(e)),
},
Expand All @@ -193,6 +193,12 @@ impl SourceMapDevToolPlugin {
};
source.map(Ok)
})
.collect::<Vec<_>>();

let mut mapped_sources = join_all(futures)
.await
.into_iter()
.flatten()
.collect::<Result<Vec<_>>>()?;

let source_map_modules = mapped_sources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module.exports = {
},
plugins: [
new rspack.SourceMapDevToolPlugin({
test: /\.js/,
filename: "[file].map",
sourceRoot: path.join(__dirname, "folder") + "/"
}),
Expand Down

2 comments on commit dcfcea0

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Benchmark detail: Open

Name Base (2024-10-16 4ea491e) Current Change
10000_development-mode + exec 2.11 s ± 23 ms 2.14 s ± 31 ms +1.72 %
10000_development-mode_hmr + exec 662 ms ± 17 ms 664 ms ± 17 ms +0.31 %
10000_production-mode + exec 2.64 s ± 19 ms 2.65 s ± 24 ms +0.38 %
arco-pro_development-mode + exec 1.79 s ± 88 ms 1.86 s ± 60 ms +3.77 %
arco-pro_development-mode_hmr + exec 426 ms ± 1.5 ms 428 ms ± 8.5 ms +0.56 %
arco-pro_production-mode + exec 3.07 s ± 44 ms 3.17 s ± 104 ms +3.20 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.13 s ± 84 ms 3.24 s ± 79 ms +3.34 %
threejs_development-mode_10x + exec 1.61 s ± 22 ms 1.61 s ± 17 ms -0.38 %
threejs_development-mode_10x_hmr + exec 759 ms ± 9.2 ms 758 ms ± 21 ms -0.13 %
threejs_production-mode_10x + exec 4.9 s ± 23 ms 4.91 s ± 29 ms +0.32 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
rspress ✅ success
rslib ✅ success
rsbuild ✅ success
examples ✅ success
devserver ❌ failure

Please sign in to comment.