fix: render CommonJS externals directly in modern-module output

crates/rspack_core/src/chunk_graph/chunk_graph_module.rs

@@ -357,9 +357,22 @@ impl ChunkGraph {
           side_effects_state_artifact,
           &compilation.exports_info_artifact,
         );
-        if active_state.is_false() {
+        let is_commonjs_external = mg
+          .module_by_identifier(module_identifier)
+          .and_then(|module| module.as_external_module())
+          .is_some_and(|external| {
+            crate::CommonJsExternalRequireKind::from_external_type(external.resolve_external_type())
+              .is_some()
+          });
+        if active_state.is_false() && !is_commonjs_external {
           return None;
         }
+        // Direct CommonJS external templates still read the request/type
+        // after the placement connection is cut out. Such modules may
+        // have no chunk module id, so also hash their semantic identity.
+        if is_commonjs_external {
+          module_identifier.hash(&mut hasher);
+        }
         visited_modules.insert(*module_identifier);
         for_each_runtime(
           runtime,

crates/rspack_core/src/concatenated_module.rs

@@ -3028,11 +3028,10 @@ pub fn collect_ident<'a>(
       }
     }
 
-    /// https://github.com/webpack/webpack/blob/1f99ad6367f2b8a6ef17cce0e058f7a67fb7db18/lib/optimize/ConcatenatedModule.js#L1173-L1197
+    /// Reserve every class expression's inner name, even without a superclass,
+    /// so renaming an outer binding cannot make it captured by the class scope.
     fn visit_class_expr(&mut self, node: &ClassExpr<'a>) {
-      if let Some(ident) = &node.ident
-        && node.class.super_class.is_some()
-      {
+      if let Some(ident) = &node.ident {
         self.ids.push(NewConcatenatedModuleIdent {
           id: ident.as_ref().clone_in(self.allocator),
           shorthand: false,

crates/rspack_core/src/external_module.rs

@@ -11,10 +11,10 @@ use rustc_hash::FxHashMap as HashMap;
 use serde::Serialize;
 
 use crate::{
-  BoxModule, BuildContext, BuildInfo, BuildMeta, BuildMetaExportsType, ChunkGraph,
-  ChunkInitFragments, ChunkUkey, CodeGenerationDataChunkInitFragments, CodeGenerationDataUrl,
-  CodeGenerationResultBuilder, Compilation, ConcatenationScope, Context, CssLayer,
-  CssModuleRenderCondition, DependenciesBlock, DependenciesBlockData, DependencyRef,
+  BoxChunkInitFragment, BoxModule, BuildContext, BuildInfo, BuildMeta, BuildMetaExportsType,
+  ChunkGraph, ChunkInitFragments, ChunkUkey, CodeGenerationDataChunkInitFragments,
+  CodeGenerationDataUrl, CodeGenerationResultBuilder, Compilation, ConcatenationScope, Context,
+  CssLayer, CssModuleRenderCondition, DependenciesBlock, DependenciesBlockData, DependencyRef,
   ExportProvided, ExternalType, FactoryMetaStore, FreezeLock, ImportAttributes, ImportPhase,
   InitFragmentExt, InitFragmentKey, InitFragmentStage, LibIdentOptions, Module, ModuleArgument,
   ModuleCodeGenerationContext, ModuleCodeTemplate, ModuleGraph, ModuleType,
@@ -56,6 +56,57 @@ impl ExternalRequestValue {
   }
 }
 
+/// The CommonJS require form used to render an external request.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum CommonJsExternalRequireKind {
+  CommonJs,
+  NodeCommonJs,
+}
+
+impl CommonJsExternalRequireKind {
+  pub fn from_external_type(external_type: &str) -> Option<Self> {
+    match external_type {
+      "commonjs" | "commonjs2" | "commonjs-module" | "commonjs-static" => Some(Self::CommonJs),
+      "node-commonjs" => Some(Self::NodeCommonJs),
+      _ => None,
+    }
+  }
+
+  /// Renders the complete require expression and installs any init fragment
+  /// required by its callee.
+  pub fn render_expression<S: AsRef<str>>(
+    self,
+    request: Option<&str>,
+    properties: impl IntoIterator<Item = S>,
+    compilation: &Compilation,
+    chunk_init_fragments: &mut ChunkInitFragments,
+  ) -> String {
+    let require = self.render_callee(compilation, chunk_init_fragments);
+    format!(
+      "{require}({}){}",
+      request.map_or_else(|| "undefined".to_string(), json_stringify_str),
+      property_access(properties, 0)
+    )
+  }
+
+  /// Renders only the require callee and installs its required init fragment.
+  ///
+  /// This is the split-range counterpart of [`Self::render_expression`].
+  pub fn render_callee(
+    self,
+    compilation: &Compilation,
+    chunk_init_fragments: &mut ChunkInitFragments,
+  ) -> &'static str {
+    match self {
+      Self::NodeCommonJs if compilation.options.output.module => {
+        chunk_init_fragments.push(create_node_commonjs_init_fragment(compilation));
+        "__rspack_createRequire_require"
+      }
+      Self::CommonJs | Self::NodeCommonJs => "require",
+    }
+  }
+}
+
 impl Serialize for ExternalRequestValue {
   fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
   where
@@ -128,16 +179,34 @@ fn get_request_string(request: &ExternalRequestValue) -> String {
   format!("{variable_name}{object_lookup}")
 }
 
-fn get_source_for_commonjs(module_and_specifiers: Option<&ExternalRequestValue>) -> String {
-  let (module_name, properties) = if let Some(module_and_specifiers) = module_and_specifiers {
-    (
-      module_and_specifiers.primary(),
-      property_access(module_and_specifiers.iter(), 1),
-    )
-  } else {
-    ("undefined", String::new())
-  };
-  format!("require({}){}", json_stringify_str(module_name), properties)
+fn create_node_commonjs_init_fragment(compilation: &Compilation) -> BoxChunkInitFragment {
+  let need_prefix = compilation
+    .options
+    .output
+    .environment
+    .supports_node_prefix_for_core_modules();
+
+  NormalInitFragment::new(
+    format!(
+      "import {{ createRequire as __rspack_createRequire }} from \"{}\";\n{} __rspack_createRequire_require = __rspack_createRequire({}.url);\n",
+      if need_prefix { "node:module" } else { "module" },
+      if compilation.options.output.environment.supports_const() {
+        "const"
+      } else {
+        "var"
+      },
+      compilation.options.output.import_meta_name
+    ),
+    InitFragmentStage::StageESMImports,
+    0,
+    InitFragmentKey::ModuleExternal("node-commonjs".to_string()),
+    None,
+  )
+  .with_top_level_decl_symbols(vec![
+    "__rspack_createRequire".into(),
+    "__rspack_createRequire_require".into(),
+  ])
+  .boxed()
 }
 
 fn get_source_for_import(
@@ -484,6 +553,37 @@ pub struct DependencyMeta {
 }
 
 impl ExternalModule {
+  fn create_identifier(
+    request: &ExternalRequest,
+    external_type: &str,
+    dependency_meta: &DependencyMeta,
+  ) -> Identifier {
+    let resolved_type = resolve_external_type(external_type, dependency_meta);
+    let request_str = simd_json::to_string(request).expect("invalid json to_string");
+    let attrs_str = dependency_meta
+      .attributes
+      .as_ref()
+      .map_or(String::new(), |attrs| {
+        format!(
+          " {}",
+          simd_json::to_string(attrs).expect("invalid json to_string")
+        )
+      });
+    let phase_str = if dependency_meta.phase == ImportPhase::Evaluation {
+      String::new()
+    } else {
+      format!(" phase={}", dependency_meta.phase.as_str())
+    };
+    let css_import_str =
+      css_module_render_conditions_identifier(dependency_meta.css_import_conditions.iter())
+        .map_or(String::new(), |conditions| {
+          format!(" css-import-conditions={}", json_stringify_str(&conditions))
+        });
+    Identifier::from(format!(
+      "external {resolved_type} {request_str}{attrs_str}{phase_str}{css_import_str}"
+    ))
+  }
+
   pub fn new(
     request: ExternalRequest,
     external_type: ExternalType,
@@ -493,30 +593,7 @@ impl ExternalModule {
   ) -> Self {
     Self {
       dependencies_block: Default::default(),
-      id: Identifier::from({
-        let resolved_type = resolve_external_type(external_type.as_str(), &dependency_meta);
-        let request_str = simd_json::to_string(&request).expect("invalid json to_string");
-        let attrs_str = dependency_meta
-          .attributes
-          .as_ref()
-          .map_or(String::new(), |attrs| {
-            format!(
-              " {}",
-              simd_json::to_string(attrs).expect("invalid json to_string")
-            )
-          });
-        let phase_str = if dependency_meta.phase == ImportPhase::Evaluation {
-          String::new()
-        } else {
-          format!(" phase={}", dependency_meta.phase.as_str())
-        };
-        let css_import_str =
-          css_module_render_conditions_identifier(dependency_meta.css_import_conditions.iter())
-            .map_or(String::new(), |conditions| {
-              format!(" css-import-conditions={}", json_stringify_str(&conditions))
-            });
-        format!("external {resolved_type} {request_str}{attrs_str}{phase_str}{css_import_str}")
-      }),
+      id: Self::create_identifier(&request, &external_type, &dependency_meta),
       request,
       external_type,
       user_request,
@@ -577,7 +654,17 @@ impl ExternalModule {
   }
 
   pub fn set_external_type(&mut self, new_type: ExternalType) {
+    if let ExternalRequest::Map(map) = &mut self.request
+      && !map.contains_key(&new_type)
+      && let Some(request) = map.get(&self.external_type).cloned()
+    {
+      // Preserve the request selected by the previous type when a plugin
+      // changes how the external is rendered. Object-form externals are keyed
+      // by type, so changing only the type would make `get_request` panic.
+      map.insert(new_type.clone(), request);
+    }
     self.external_type = new_type;
+    self.id = Self::create_identifier(&self.request, &self.external_type, &self.dependency_meta);
   }
 
   pub fn get_request(&self) -> &ExternalRequestValue {
@@ -587,6 +674,10 @@ impl ExternalModule {
     }
   }
 
+  pub fn try_get_request(&self) -> Option<&ExternalRequestValue> {
+    self.get_request_and_external_type().0
+  }
+
   fn get_request_and_external_type(&self) -> (Option<&ExternalRequestValue>, &ExternalType) {
     match &self.request {
       ExternalRequest::Single(request) => (Some(request), &self.external_type),
@@ -606,6 +697,28 @@ impl ExternalModule {
     let mut chunk_init_fragments: ChunkInitFragments = Default::default();
     let supports_const = compilation.options.output.environment.supports_const();
     let resolved_external_type = self.resolve_external_type();
+    if let Some(require_kind) =
+      CommonJsExternalRequireKind::from_external_type(resolved_external_type)
+    {
+      // For a missing object-form request, only ESM node-commonjs uses the
+      // undefined value; other CommonJS types request the name "undefined".
+      let fallback = (require_kind != CommonJsExternalRequireKind::NodeCommonJs
+        || !compilation.options.output.module)
+        .then_some("undefined");
+      let require_expression = require_kind.render_expression(
+        request.map(ExternalRequestValue::primary).or(fallback),
+        request
+          .and_then(ExternalRequestValue::rest)
+          .unwrap_or_default(),
+        compilation,
+        &mut chunk_init_fragments,
+      );
+      let source = format!(
+        "{} = {require_expression};",
+        get_namespace_object_export(concatenation_scope, supports_const, runtime_template)
+      );
+      return Ok((RawStringSource::from(source).boxed(), chunk_init_fragments));
+    }
     let module_graph = compilation.get_module_graph();
     let module_graph_cache = &compilation.module_graph_cache_artifact;
 
@@ -625,66 +738,6 @@ impl ExternalModule {
         get_namespace_object_export(concatenation_scope, supports_const, runtime_template),
         get_source_for_global_variable_external(request, &compilation.options.output.global_object)
       ),
-      "commonjs" | "commonjs2" | "commonjs-module" | "commonjs-static" => {
-        format!(
-          "{} = {};",
-          get_namespace_object_export(concatenation_scope, supports_const, runtime_template),
-          get_source_for_commonjs(request)
-        )
-      }
-      "node-commonjs" => {
-        let need_prefix = compilation
-          .options
-          .output
-          .environment
-          .supports_node_prefix_for_core_modules();
-
-        if compilation.options.output.module {
-          chunk_init_fragments.push(
-            NormalInitFragment::new(
-              format!(
-                "import {{ createRequire as __rspack_createRequire }} from \"{}\";\n{} __rspack_createRequire_require = __rspack_createRequire({}.url);\n",
-                if need_prefix { "node:module" } else { "module" },
-                if compilation.options.output.environment.supports_const() {
-                  "const"
-                } else {
-                  "var"
-                },
-                compilation.options.output.import_meta_name
-              ),
-              InitFragmentStage::StageESMImports,
-              0,
-              InitFragmentKey::ModuleExternal("node-commonjs".to_string()),
-              None,
-            )
-            .with_top_level_decl_symbols(vec![
-              "__rspack_createRequire".into(),
-              "__rspack_createRequire_require".into(),
-            ])
-            .boxed(),
-          );
-          let (request, specifiers) = if let Some(request) = request {
-            (
-              json_stringify_str(request.primary()),
-              property_access(request.iter(), 1),
-            )
-          } else {
-            ("undefined".to_string(), String::new())
-          };
-          format!(
-            "{} = __rspack_createRequire_require({}){};",
-            get_namespace_object_export(concatenation_scope, supports_const, runtime_template),
-            request,
-            specifiers
-          )
-        } else {
-          format!(
-            "{} = {};",
-            get_namespace_object_export(concatenation_scope, supports_const, runtime_template),
-            get_source_for_commonjs(request)
-          )
-        }
-      }
       "amd" | "amd-require" | "umd" | "umd2" | "system" | "jsonp" => {
         let id = ChunkGraph::get_module_id(&compilation.module_ids_artifact, self.identifier())
           .map(|s| s.as_str())

crates/rspack_core/src/utils/concatenated_module_visitor.rs

@@ -59,11 +59,10 @@ impl Visit for IdentCollector {
     }
   }
 
-  /// https://github.com/webpack/webpack/blob/1f99ad6367f2b8a6ef17cce0e058f7a67fb7db18/lib/optimize/ConcatenatedModule.js#L1173-L1197
+  /// Reserve every class expression's inner name, even without a superclass,
+  /// so renaming an outer binding cannot make it captured by the class scope.
   fn visit_class_expr(&mut self, node: &ClassExpr) {
-    if let Some(ref ident) = node.ident
-      && node.class.super_class.is_some()
-    {
+    if let Some(ref ident) = node.ident {
       self.ids.push(ConcatenatedModuleIdent {
         id: ident.clone(),
         shorthand: false,