Skip to content

Commit

Permalink
fix: should resolve aliased loader module with query (#7660)
Browse files Browse the repository at this point in the history
  • Loading branch information
h-a-n-a authored Aug 23, 2024
1 parent 14154cc commit 076cf9d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 11 deletions.
23 changes: 17 additions & 6 deletions crates/rspack_binding_options/src/plugins/js_loader/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use rspack_collections::{Identifiable, Identifier};
use rspack_core::{
BoxLoader, Context, Loader, ModuleRuleUseLoader, NormalModuleFactoryResolveLoader, ResolveResult,
Resolver, RunnerContext, BUILTIN_LOADER_PREFIX,
Resolver, Resource, RunnerContext, BUILTIN_LOADER_PREFIX,
};
use rspack_error::{error, Result};
use rspack_hook::plugin_hook;
Expand Down Expand Up @@ -104,19 +104,30 @@ pub(crate) async fn resolve_loader(

match resolve_result {
ResolveResult::Resource(resource) => {
let path = resource.path.as_str();
let Resource {
path,
query,
description_data,
..
} = resource;

let r#type = if path.ends_with(".mjs") {
Some("module")
} else if path.ends_with(".cjs") {
Some("commonjs")
} else {
resource
.description_data
description_data
.as_ref()
.and_then(|data| data.json().get("type").and_then(|t| t.as_str()))
};
// TODO: Should move this logic to `resolver`, since `resolve.alias` may contain query or fragment too.
let resource = resource.path.as_str().to_owned() + rest.unwrap_or_default();
// favor explicit loader query over aliased query, see webpack issue-3320
let resource = if let Some(rest) = rest
&& !rest.is_empty()
{
format!("{path}{rest}")
} else {
format!("{path}{query}")
};
let ident = if let Some(ty) = r#type {
format!("{ty}|{resource}")
} else {
Expand Down
2 changes: 2 additions & 0 deletions crates/rspack_core/src/options/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,10 @@ pub struct FuncUseCtx {
#[derive(Debug, Clone)]
pub struct ModuleRuleUseLoader {
/// Loader identifier with query and fragments
/// Loader ident or query will be appended if it exists.
pub loader: String,
/// Loader options
/// This only exists if the loader is a built-in loader.
pub options: Option<String>,
}

Expand Down
9 changes: 4 additions & 5 deletions tests/webpack-test/configCases/loaders/issue-3320/index.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
// Webpack supports this, but Rspack does not support `resolve.alias` with a custom query and fragment
// it.skip("should resolve aliased loader module with query", function() {
// var foo = require('./a');
it("should resolve aliased loader module with query", function() {
var foo = require('./a');

// expect(foo).toBe("someMessage");
// });
expect(foo).toBe("someMessage");
});

it("should favor explicit loader query over aliased query (options in rule)", function() {
var foo = require('./b');
Expand Down

2 comments on commit 076cf9d

@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 ❌ failure
_selftest ✅ success
nx ❌ failure
rspress ❌ failure
rslib ✅ success
rsbuild ❌ failure
examples ✅ success

@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-08-23 2d18cc2) Current Change
10000_development-mode + exec 2.37 s ± 25 ms 2.34 s ± 26 ms -1.17 %
10000_development-mode_hmr + exec 705 ms ± 15 ms 709 ms ± 12 ms +0.53 %
10000_production-mode + exec 3.03 s ± 26 ms 3.04 s ± 20 ms +0.57 %
arco-pro_development-mode + exec 1.9 s ± 88 ms 1.9 s ± 70 ms -0.14 %
arco-pro_development-mode_hmr + exec 437 ms ± 2.6 ms 436 ms ± 2.1 ms -0.12 %
arco-pro_production-mode + exec 3.47 s ± 54 ms 3.48 s ± 75 ms +0.27 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.53 s ± 69 ms 3.53 s ± 115 ms +0.11 %
threejs_development-mode_10x + exec 1.69 s ± 15 ms 1.78 s ± 19 ms +5.12 %
threejs_development-mode_10x_hmr + exec 800 ms ± 10 ms 821 ms ± 6.6 ms +2.59 %
threejs_production-mode_10x + exec 5.49 s ± 29 ms 5.61 s ± 19 ms +2.22 %

Threshold exceeded: ["threejs_development-mode_10x + exec"]

Please sign in to comment.