refactor: import external should not create needless external module (#12064)

Author: JSerFeng

Cargo.lock

@@ -2732,12 +2732,6 @@ export interface RawRslibPluginOptions {
    * @default `false`
    */
   interceptApiPlugin?: boolean
-  /**
-   * Use the compact runtime for dynamic import from `modern-module`, commonly used in CommonJS output.
-   * This field should not be set to `true` when using `modern-module` with ESM output, as it is already in use.
-   * @default `false`
-   */
-  compactExternalModuleDynamicImport?: boolean
   /**
    * Add shims for javascript/esm modules
    * @default `false`

crates/node_binding/napi-binding.d.ts

@@ -7,10 +7,6 @@ pub struct RawRslibPluginOptions {
   /// Intercept partial parse hooks of APIPlugin, expect some statements not to be parsed as API.
   /// @default `false`
   pub intercept_api_plugin: Option<bool>,
-  /// Use the compact runtime for dynamic import from `modern-module`, commonly used in CommonJS output.
-  /// This field should not be set to `true` when using `modern-module` with ESM output, as it is already in use.
-  /// @default `false`
-  pub compact_external_module_dynamic_import: Option<bool>,
   /// Add shims for javascript/esm modules
   /// @default `false`
   pub force_node_shims: Option<bool>,
@@ -20,9 +16,6 @@ impl From<RawRslibPluginOptions> for RslibPluginOptions {
   fn from(value: RawRslibPluginOptions) -> Self {
     Self {
       intercept_api_plugin: value.intercept_api_plugin.unwrap_or_default(),
-      compact_external_module_dynamic_import: value
-        .compact_external_module_dynamic_import
-        .unwrap_or_default(),
       force_node_shims: value.force_node_shims.unwrap_or_default(),
     }
   }

crates/rspack_binding_api/src/rslib.rs

@@ -11,8 +11,6 @@ mod utils;
 pub use amd_library_plugin::AmdLibraryPlugin;
 pub use assign_library_plugin::*;
 pub use export_property_library_plugin::ExportPropertyLibraryPlugin;
-pub use modern_module::ModernModuleImportDependencyTemplate;
-pub use modern_module_library_plugin::replace_import_dependencies_for_external_modules;
 use rspack_core::{BoxPlugin, PluginExt};
 pub use system_library_plugin::SystemLibraryPlugin;
 pub use umd_library_plugin::UmdLibraryPlugin;

crates/rspack_plugin_library/src/lib.rs

@@ -1,5 +1,2 @@
-mod import_dependency;
 mod reexport_star_external_dependency;
-
-pub use import_dependency::*;
 pub use reexport_star_external_dependency::*;

crates/rspack_plugin_library/src/modern_module/mod.rs

@@ -4,9 +4,9 @@ use rspack_collections::IdentifierMap;
 use rspack_core::{
   BoxDependency, ChunkUkey, CodeGenerationExportsFinalNames, Compilation,
   CompilationOptimizeChunkModules, CompilationParams, CompilerCompilation, CompilerFinishMake,
-  ConcatenatedModule, ConcatenatedModuleExportsDefinitions, DependenciesBlock, Dependency,
-  DependencyId, ExportsType, LibraryOptions, ModuleDependency, ModuleGraph, ModuleIdentifier,
-  Plugin, PrefetchExportsInfoMode, RuntimeSpec, UsedNameItem,
+  ConcatenatedModule, ConcatenatedModuleExportsDefinitions, DependencyId, ExportsType,
+  LibraryOptions, ModuleGraph, ModuleIdentifier, Plugin, PrefetchExportsInfoMode, RuntimeSpec,
+  UsedNameItem,
   rspack_sources::{ConcatSource, RawStringSource, SourceExt},
   to_identifier,
 };
@@ -16,82 +16,18 @@ use rspack_hook::{plugin, plugin_hook};
 use rspack_plugin_javascript::{
   ConcatConfiguration, JavascriptModulesChunkHash, JavascriptModulesRenderStartup, JsPlugin,
   ModuleConcatenationPlugin, RenderSource,
-  dependency::{
-    ESMExportImportedSpecifierDependency, ESMImportSideEffectDependency, ImportDependency,
-  },
+  dependency::{ESMExportImportedSpecifierDependency, ESMImportSideEffectDependency},
 };
 use rustc_hash::FxHashSet as HashSet;
 
 use super::modern_module::ModernModuleReexportStarExternalDependency;
 use crate::{
-  modern_module::{
-    ModernModuleImportDependency, ModernModuleImportDependencyTemplate,
-    ModernModuleReexportStarExternalDependencyTemplate,
-  },
+  modern_module::ModernModuleReexportStarExternalDependencyTemplate,
   utils::{COMMON_LIBRARY_NAME_MESSAGE, get_options_for_chunk},
 };
 
 const PLUGIN_NAME: &str = "rspack.ModernModuleLibraryPlugin";
 
-/// Replaces ImportDependency instances with ModernModuleImportDependency for external modules.
-/// This function iterates through all modules and their blocks to find ImportDependencies
-/// that point to external modules, then creates ModernModuleImportDependency replacements.
-pub fn replace_import_dependencies_for_external_modules(
-  compilation: &mut Compilation,
-) -> Result<()> {
-  let mg = compilation.get_module_graph();
-  let mut deps_to_replace: Vec<BoxDependency> = Vec::new();
-
-  for module in mg.modules().values() {
-    for block_id in module.get_blocks() {
-      let Some(block) = mg.block_by_id(block_id) else {
-        continue;
-      };
-      for block_dep_id in block.get_dependencies() {
-        let block_dep = mg.dependency_by_id(block_dep_id);
-        if let Some(block_dep) = block_dep
-          && let Some(import_dependency) = block_dep.as_any().downcast_ref::<ImportDependency>()
-        {
-          let import_dep_connection = mg.connection_by_dependency_id(block_dep_id);
-          if let Some(import_dep_connection) = import_dep_connection {
-            // Try find the connection with a import dependency pointing to an external module.
-            // If found, remove the connection and add a new import dependency to performs the external module ID replacement.
-            let import_module_id = import_dep_connection.module_identifier();
-            let Some(import_module) = mg.module_by_identifier(import_module_id) else {
-              continue;
-            };
-
-            if let Some(external_module) = import_module.as_external_module() {
-              let new_dep = ModernModuleImportDependency::new(
-                *block_dep.id(),
-                import_dependency.request().into(),
-                external_module.request.clone(),
-                external_module.external_type.clone(),
-                import_dependency.range,
-                import_dependency.get_attributes().cloned(),
-                import_dependency.comments.clone(),
-              );
-
-              deps_to_replace.push(Box::new(new_dep));
-            }
-          }
-        }
-      }
-    }
-  }
-
-  let mut mg = compilation.get_module_graph_mut();
-  for dep in deps_to_replace {
-    let dep_id = dep.id();
-    // remove connection
-    mg.revoke_dependency(dep_id, false);
-    // overwrite dependency
-    mg.add_dependency(dep);
-  }
-
-  Ok(())
-}
-
 #[plugin]
 #[derive(Debug, Default)]
 pub struct ModernModuleLibraryPlugin;
@@ -489,8 +425,6 @@ pub fn render_as_default_export_impl(exports: &[(String, Option<String>)]) -> St
 
 #[plugin_hook(CompilerFinishMake for ModernModuleLibraryPlugin)]
 async fn finish_make(&self, compilation: &mut Compilation) -> Result<()> {
-  // Replace ImportDependency instances with ModernModuleImportDependency for external modules
-  replace_import_dependencies_for_external_modules(compilation)?;
   self.preserve_reexports_star(compilation)?;
 
   Ok(())
@@ -527,10 +461,6 @@ async fn compilation(
   hooks.render_startup.tap(render_startup::new(self));
   hooks.chunk_hash.tap(js_chunk_hash::new(self));
 
-  compilation.set_dependency_template(
-    ModernModuleImportDependencyTemplate::template_type(),
-    Arc::new(ModernModuleImportDependencyTemplate::default()),
-  );
   compilation.set_dependency_template(
     ModernModuleReexportStarExternalDependencyTemplate::template_type(),
     Arc::new(ModernModuleReexportStarExternalDependencyTemplate::default()),

crates/rspack_plugin_library/src/modern_module_library_plugin.rs

@@ -8,13 +8,15 @@ version.workspace = true
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+rspack_cacheable         = { workspace = true }
 rspack_core              = { workspace = true }
 rspack_error             = { workspace = true }
 rspack_hook              = { workspace = true }
 rspack_plugin_javascript = { workspace = true }
-rspack_plugin_library    = { workspace = true }
-swc_core                 = { workspace = true }
-tracing                  = { workspace = true }
+
+serde_json = { workspace = true }
+swc_core   = { workspace = true }
+tracing    = { workspace = true }
 
 [package.metadata.cargo-shear]
 ignored = ["tracing"]

crates/rspack_plugin_rslib/Cargo.toml

@@ -10,7 +10,7 @@ use swc_core::ecma::atoms::Atom;
 
 #[cacheable]
 #[derive(Debug, Clone)]
-pub struct ModernModuleImportDependency {
+pub struct RslibImportDependency {
   id: DependencyId,
   #[cacheable(with=AsPreset)]
   request: Atom,
@@ -23,7 +23,7 @@ pub struct ModernModuleImportDependency {
   pub comments: Vec<(bool, String)>,
 }
 
-impl ModernModuleImportDependency {
+impl RslibImportDependency {
   pub fn new(
     id: DependencyId,
     request: Atom,
@@ -50,7 +50,7 @@ impl ModernModuleImportDependency {
 }
 
 #[cacheable_dyn]
-impl Dependency for ModernModuleImportDependency {
+impl Dependency for RslibImportDependency {
   fn id(&self) -> &DependencyId {
     &self.id
   }
@@ -81,7 +81,7 @@ impl Dependency for ModernModuleImportDependency {
 }
 
 #[cacheable_dyn]
-impl ModuleDependency for ModernModuleImportDependency {
+impl ModuleDependency for RslibImportDependency {
   fn request(&self) -> &str {
     &self.request
   }
@@ -100,24 +100,24 @@ impl ModuleDependency for ModernModuleImportDependency {
 }
 
 #[cacheable_dyn]
-impl DependencyCodeGeneration for ModernModuleImportDependency {
+impl DependencyCodeGeneration for RslibImportDependency {
   fn dependency_template(&self) -> Option<DependencyTemplateType> {
-    Some(ModernModuleImportDependencyTemplate::template_type())
+    Some(RslibDependencyTemplate::template_type())
   }
 }
 
-impl AsContextDependency for ModernModuleImportDependency {}
+impl AsContextDependency for RslibImportDependency {}
 
 #[cacheable]
 #[derive(Debug, Clone, Default)]
-pub struct ModernModuleImportDependencyTemplate;
-impl ModernModuleImportDependencyTemplate {
+pub struct RslibDependencyTemplate;
+impl RslibDependencyTemplate {
   pub fn template_type() -> DependencyTemplateType {
-    DependencyTemplateType::Custom("ModernModuleImportDependency")
+    DependencyTemplateType::Custom("RslibImportDependency")
   }
 }
 
-impl DependencyTemplate for ModernModuleImportDependencyTemplate {
+impl DependencyTemplate for RslibDependencyTemplate {
   fn render(
     &self,
     dep: &dyn DependencyCodeGeneration,
@@ -126,10 +126,8 @@ impl DependencyTemplate for ModernModuleImportDependencyTemplate {
   ) {
     let dep = dep
       .as_any()
-      .downcast_ref::<ModernModuleImportDependency>()
-      .expect(
-        "ModernModuleImportDependencyTemplate should be used for ModernModuleImportDependency",
-      );
+      .downcast_ref::<RslibImportDependency>()
+      .expect("RslibDependencyTemplate should be used for RslibImportDependency");
 
     let request_and_external_type = match &dep.target_request {
       ExternalRequest::Single(request) => (Some(request), &dep.external_type),

crates/rspack_plugin_library/src/modern_module/import_dependency.rs renamed to crates/rspack_plugin_rslib/src/import_dependency.rs

@@ -0,0 +1,64 @@
+use rspack_core::{BoxDependency, Compilation, DependenciesBlock, Dependency, ModuleDependency};
+use rspack_error::Result;
+use rspack_plugin_javascript::dependency::ImportDependency;
+
+use crate::import_dependency::RslibImportDependency;
+
+/// Replaces ImportDependency instances with RslibImportDependency for external modules.
+/// This function iterates through all modules and their blocks to find ImportDependencies
+/// that point to external modules, then creates RslibImportDependency replacements.
+pub fn replace_import_dependencies_for_external_modules(
+  compilation: &mut Compilation,
+) -> Result<()> {
+  let mg = compilation.get_module_graph();
+  let mut deps_to_replace: Vec<BoxDependency> = Vec::new();
+
+  for module in mg.modules().values() {
+    for block_id in module.get_blocks() {
+      let Some(block) = mg.block_by_id(block_id) else {
+        continue;
+      };
+      for block_dep_id in block.get_dependencies() {
+        let block_dep = mg.dependency_by_id(block_dep_id);
+        if let Some(block_dep) = block_dep
+          && let Some(import_dependency) = block_dep.as_any().downcast_ref::<ImportDependency>()
+        {
+          let import_dep_connection = mg.connection_by_dependency_id(block_dep_id);
+          if let Some(import_dep_connection) = import_dep_connection {
+            // Try find the connection with a import dependency pointing to an external module.
+            // If found, remove the connection and add a new import dependency to performs the external module ID replacement.
+            let import_module_id = import_dep_connection.module_identifier();
+            let Some(import_module) = mg.module_by_identifier(import_module_id) else {
+              continue;
+            };
+
+            if let Some(external_module) = import_module.as_external_module() {
+              let new_dep = RslibImportDependency::new(
+                *block_dep.id(),
+                import_dependency.request().into(),
+                external_module.request.clone(),
+                external_module.external_type.clone(),
+                import_dependency.range,
+                import_dependency.get_attributes().cloned(),
+                import_dependency.comments.clone(),
+              );
+
+              deps_to_replace.push(Box::new(new_dep));
+            }
+          }
+        }
+      }
+    }
+  }
+
+  let mut mg = compilation.get_module_graph_mut();
+  for dep in deps_to_replace {
+    let dep_id = dep.id();
+    // remove connection
+    mg.revoke_dependency(dep_id, false);
+    // overwrite dependency
+    mg.add_dependency(dep);
+  }
+
+  Ok(())
+}

crates/rspack_plugin_rslib/src/import_external.rs

@@ -1,4 +1,5 @@
+mod import_dependency;
+mod import_external;
 mod parser_plugin;
 mod plugin;
-
 pub use plugin::*;