Skip to content

Commit

Permalink
fix: should generate correct context request with query (#6018)
Browse files Browse the repository at this point in the history
  • Loading branch information
LingyuCoder authored Mar 25, 2024
1 parent 5dbba23 commit 70b2bd2
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 16 deletions.
6 changes: 5 additions & 1 deletion crates/rspack_core/src/context_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ pub struct ContextOptions {
pub exclude: Option<String>,
pub category: DependencyCategory,
pub request: String,
pub context: String,
pub namespace_object: ContextNameSpaceObject,
pub chunk_name: Option<String>,
pub start: u32,
Expand All @@ -141,7 +142,7 @@ impl Display for ContextOptions {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"ContextOptions|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}",
"ContextOptions|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}|{:?}",
self.mode,
self.recursive,
self.reg_exp,
Expand All @@ -150,6 +151,7 @@ impl Display for ContextOptions {
self.exclude,
self.category,
self.request,
self.context,
self.namespace_object,
self.chunk_name
)
Expand All @@ -165,6 +167,7 @@ impl PartialEq for ContextOptions {
&& self.exclude == other.exclude
&& self.category == other.category
&& self.request == other.request
&& self.context == other.context
&& self.namespace_object == other.namespace_object
}
}
Expand All @@ -180,6 +183,7 @@ impl Hash for ContextOptions {
self.exclude.hash(state);
self.category.hash(state);
self.request.hash(state);
self.context.hash(state);
self.namespace_object.hash(state);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,17 @@ impl DependencyTemplate for ImportContextDependency {

source.replace(self.callee_start, self.callee_end, &expr, None);

let context = normalize_context(&self.options.request);
let context = normalize_context(&self.options.context);
let query = parse_resource(&self.options.request).and_then(|data| data.query);
if !context.is_empty() {
if !context.is_empty() || query.is_some() {
source.insert(self.callee_end, "(", None);
source.insert(
self.args_end,
format!(".replace('{context}', './')").as_str(),
None,
);
if !context.is_empty() {
source.insert(
self.args_end,
format!(".replace('{context}', './')").as_str(),
None,
);
}
if let Some(query) = query {
source.insert(
self.args_end,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ fn create_commonjs_require_context_dependency(
exclude: None,
category: DependencyCategory::CommonJS,
request: format!("{}{}{}", result.context, result.query, result.fragment),
context: result.context,
namespace_object: ContextNameSpaceObject::Unset,
start: callee_start,
end: callee_end,
Expand Down Expand Up @@ -232,6 +233,7 @@ impl CommonJsImportsParserPlugin {
exclude: None,
category: DependencyCategory::Unknown,
request: ".".to_string(),
context: ".".to_string(),
namespace_object: ContextNameSpaceObject::Unset,
start: ident.span().real_lo(),
end: ident.span().real_hi(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ fn create_import_meta_context_dependency(node: &CallExpr) -> Option<ImportMetaCo
exclude: None,
recursive,
category: DependencyCategory::Esm,
request: context,
request: context.clone(),
context,
namespace_object: ContextNameSpaceObject::Unset,
mode,
start: node.span().real_lo(),
Expand All @@ -79,7 +80,8 @@ fn create_import_meta_context_dependency(node: &CallExpr) -> Option<ImportMetaCo
reg_exp: context_reg_exp(reg, ""),
reg_str,
category: DependencyCategory::Esm,
request: context,
request: context.clone(),
context,
namespace_object: ContextNameSpaceObject::Unset,
start: node.span().real_lo(),
end: node.span().real_hi(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ impl JavascriptParserPlugin for ImportParserPlugin {
include: None,
exclude: None,
category: DependencyCategory::Esm,
request: format!("{}{}{}", context, query, fragment),
request: format!("{}{}{}", context.clone(), query, fragment),
context,
namespace_object: if parser.build_meta.strict_harmony_module {
ContextNameSpaceObject::Strict
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl JavascriptParserPlugin for RequireContextDependencyParserPlugin {
exclude: None,
category: DependencyCategory::CommonJS,
request: request_expr.string().to_string(),
context: request_expr.string().to_string(),
namespace_object: rspack_core::ContextNameSpaceObject::Unset,
chunk_name: None,
start: expr.span().real_lo(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ exports[`StatsTestCases should print correct stats for ignore-plugin 1`] = `
"runtime modules 1 module
./index.js
./locals/en.js
Xdir/ignore-plugin/locals|Some(\\"\\")|Some(\\"\\")|ContextOptions|Sync|true|Some(RspackRegex(\\"^\\\\\\\\.\\\\\\\\/.*$\\"))|\\"^\\\\\\\\.\\\\\\\\/.*$\\"|None|None|CommonJS|\\"./locals\\"|Unset|None"
Xdir/ignore-plugin/locals|Some(\\"\\")|Some(\\"\\")|ContextOptions|Sync|true|Some(RspackRegex(\\"^\\\\\\\\.\\\\\\\\/.*$\\"))|\\"^\\\\\\\\.\\\\\\\\/.*$\\"|None|None|CommonJS|\\"./locals\\"|\\"./locals\\"|Unset|None"
`;
exports[`StatsTestCases should print correct stats for ignore-warning 1`] = `"Rspack compiled successfully"`;
Expand Down
57 changes: 57 additions & 0 deletions packages/rspack/tests/cases/context/resource-query-prefix/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
it("should detect query strings in dynamic import in sub directories by concat as a static value", function () {
var testFileName = "a";

return Promise.all([
import(`./sub/${testFileName}`).then(({ default: a }) => {
expect(a()).toBe("a");
}),
import(`./sub/${testFileName}`).then(({ default: a }) => {
expect(a()).toBe("a");
}),
import(`./sub/${testFileName}bc`).then(({ default: a }) => {
expect(a()).toBe("abc");
}),
import(`./sub/${testFileName}?queryString`).then(({ default: a }) => {
expect(a()).toBe("a?queryString");
})
]);
});

it("should detect query strings in dynamic import in sub directories by template string as a static value", function () {
var testFileName = "a";

return Promise.all([
import("./sub/".concat(testFileName)).then(({ default: a }) => {
expect(a()).toBe("a");
}),
import("./sub/".concat(testFileName).concat("")).then(({ default: a }) => {
expect(a()).toBe("a");
}),
import("./sub/".concat(testFileName).concat("bc")).then(({ default: a }) => {
expect(a()).toBe("abc");
}),
import("./sub/".concat(testFileName).concat("?queryString")).then(({ default: a }) => {
expect(a()).toBe("a?queryString");
})
]);
});

it("should detect query strings in dynamic import in sub directories by add as a static value", function () {
var testFileName = "a";

return Promise.all([
import("./sub/" + testFileName).then(({ default: a }) => {
expect(a()).toBe("a");
}),
import("./sub/" + testFileName + "").then(({ default: a }) => {
expect(a()).toBe("a");
}),
import("./sub/" + testFileName + "bc").then(({ default: a }) => {
expect(a()).toBe("abc");
}),
import("./sub/" + testFileName + "?queryString").then(({ default: a }) => {
expect(a()).toBe("a?queryString");
})
]);
});

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function a() {
return "a" + __resourceQuery;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function abc() {
return "abc" + __resourceQuery;
}
4 changes: 0 additions & 4 deletions webpack-test/cases/parsing/issue-7778/test.filter.js

This file was deleted.

2 comments on commit 70b2bd2

@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, self-hosted, Linux, ci, ec2-linux ✅ success
_selftest, ubuntu-latest ✅ success
nx, ubuntu-latest ✅ success
rspress, ubuntu-latest ✅ success
rsbuild, ubuntu-latest ✅ success
compat, ubuntu-latest ✅ success
examples, ubuntu-latest ✅ 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-03-25 ca8afe6) Current Change
10000_development-mode + exec 1.92 s ± 15 ms 1.96 s ± 41 ms +2.03 %
10000_development-mode_hmr + exec 738 ms ± 10 ms 740 ms ± 7.3 ms +0.37 %
10000_production-mode + exec 2.96 s ± 41 ms 2.86 s ± 74 ms -3.35 %
arco-pro_development-mode + exec 2.52 s ± 17 ms 2.47 s ± 21 ms -1.90 %
arco-pro_development-mode_hmr + exec 513 ms ± 4.1 ms 514 ms ± 4.6 ms +0.18 %
arco-pro_development-mode_hmr_intercept-plugin + exec 531 ms ± 2 ms 525 ms ± 30 ms -1.08 %
arco-pro_development-mode_intercept-plugin + exec 3.39 s ± 33 ms 3.35 s ± 52 ms -1.20 %
arco-pro_production-mode + exec 4.19 s ± 61 ms 4.09 s ± 41 ms -2.33 %
arco-pro_production-mode_intercept-plugin + exec 5.07 s ± 97 ms 4.93 s ± 53 ms -2.77 %
threejs_development-mode_10x + exec 1.88 s ± 21 ms 1.87 s ± 25 ms -0.17 %
threejs_development-mode_10x_hmr + exec 738 ms ± 3.5 ms 739 ms ± 9.5 ms +0.14 %
threejs_production-mode_10x + exec 5.65 s ± 40 ms 5.65 s ± 41 ms -0.01 %

Please sign in to comment.