Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: should resolve aliased loader module with query #7660

Merged
merged 1 commit into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading