fix: amd define in function params (#9088)

crates/rspack_plugin_javascript/src/parser_plugin/amd_define_dependency_parser_plugin.rs → crates/rspack_plugin_javascript/src/parser_plugin/amd/amd_define_dependency_parser_plugin.rs

@@ -13,7 +13,6 @@ use swc_core::{
   },
 };
 
-use super::JavascriptParserPlugin;
 use crate::{
   dependency::{
     amd_define_dependency::AmdDefineDependency,
@@ -22,6 +21,7 @@ use crate::{
   },
   utils::eval::BasicEvaluatedExpression,
   visitors::{scope_info::FreeName, JavascriptParser, Statement},
+  JavascriptParserPlugin,
 };
 
 pub struct AMDDefineDependencyParserPlugin;

crates/rspack_plugin_javascript/src/parser_plugin/require_js_stuff_plugin.rs → crates/rspack_plugin_javascript/src/parser_plugin/amd/amd_plugin.rs

@@ -2,18 +2,18 @@ use rspack_core::{ConstDependency, RuntimeGlobals, SpanExt};
 use swc_core::ecma::ast::{CallExpr, Expr, MemberExpr};
 use swc_core::{common::Spanned, ecma::ast::UnaryExpr};
 
-use super::JavascriptParserPlugin;
 use crate::utils::eval::{evaluate_to_identifier, evaluate_to_string, BasicEvaluatedExpression};
-use crate::visitors::{expr_matcher, JavascriptParser};
+use crate::visitors::JavascriptParser;
+use crate::JavascriptParserPlugin;
 
-pub struct RequireJsStuffPlugin;
+pub struct AMDParserPlugin;
 
 const DEFINE: &str = "define";
 const REQUIRE: &str = "require";
 const DEFINE_AMD: &str = "define.amd";
 const REQUIRE_AMD: &str = "require.amd";
 
-impl JavascriptParserPlugin for RequireJsStuffPlugin {
+impl JavascriptParserPlugin for AMDParserPlugin {
   fn call(
     &self,
     parser: &mut JavascriptParser,
@@ -29,19 +29,18 @@ impl JavascriptParserPlugin for RequireJsStuffPlugin {
           "undefined".into(),
           None,
         )));
-      Some(true)
-    } else {
-      None
+      return Some(true);
     }
+    None
   }
 
   fn member(
     &self,
     parser: &mut JavascriptParser,
     expr: &MemberExpr,
-    _for_name: &str,
+    for_name: &str,
   ) -> Option<bool> {
-    if expr_matcher::is_require_version(expr) {
+    if for_name == "require.version" {
       parser
         .presentational_dependencies
         .push(Box::new(ConstDependency::new(
@@ -52,8 +51,7 @@ impl JavascriptParserPlugin for RequireJsStuffPlugin {
         )));
       return Some(true);
     }
-
-    if expr_matcher::is_require_onerror(expr) || expr_matcher::is_requirejs_onerror(expr) {
+    if for_name == "requirejs.onError" {
       parser
         .presentational_dependencies
         .push(Box::new(ConstDependency::new(
@@ -65,8 +63,8 @@ impl JavascriptParserPlugin for RequireJsStuffPlugin {
       return Some(true);
     }
 
-    // AMDPlugin
-    if expr_matcher::is_define_amd(expr) || expr_matcher::is_require_amd(expr) {
+    // AMD
+    if for_name == "define.amd" || for_name == "require.amd" {
       parser
         .presentational_dependencies
         .push(Box::new(ConstDependency::new(
@@ -77,7 +75,6 @@ impl JavascriptParserPlugin for RequireJsStuffPlugin {
         )));
       return Some(true);
     }
-
     None
   }
 
@@ -191,7 +188,7 @@ impl JavascriptParserPlugin for RequireJsStuffPlugin {
           RuntimeGlobals::AMD_DEFINE.name().into(),
           Some(RuntimeGlobals::AMD_DEFINE),
         )));
-      return Some(true);
+      return Some(false);
     }
     None
   }

crates/rspack_plugin_javascript/src/parser_plugin/amd_require_dependencies_block_parser_plugin.rs → crates/rspack_plugin_javascript/src/parser_plugin/amd/amd_require_dependencies_block_parser_plugin.rs

@@ -13,17 +13,16 @@ use swc_core::{
   ecma::ast::{BlockStmtOrExpr, CallExpr, ExprOrSpread, Pat},
 };
 
-use super::{
-  require_ensure_dependencies_block_parse_plugin::GetFunctionExpression, JavascriptParserPlugin,
-};
 use crate::{
   dependency::{
     amd_require_dependency::AMDRequireDependency,
     amd_require_item_dependency::AMDRequireItemDependency,
     local_module_dependency::LocalModuleDependency, unsupported_dependency::UnsupportedDependency,
   },
+  parser_plugin::require_ensure_dependencies_block_parse_plugin::GetFunctionExpression,
   utils::eval::BasicEvaluatedExpression,
   visitors::{create_traceable_error, JavascriptParser, Statement},
+  JavascriptParserPlugin,
 };
 
 fn is_reserved_param(pat: &Pat) -> bool {

crates/rspack_plugin_javascript/src/parser_plugin/amd/mod.rs

@@ -0,0 +1,7 @@
+mod amd_define_dependency_parser_plugin;
+mod amd_plugin;
+mod amd_require_dependencies_block_parser_plugin;
+
+pub use amd_define_dependency_parser_plugin::AMDDefineDependencyParserPlugin;
+pub use amd_plugin::AMDParserPlugin;
+pub use amd_require_dependencies_block_parser_plugin::AMDRequireDependenciesBlockParserPlugin;

crates/rspack_plugin_javascript/src/parser_plugin/mod.rs

@@ -1,3 +1,4 @@
+mod amd;
 mod api_plugin;
 mod check_var_decl;
 mod common_js_exports_parse_plugin;
@@ -27,15 +28,13 @@ mod use_strict_plugin;
 mod webpack_included_plugin;
 mod worker_plugin;
 
-pub mod amd_define_dependency_parser_plugin;
-pub mod amd_require_dependencies_block_parser_plugin;
 pub mod define_plugin;
 pub mod hot_module_replacement_plugin;
 pub mod provide_plugin;
-pub mod require_js_stuff_plugin;
 
-pub(crate) use self::amd_define_dependency_parser_plugin::AMDDefineDependencyParserPlugin;
-pub(crate) use self::amd_require_dependencies_block_parser_plugin::AMDRequireDependenciesBlockParserPlugin;
+pub(crate) use self::amd::AMDDefineDependencyParserPlugin;
+pub(crate) use self::amd::AMDParserPlugin;
+pub(crate) use self::amd::AMDRequireDependenciesBlockParserPlugin;
 pub(crate) use self::api_plugin::APIPlugin;
 pub(crate) use self::check_var_decl::CheckVarDeclaratorIdent;
 pub(crate) use self::common_js_exports_parse_plugin::CommonJsExportsParserPlugin;
@@ -60,7 +59,6 @@ pub(crate) use self::r#const::{is_logic_op, ConstPlugin};
 pub use self::r#trait::{BoxJavascriptParserPlugin, JavascriptParserPlugin};
 pub(crate) use self::require_context_dependency_parser_plugin::RequireContextDependencyParserPlugin;
 pub(crate) use self::require_ensure_dependencies_block_parse_plugin::RequireEnsureDependenciesBlockParserPlugin;
-pub(crate) use self::require_js_stuff_plugin::RequireJsStuffPlugin;
 pub(crate) use self::url_plugin::URLPlugin;
 pub(crate) use self::use_strict_plugin::UseStrictPlugin;
 pub(crate) use self::webpack_included_plugin::WebpackIsIncludedPlugin;

crates/rspack_plugin_javascript/src/visitors/dependency/parser/mod.rs

@@ -315,7 +315,7 @@ impl<'parser> JavascriptParser<'parser> {
         parser_plugin::AMDRequireDependenciesBlockParserPlugin,
       ));
       plugins.push(Box::new(parser_plugin::AMDDefineDependencyParserPlugin));
-      plugins.push(Box::new(parser_plugin::RequireJsStuffPlugin));
+      plugins.push(Box::new(parser_plugin::AMDParserPlugin));
     }
 
     if module_type.is_js_auto() || module_type.is_js_dynamic() {

crates/rspack_plugin_javascript/src/visitors/dependency/parser/walk.rs

@@ -24,7 +24,7 @@ use super::{
   TopLevelScope,
 };
 use crate::parser_plugin::{is_logic_op, JavascriptParserPlugin};
-use crate::visitors::scope_info::{FreeName, VariableInfo};
+use crate::visitors::scope_info::FreeName;
 
 fn warp_ident_to_pat(ident: Ident) -> Pat {
   Pat::Ident(ident.into())
@@ -791,31 +791,70 @@ impl JavascriptParser<'_> {
   fn _walk_iife<'a>(
     &mut self,
     expr: &'a Expr,
-    params: impl Iterator<Item = &'a Expr>,
+    args: impl Iterator<Item = &'a Expr>,
     current_this: Option<&'a Expr>,
   ) {
-    let mut fn_params = vec![];
+    fn get_var_name(parser: &mut JavascriptParser, expr: &Expr) -> Option<String> {
+      if let Some(rename_identifier) = parser.get_rename_identifier(expr)
+        && let drive = parser.plugin_drive.clone()
+        && rename_identifier
+          .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
+          .unwrap_or_default()
+        && !rename_identifier
+          .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
+          .unwrap_or_default()
+      {
+        let variable = parser
+          .get_variable_info(&rename_identifier)
+          .map(|info| info.free_name.as_ref())
+          .and_then(|free_name| free_name)
+          .and_then(|free_name| match free_name {
+            FreeName::String(s) => Some(s.to_string()),
+            FreeName::True => None,
+          })
+          .unwrap_or(rename_identifier);
+        return Some(variable);
+      }
+      parser.walk_expression(expr);
+      None
+    }
+
+    let rename_this = current_this.and_then(|this| get_var_name(self, this));
+    let variable_info_for_args = args
+      .map(|param| get_var_name(self, param))
+      .collect::<Vec<_>>();
+
+    let mut params = vec![];
     let mut scope_params = vec![];
-    if let Some(expr) = expr.as_fn_expr() {
-      for param in &expr.function.params {
-        let ident = param.pat.as_ident().expect("should be a `BindingIdent`");
-        fn_params.push(ident);
-        if get_variable_info(self, &Expr::Ident(ident.id.clone())).is_none() {
-          scope_params.push(Cow::Borrowed(&param.pat));
+    if let Some(fn_expr) = expr.as_fn_expr() {
+      for (i, pat) in fn_expr.function.params.iter().map(|p| &p.pat).enumerate() {
+        // SAFETY: is_simple_function will ensure pat is always a BindingIdent.
+        let ident = pat.as_ident().expect("should be a `BindingIdent`");
+        params.push(ident);
+        if variable_info_for_args
+          .get(i)
+          .and_then(|i| i.as_deref())
+          .is_none()
+        {
+          scope_params.push(Cow::Borrowed(pat));
         }
       }
-    } else if let Some(expr) = expr.as_arrow() {
-      for param in &expr.params {
-        let ident = param.as_ident().expect("should be a `BindingIdent`");
-        fn_params.push(ident);
-        if get_variable_info(self, &Expr::Ident(ident.id.clone())).is_none() {
-          scope_params.push(Cow::Borrowed(param));
+    } else if let Some(arrow_expr) = expr.as_arrow() {
+      for (i, pat) in arrow_expr.params.iter().enumerate() {
+        // SAFETY: is_simple_function will ensure pat is always a BindingIdent.
+        let ident = pat.as_ident().expect("should be a `BindingIdent`");
+        params.push(ident);
+        if variable_info_for_args
+          .get(i)
+          .and_then(|i| i.as_deref())
+          .is_none()
+        {
+          scope_params.push(Cow::Borrowed(pat));
         }
       }
-    };
-    let variable_info_for_args = params
-      .map(|param| get_variable_name(self, param))
-      .collect::<Vec<_>>();
+    }
+
+    // Add function name in scope for recursive calls
     if let Some(expr) = expr.as_fn_expr() {
       if let Some(ident) = &expr.ident {
         scope_params.push(Cow::Owned(Pat::Ident(ident.clone().into())));
@@ -824,24 +863,24 @@ impl JavascriptParser<'_> {
 
     let was_top_level_scope = self.top_level_scope;
     self.top_level_scope =
-      if !matches!(was_top_level_scope, TopLevelScope::False) && expr.as_arrow().is_some() {
+      if !matches!(was_top_level_scope, TopLevelScope::False) && expr.is_arrow() {
         TopLevelScope::ArrowFunction
       } else {
         TopLevelScope::False
       };
 
-    let rename_this = current_this.and_then(|this| get_variable_name(self, this));
     self.in_function_scope(true, scope_params.into_iter(), |parser| {
       if let Some(this) = rename_this
-        && matches!(expr, Expr::Fn(_))
+        && !expr.is_arrow()
       {
         parser.set_variable("this".to_string(), this)
       }
-      for (variable_info, param) in variable_info_for_args.into_iter().zip(fn_params) {
-        let Some(variable_info) = variable_info else {
-          continue;
-        };
-        parser.set_variable(param.sym.to_string(), variable_info);
+      for (i, var_info) in variable_info_for_args.into_iter().enumerate() {
+        if let Some(var_info) = var_info
+          && let Some(param) = params.get(i)
+        {
+          parser.set_variable(param.sym.to_string(), var_info);
+        }
       }
 
       if let Some(expr) = expr.as_fn_expr() {
@@ -989,6 +1028,7 @@ impl JavascriptParser<'_> {
           } else {
             self.walk_expression(callee);
           }
+          self.walk_expr_or_spread(&expr.args);
         }
       }
       Callee::Import(_) => {
@@ -1002,11 +1042,15 @@ impl JavascriptParser<'_> {
           self.enter_call -= 1;
           return;
         }
+
+        self.walk_expr_or_spread(&expr.args);
+      }
+      Callee::Super(_) => {
+        // Do nothing about super, same as webpack
+        self.walk_expr_or_spread(&expr.args);
       }
-      Callee::Super(_) => {} // Do nothing about super, same as webpack
     }
 
-    self.walk_expr_or_spread(&expr.args);
     self.enter_call -= 1;
   }
 
@@ -1481,45 +1525,3 @@ fn member_prop_len(member_prop: &MemberProp) -> Option<usize> {
     MemberProp::Computed(_) => None,
   }
 }
-
-fn get_variable_info<'p>(
-  parser: &'p mut JavascriptParser,
-  expr: &Expr,
-) -> Option<&'p VariableInfo> {
-  if let Some(rename_identifier) = parser.get_rename_identifier(expr)
-    && let drive = parser.plugin_drive.clone()
-    && rename_identifier
-      .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
-      .unwrap_or_default()
-    && !rename_identifier
-      .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
-      .unwrap_or_default()
-  {
-    return parser.get_variable_info(&rename_identifier);
-  }
-  None
-}
-
-fn get_variable_name(parser: &mut JavascriptParser, expr: &Expr) -> Option<String> {
-  if let Some(rename_identifier) = parser.get_rename_identifier(expr)
-    && let drive = parser.plugin_drive.clone()
-    && rename_identifier
-      .call_hooks_name(parser, |this, for_name| drive.can_rename(this, for_name))
-      .unwrap_or_default()
-    && !rename_identifier
-      .call_hooks_name(parser, |this, for_name| drive.rename(this, expr, for_name))
-      .unwrap_or_default()
-  {
-    let variable = parser
-      .get_variable_info(&rename_identifier)
-      .map(|info| info.free_name.as_ref())
-      .and_then(|free_name| free_name)
-      .and_then(|free_name| match free_name {
-        FreeName::String(s) => Some(s.to_string()),
-        FreeName::True => None,
-      })
-      .unwrap_or(rename_identifier);
-    return Some(variable);
-  }
-  None
-}

crates/rspack_plugin_javascript/src/visitors/dependency/util.rs

@@ -198,10 +198,7 @@ pub(crate) mod expr_matcher {
     is_object_define_property: "Object.defineProperty",
     is_require_ensure: "require.ensure",
     is_require_version: "require.version",
-    is_require_amd: "require.amd",
     is_require_onerror: "require.onError",
-    is_requirejs_onerror: "requirejs.onError",
-    is_define_amd: "define.amd",
     // unsupported
     is_require_extensions: "require.extensions",
     is_require_config: "require.config",