refactor: move context dependency walk_expression into `create_cont…

…ext_dependency` (#6963) refactor: move context dependency `walk_expression` into `create_context_dependency` (#6963) --------- Co-authored-by: ahabhgk <[email protected]>
web-infra-dev · Jul 16, 2024 · 7a66fab · 7a66fab · rspack-bot · Jul 16, 2024
1 parent 7a6d2a5
commit 7a66fab
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 9 deletions.
diff --git a/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs b/crates/rspack_plugin_javascript/src/parser_plugin/common_js_imports_parse_plugin.rs
@@ -19,12 19,13 @@ use crate::visitors::{extract_require_call_info, is_require_call_start};
 fn create_commonjs_require_context_dependency(
  parser: &mut JavascriptParser,
  param: &BasicEvaluatedExpression,
  expr: &Expr,
  callee_start: u32,
  callee_end: u32,
  args_end: u32,
  span: Option<ErrorSpan>,
 ) -> CommonJsRequireContextDependency {
- let result = create_context_dependency(param, parser);
  let result = create_context_dependency(param, expr, parser);
  let options = ContextOptions {
  mode: ContextMode::Sync,
  recursive: true,
@@ -136,14 137,13 @@ impl CommonJsImportsParserPlugin {
  let dep = create_commonjs_require_context_dependency(
  parser,
  param,
  argument_expr,
  call_expr.callee.span().real_lo(),
  call_expr.callee.span().real_hi(),
  call_expr.span.real_hi(),
  Some(call_expr.span.into()),
  );
  parser.dependencies.push(Box::new(dep));
- // FIXME: align `parser.walk_expression` to webpack, which put into `context_dependency_helper`
- parser.walk_expression(argument_expr);
  Some(true)
  }
 

diff --git a/crates/rspack_plugin_javascript/src/parser_plugin/import_parser_plugin.rs b/crates/rspack_plugin_javascript/src/parser_plugin/import_parser_plugin.rs
@@ -121,7 121,7 @@ impl JavascriptParserPlugin for ImportParserPlugin {
  query,
  fragment,
  replaces,
- } = create_context_dependency(&param, parser);
  } = create_context_dependency(&param, &dyn_imported.expr, parser);
  let reg_exp = context_reg_exp(&reg, "", Some(dyn_imported.span().into()), parser);
  parser
  .dependencies
@@ -156,8 156,6 @@ impl JavascriptParserPlugin for ImportParserPlugin {
  Some(node.span.into()),
  parser.in_try,
  )));
- // FIXME: align `parser.walk_expression` to webpack, which put into `context_dependency_helper`
- parser.walk_expression(&dyn_imported.expr);
  Some(true)
  }
  }

diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/context_dependency_helper.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/context_dependency_helper.rs
@@ -6,6 6,7 @@ use regex::Regex;
 use rspack_core::parse_resource;
 use rspack_error::Severity;
 use rspack_util::json_stringify;
 use swc_core::ecma::ast::Expr;
 
 use super::create_traceable_error;
 use crate::utils::eval::{BasicEvaluatedExpression, TemplateStringKind};
@@ -15,6 16,7 @@ const DEFAULT_WRAPPED_CONTEXT_REGEXP: &str = ".*";
 
 pub fn create_context_dependency(
  param: &BasicEvaluatedExpression,
  expr: &Expr,
  parser: &mut crate::visitors::JavascriptParser,
 ) -> ContextModuleScanResult {
  if param.is_template_string() {
@@ -93,6 95,12 @@ pub fn create_context_dependency(
  ));
  }
 
  // Webpack will walk only the expression parts of the template string
  // but we walk the whole template string, which allows us don't need to implement
  // setExpression for BasicEvaluatedExpression (will introduce lots of lifetime)
  // This may have slight performance difference in some cases
  parser.walk_expression(expr);
 
  ContextModuleScanResult {
  context,
  reg,
@@ -161,14 169,19 @@ pub fn create_context_dependency(
  ));
  }
 
  // Webpack will walk only the dynamic parts of evaluated expression
  // but we walk the whole expression, which allows us don't need to implement
  // setExpression for BasicEvaluatedExpression (will introduce lots of lifetime)
  // This may have slight performance difference in some cases
  parser.walk_expression(expr);
 
  ContextModuleScanResult {
  context,
  reg,
  query,
  fragment,
  replaces,
  }
- // TODO: handle `param.wrappedInnerExpressions`
  } else {
  if parser.javascript_options.expr_context_critical {
  let range = param.range();
@@ -182,6 195,9 @@ pub fn create_context_dependency(
  .with_severity(Severity::Warn),
  ));
  }
 
  parser.walk_expression(expr);
 
  ContextModuleScanResult {
  context: String::from("."),
  reg: String::new(),

diff --git a/.../rspack-test-tools/tests/builtinCases/rspack/dynamic-import/__snapshots__/output.snap.txt b/.../rspack-test-tools/tests/builtinCases/rspack/dynamic-import/__snapshots__/output.snap.txt
@@ -71,14 71,16 @@ __webpack_require__.e(/* import() */ "child_b_js").then(__webpack_require__.bind
 __webpack_require__("./child lazy recursive ^\\.\\/.*\\.js$")(`./${request}.js`).then(({ a }) =>
  console.log("context_module_tpl", a)
 );
 __webpack_require__.e(/* import() */ "child_a_js").then(__webpack_require__.bind(__webpack_require__, "./child/a.js")).then(({ a }) =>
  console.log("context_module_tpl_with_cond", a)
 );
 __webpack_require__("./child lazy recursive ^\\.\\/.*\\.js$")("./"   request   ".js").then(({ a }) =>
  console.log("context_module_bin", a)
 );
 __webpack_require__("./child lazy recursive ^\\.\\/.*\\.js$")("./".concat(request, ".js")).then(({ a }) =>
  console.log("context_module_concat", a)
 );
 
-
 }),
 
 },function(__webpack_require__) {

diff --git a/packages/rspack-test-tools/tests/builtinCases/rspack/dynamic-import/index.js b/packages/rspack-test-tools/tests/builtinCases/rspack/dynamic-import/index.js
@@ -4,9 4,12 @@ import(`./child/b.js`).then(({ b }) => console.log("Template Literal", b));
 import(`./child/${request}.js`).then(({ a }) =>
  console.log("context_module_tpl", a)
 );
 import(`./child/${true ? "a" : "b"}.js`).then(({ a }) =>
  console.log("context_module_tpl_with_cond", a)
 );
 import("./child/"   request   ".js").then(({ a }) =>
  console.log("context_module_bin", a)
 );
 import("./child/".concat(request, ".js")).then(({ a }) =>
  console.log("context_module_concat", a)
-);
 );
suite	result
modernjs	❌ failure
_selftest	✅ success
nx	❌ failure
rspress	✅ success
rsbuild	✅ success
examples	✅ success
Name	Base (2024-07-16 8645c8d)	Current	Change
10000_development-mode exec	2.25 s ± 17 ms	2.23 s ± 28 ms	-0.62 %
10000_development-mode_hmr exec	698 ms ± 4.7 ms	701 ms ± 8.9 ms	0.46 %
10000_production-mode exec	2.8 s ± 34 ms	2.79 s ± 26 ms	-0.34 %
arco-pro_development-mode exec	1.88 s ± 72 ms	1.91 s ± 53 ms	1.67 %
arco-pro_development-mode_hmr exec	434 ms ± 3.2 ms	433 ms ± 1.3 ms	-0.13 %
arco-pro_production-mode exec	3.44 s ± 90 ms	3.43 s ± 74 ms	-0.29 %
threejs_development-mode_10x exec	1.75 s ± 14 ms	1.75 s ± 24 ms	0.09 %
threejs_development-mode_10x_hmr exec	860 ms ± 12 ms	864 ms ± 5.8 ms	0.43 %
threejs_production-mode_10x exec	5.72 s ± 28 ms	5.74 s ± 24 ms	0.22 %