From ba8c5988ea119c032c983f3a36f2c2d0fd4e6e45 Mon Sep 17 00:00:00 2001
From: Hana <andywangsy@gmail.com>
Date: Thu, 22 Aug 2024 19:00:59 +0800
Subject: [PATCH] fix: should resolve aliased loader module with query

---
 .../src/plugins/js_loader/resolver.rs         | 23 ++++++++++++++-----
 crates/rspack_core/src/options/module.rs      |  2 ++
 .../configCases/loaders/issue-3320/index.js   |  9 ++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

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 b16985321c6..fbf3ef6890e 100644
--- a/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs
+++ b/crates/rspack_binding_options/src/plugins/js_loader/resolver.rs
@@ -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;
@@ -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 {
diff --git a/crates/rspack_core/src/options/module.rs b/crates/rspack_core/src/options/module.rs
index d10cef2023a..ff54c5e0d4d 100644
--- a/crates/rspack_core/src/options/module.rs
+++ b/crates/rspack_core/src/options/module.rs
@@ -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>,
 }
 
diff --git a/tests/webpack-test/configCases/loaders/issue-3320/index.js b/tests/webpack-test/configCases/loaders/issue-3320/index.js
index 6fd28de49d1..7d496b8ebef 100644
--- a/tests/webpack-test/configCases/loaders/issue-3320/index.js
+++ b/tests/webpack-test/configCases/loaders/issue-3320/index.js
@@ -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');