Skip to content

Commit 4b96924

Browse files
h-a-n-aclaude
andauthored
fix(loader): fix JS loader type detection for issue #11129 (#11361)
* fix(loader): fix JS loader type detection for issue #11129 - Refactor JsLoader to separate module type from identifier - Add exports-loader test case with inline and object configurations - Include exports-loader dependency for testing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> * fix(loader): improve JS loader type detection for issue #11129 Updates resolver and loader runner to better handle JS loader type detection, addressing edge cases in the module loading process. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent 44d7e9f commit 4b96924

File tree

7 files changed

+85
-18
lines changed

7 files changed

+85
-18
lines changed

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

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
use std::sync::Arc;
1+
use std::{borrow::Cow, sync::Arc};
22

3-
use rspack_cacheable::{cacheable, cacheable_dyn};
3+
use rspack_cacheable::{
4+
cacheable, cacheable_dyn,
5+
with::{AsOption, AsRefStr},
6+
};
47
use rspack_collections::{Identifiable, Identifier};
58
use rspack_core::{
69
BoxLoader, Context, Loader, ModuleRuleUseLoader, NormalModuleFactoryResolveLoader, ResolveResult,
@@ -14,10 +17,17 @@ use super::{JsLoaderRspackPlugin, JsLoaderRspackPluginInner};
1417

1518
#[cacheable]
1619
#[derive(Debug)]
17-
pub struct JsLoader(pub Identifier);
20+
pub struct JsLoader(
21+
pub Identifier,
22+
/* LoaderType */ #[cacheable(with=AsOption<AsRefStr>)] pub Option<Cow<'static, str>>,
23+
);
1824

1925
#[cacheable_dyn]
20-
impl Loader<RunnerContext> for JsLoader {}
26+
impl Loader<RunnerContext> for JsLoader {
27+
fn r#type(&self) -> Option<&str> {
28+
self.1.as_deref()
29+
}
30+
}
2131

2232
impl Identifiable for JsLoader {
2333
fn identifier(&self) -> Identifier {
@@ -88,13 +98,16 @@ pub(crate) async fn resolve_loader(
8898
let path = path.as_str();
8999

90100
let r#type = if path.ends_with(".mjs") {
91-
Some("module")
101+
Some(Cow::Borrowed("module"))
92102
} else if path.ends_with(".cjs") {
93-
Some("commonjs")
103+
Some(Cow::Borrowed("commonjs"))
94104
} else {
95-
description_data
96-
.as_ref()
97-
.and_then(|data| data.json().get("type").and_then(|t| t.as_str()))
105+
description_data.as_ref().and_then(|data| {
106+
data
107+
.json()
108+
.get("type")
109+
.and_then(|t| t.as_str().map(|t| Cow::Owned(t.to_owned())))
110+
})
98111
};
99112
// favor explicit loader query over aliased query, see webpack issue-3320
100113
let resource = if let Some(rest) = rest
@@ -104,12 +117,7 @@ pub(crate) async fn resolve_loader(
104117
} else {
105118
format!("{path}{query}")
106119
};
107-
let ident = if let Some(ty) = r#type {
108-
format!("{ty}|{resource}")
109-
} else {
110-
resource
111-
};
112-
Ok(Some(Arc::new(JsLoader(ident.into()))))
120+
Ok(Some(Arc::new(JsLoader(resource.into(), r#type))))
113121
}
114122
ResolveResult::Ignored => Ok(None),
115123
}

crates/rspack_loader_runner/src/loader.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,31 +193,40 @@ where
193193
loader_context.current_loader().set_finish_called();
194194
Ok(())
195195
}
196+
196197
async fn pitch(&self, _loader_context: &mut LoaderContext<Context>) -> Result<()> {
197198
// noop
198199
Ok(())
199200
}
201+
202+
/// Returns the loader type based on the module's package.json type field or file extension.
203+
/// This affects how the loader context interprets the module (e.g., "commonjs", "module").
204+
fn r#type(&self) -> Option<&str> {
205+
None
206+
}
200207
}
201208

202209
impl<C> From<Arc<dyn Loader<C>>> for LoaderItem<C>
203210
where
204211
C: Send,
205212
{
206213
fn from(loader: Arc<dyn Loader<C>>) -> Self {
207-
if let Some((r#type, ident)) = loader.identifier().split_once('|') {
214+
let ident = &**loader.identifier();
215+
if let Some(r#type) = loader.r#type() {
208216
let ResourceParsedData {
209217
path,
210218
query,
211219
fragment,
212220
} = parse_resource(ident).expect("identifier should be valid");
221+
let ty = r#type.to_string();
213222
return Self {
214223
loader,
215224
request: ident.into(),
216225
path,
217226
query,
218227
fragment,
219228
data: serde_json::Value::Null,
220-
r#type: r#type.to_string(),
229+
r#type: ty,
221230
pitch_executed: AtomicBool::new(false),
222231
normal_executed: AtomicBool::new(false),
223232
finish_called: AtomicBool::new(false),

packages/rspack-test-tools/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@
110110
"terser": "5.43.1",
111111
"typescript": "^5.9.2",
112112
"wast-loader": "^1.14.1",
113-
"worker-rspack-loader": "^3.1.2"
113+
"worker-rspack-loader": "^3.1.2",
114+
"exports-loader": "^5.0.0"
114115
},
115116
"peerDependencies": {
116117
"@rspack/core": ">=1.0.0"
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
it("should handle exports-loader with inline-style options", function () {
2+
const inlineResult = require("./lib.js?inline");
3+
const objectResult = require("./lib.js?object");
4+
5+
expect(inlineResult).toBeDefined();
6+
expect(objectResult).toBeDefined();
7+
});
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
function noop() {}
2+
3+
var lamejs = noop;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
const path = require("path");
2+
3+
/** @type {import("@rspack/core").Configuration} */
4+
module.exports = {
5+
module: {
6+
rules: [
7+
{
8+
test: path.resolve(__dirname, "lib.js"),
9+
resourceQuery: /inline/,
10+
use: "exports-loader?type=commonjs&exports=single|lamejs"
11+
},
12+
{
13+
test: path.resolve(__dirname, "lib.js"),
14+
resourceQuery: /object/,
15+
use: {
16+
loader: "exports-loader",
17+
options: {
18+
type: "commonjs",
19+
exports: "single|lamejs"
20+
}
21+
}
22+
}
23+
]
24+
}
25+
};

pnpm-lock.yaml

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)