fix: only import needed runtime symbols (#324)

<!-- Thank you for contributing! -->

### Description

The fix is done by more delicate analyzing the module type and import kind.

This also fixes #319 (comment).

<!-- Please insert your description here and provide especially info about the "what" this PR is solving -->

### Test Plan

<!-- e.g. is there anything you'd like reviewers to focus on? -->

---

crates/rolldown/src/bundler/linker/linker.rs

@@ -21,10 +21,8 @@ impl<'graph> Linker<'graph> {
     // Here take the symbols to avoid borrow graph and mut borrow graph at same time
     let mut symbols = std::mem::take(&mut self.graph.symbols);
 
-    self.mark_module_wrapped(&symbols, linking_infos);
-
     // Create symbols for external module
-    self.mark_extra_symbols(&mut symbols);
+    self.initialize_symbols_for_external_modules(&mut symbols);
 
     // Propagate star exports
     // Create resolved exports for named export declarations
@@ -78,79 +76,10 @@ impl<'graph> Linker<'graph> {
 
     // Set the symbols back and add linker modules to graph
     self.graph.symbols = symbols;
-
-    // FIXME: should move `linking_info.facade_stmt_infos` into a separate field
-    for (id, linking_info) in linking_infos.iter_mut_enumerated() {
-      std::mem::take(&mut linking_info.facade_stmt_infos).into_iter().for_each(|info| {
-        if let Module::Normal(module) = &mut self.graph.modules[id] {
-          module.stmt_infos.add_stmt_info(info);
-        }
-      });
-    }
-  }
-
-  // TODO: should move this to a separate stage
-  fn mark_module_wrapped(&self, _symbols: &Symbols, linking_infos: &mut LinkingInfoVec) {
-    // Generate symbol for import warp module
-    // Case esm import commonjs, eg var commonjs_ns = __toESM(require_a())
-    // Case commonjs require esm, eg (init_esm(), __toCommonJS(esm_ns))
-    // Case esm export star commonjs, eg __reExport(esm_ns, __toESM(require_a())
-    for module in &self.graph.modules {
-      match module {
-        Module::Normal(importer) => {
-          importer.static_imports().for_each(|r| {
-            let importee_linking_info =
-              &linking_infos.get(r.resolved_module).unwrap_or_else(|| {
-                panic!("importer: {:?}, importee_linking_info {:#?}", importer.resource_id, r,)
-              });
-            let importee = &self.graph.modules[r.resolved_module];
-            let Module::Normal(importee) = importee else {
-              return;
-            };
-            if let Some(importee_warp_symbol) = importee_linking_info.wrapper_ref {
-              let importer_linking_info = &mut linking_infos[importer.id];
-              importer_linking_info.reference_symbol_in_facade_stmt_infos(importee_warp_symbol);
-              match (importer.exports_kind, importee.exports_kind) {
-                (ExportsKind::Esm | ExportsKind::CommonJs, ExportsKind::CommonJs) => {
-                  // importer.create_local_symbol_for_import_cjs(
-                  //   importee,
-                  //   importer_linking_info,
-                  //   symbols,
-                  // );
-                  importer_linking_info.reference_symbol_in_facade_stmt_infos(
-                    self.graph.runtime.resolve_symbol("__toESM"),
-                  );
-                }
-                (_, ExportsKind::Esm) => {
-                  importer_linking_info
-                    .reference_symbol_in_facade_stmt_infos(importee.namespace_symbol);
-                  importer_linking_info.reference_symbol_in_facade_stmt_infos(
-                    self.graph.runtime.resolve_symbol("__toCommonJS"),
-                  );
-                }
-                _ => {}
-              }
-            }
-          });
-          importer.star_export_modules().for_each(|id| match &self.graph.modules[id] {
-            Module::Normal(importee) => {
-              if importee.exports_kind == ExportsKind::CommonJs {
-                let importee_linking_info = &mut linking_infos[importer.id];
-                importee_linking_info.reference_symbol_in_facade_stmt_infos(
-                  self.graph.runtime.resolve_symbol("__reExport"),
-                );
-              }
-            }
-            Module::External(_) => {}
-          });
-        }
-        Module::External(_) => {}
-      }
-    }
   }
 
-  #[allow(clippy::needless_collect)]
-  fn mark_extra_symbols(&mut self, symbols: &mut Symbols) {
+  // TODO(hyf0): Probably should take a look when improving supporting external modules
+  fn initialize_symbols_for_external_modules(&mut self, symbols: &mut Symbols) {
     for importer_id in &self.graph.sorted_modules {
       let importer = &self.graph.modules[*importer_id];
 
@@ -165,16 +94,6 @@ impl<'graph> Linker<'graph> {
               extra_symbols.push((import_record.resolved_module, info.imported.clone()));
             }
           });
-          // importer.named_exports.iter().for_each(|(_, export)| match &export {
-          //   LocalOrReExport::Local(_) => {}
-          //   LocalOrReExport::Re(re) => {
-          //     let import_record = &importer.import_records[re.record_id];
-          //     let importee = &self.graph.modules[import_record.resolved_module];
-          //     if let Module::External(_) = importee {
-          //       extra_symbols.push((import_record.resolved_module, re.imported.clone()));
-          //     }
-          //   }
-          // });
         }
         Module::External(_) => {}
       }

crates/rolldown/src/bundler/stages/link_stage.rs

@@ -62,7 +62,15 @@ impl LinkStage {
     Linker::new(&mut self).link(&mut linking_infos);
     self.linking_infos = linking_infos;
     tracing::debug!("linking modules {:#?}", self.linking_infos);
-
+    self.reference_needed_symbols();
+    // FIXME: should move `linking_info.facade_stmt_infos` into a separate field
+    for (id, linking_info) in self.linking_infos.iter_mut_enumerated() {
+      std::mem::take(&mut linking_info.facade_stmt_infos).into_iter().for_each(|info| {
+        if let Module::Normal(module) = &mut self.modules[id] {
+          module.stmt_infos.add_stmt_info(info);
+        }
+      });
+    }
     self.include_statements();
     LinkStageOutput {
       modules: self.modules,
@@ -307,6 +315,57 @@ impl LinkStage {
       });
     });
   }
+
+  fn reference_needed_symbols(&mut self) {
+    self.modules.iter().for_each(|importer| {
+      let Module::Normal(importer) = importer else {
+        return;
+      };
+
+      importer.static_imports().for_each(|rec| {
+        let Module::Normal(importee) = &self.modules[rec.resolved_module] else {
+          return;
+        };
+        // Reference runtime symbols in importers of wrapped modules
+        match self.linking_infos[importee.id].wrap_kind {
+          WrapKind::Cjs | WrapKind::Esm => {
+            let importee_wrapper_ref =
+              self.linking_infos[importee.id].wrapper_ref.expect("Should have wrapper ref");
+            self.linking_infos[importer.id]
+              .reference_symbol_in_facade_stmt_infos(importee_wrapper_ref);
+
+            match (rec.kind, importee.exports_kind) {
+              (ImportKind::Import, ExportsKind::CommonJs) => {
+                self.linking_infos[importer.id]
+                  .reference_symbol_in_facade_stmt_infos(self.runtime.resolve_symbol("__toESM"));
+              }
+              (ImportKind::Require, ExportsKind::Esm) => {
+                self.linking_infos[importer.id]
+                  .reference_symbol_in_facade_stmt_infos(importee.namespace_symbol);
+                self.linking_infos[importer.id].reference_symbol_in_facade_stmt_infos(
+                  self.runtime.resolve_symbol("__toCommonJS"),
+                );
+              }
+              _ => {}
+            }
+          }
+          WrapKind::None => {}
+        }
+      });
+
+      importer.star_export_modules().for_each(|importee_id| {
+        let Module::Normal(importee) = &self.modules[importee_id] else {
+          return;
+        };
+        if importee.exports_kind == ExportsKind::CommonJs {
+          self.linking_infos[importer.id]
+            .reference_symbol_in_facade_stmt_infos(self.runtime.resolve_symbol("__reExport"));
+          self.linking_infos[importer.id]
+            .reference_symbol_in_facade_stmt_infos(self.runtime.resolve_symbol("__toESM"));
+        }
+      });
+    });
+  }
 }
 
 #[derive(PartialEq, Eq, PartialOrd, Ord, Hash)]

crates/rolldown/src/bundler/visitors/scanner.rs

@@ -149,7 +149,6 @@ impl<'ast> Scanner<'ast> {
   }
 
   fn add_star_import(&mut self, local: SymbolId, record_id: ImportRecordId) {
-    self.result.import_records[record_id].is_import_namespace = true;
     self.result.named_imports.insert(
       local,
       NamedImport { imported: Specifier::Star, imported_as: (self.idx, local).into(), record_id },
@@ -207,7 +206,6 @@ impl<'ast> Scanner<'ast> {
     let name_import =
       NamedImport { imported: Specifier::Star, imported_as: generated_imported_as_ref, record_id };
     self.result.named_imports.insert(generated_imported_as_ref.symbol, name_import);
-    self.result.import_records[record_id].is_import_namespace = true;
     self
       .result
       .named_exports

crates/rolldown/tests/esbuild/default/export_forms_common_js/artifacts.snap

@@ -12,9 +12,6 @@ import { __esm, __toCommonJS } from "./_rolldown_runtime.mjs";
 
 // a.js
 var abc;
-var a_ns = {
-	get abc() { return abc }
-};
 var init_a = __esm({
 	'a.js'() {
 		abc = undefined

crates/rolldown/tests/esbuild/default/require_main_cache_common_js/artifacts.snap

@@ -8,7 +8,7 @@ input_file: crates/rolldown/tests/esbuild/default/require_main_cache_common_js
 ## entry_js.mjs
 
 ```js
-import { __commonJS, __toESM } from "./_rolldown_runtime.mjs";
+import { __commonJS } from "./_rolldown_runtime.mjs";
 
 // is-main.js
 var require_is_main = __commonJS({

crates/rolldown/tests/fixtures/compat/esm_require_cjs/_test.mjs

@@ -0,0 +1,4 @@
+import assert from 'assert'
+import { cjs } from './dist/main.mjs'
+
+assert.equal(cjs, 'cjs')

crates/rolldown/tests/fixtures/compat/esm_require_cjs/artifacts.snap

@@ -0,0 +1,26 @@
+---
+source: crates/rolldown/tests/common/case.rs
+expression: content
+input_file: crates/rolldown/tests/fixtures/compat/esm_require_cjs
+---
+# Assets
+
+## main.mjs
+
+```js
+import { __commonJS } from "./_rolldown_runtime.mjs";
+
+// cjs.js
+var require_cjs = __commonJS({
+	'cjs.js'(exports, module) {
+		module.exports = 'cjs'
+
+	}
+});
+// main.js
+const cjs = require_cjs()
+
+
+
+export { cjs };
+```

crates/rolldown/tests/fixtures/compat/esm_require_cjs/cjs.js

@@ -0,0 +1 @@
+module.exports = 'cjs'

crates/rolldown/tests/fixtures/compat/esm_require_cjs/main.js

@@ -0,0 +1,3 @@
+const cjs = require('./cjs')
+
+export { cjs }

crates/rolldown/tests/fixtures/compat/esm_require_cjs/test.config.json

@@ -0,0 +1 @@
+{}

crates/rolldown/tests/fixtures/compat/esm_require_esm/_test.mjs

@@ -0,0 +1,4 @@
+import assert from 'assert'
+import { esm } from './dist/main.mjs'
+
+assert.equal(esm.default, 'esm')

crates/rolldown/tests/fixtures/compat/esm_require_esm/artifacts.snap

@@ -0,0 +1,31 @@
+---
+source: crates/rolldown/tests/common/case.rs
+assertion_line: 97
+expression: content
+input_file: crates/rolldown/tests/fixtures/compat/esm_require_esm
+---
+# Assets
+
+## main.mjs
+
+```js
+import { __esm, __toCommonJS } from "./_rolldown_runtime.mjs";
+
+// esm.js
+var esm_default;
+var esm_ns = {
+	get default() { return esm_default }
+};
+var init_esm = __esm({
+	'esm.js'() {
+		esm_default = 'esm'
+
+	}
+});
+// main.js
+const esm = (init_esm(), __toCommonJS(esm_ns))
+
+
+
+export { esm };
+```

crates/rolldown/tests/fixtures/compat/esm_require_esm/esm.js

@@ -0,0 +1 @@
+export default 'esm'

crates/rolldown/tests/fixtures/compat/esm_require_esm/main.js

@@ -0,0 +1,3 @@
+const esm = require('./esm')
+
+export { esm }

crates/rolldown/tests/fixtures/compat/esm_require_esm/test.config.json

@@ -0,0 +1 @@
+{}

crates/rolldown/tests/fixtures/compat/esm_require_esm_unused/artifacts.snap

@@ -0,0 +1,26 @@
+---
+source: crates/rolldown/tests/common/case.rs
+expression: content
+input_file: crates/rolldown/tests/fixtures/compat/esm_require_esm_unused
+---
+# Assets
+
+## main.mjs
+
+```js
+import { __esm, __toCommonJS } from "./_rolldown_runtime.mjs";
+
+// esm.js
+var esm_default;
+var esm_ns = {
+	get default() { return esm_default }
+};
+var init_esm = __esm({
+	'esm.js'() {
+		esm_default = 'esm'
+
+	}
+});
+// main.js
+init_esm()
+```

crates/rolldown/tests/fixtures/compat/esm_require_esm_unused/esm.js

@@ -0,0 +1 @@
+export default 'esm'

crates/rolldown/tests/fixtures/compat/esm_require_esm_unused/main.js

@@ -0,0 +1 @@
+require('./esm')

crates/rolldown/tests/fixtures/compat/esm_require_esm_unused/test.config.json

@@ -0,0 +1 @@
+{}

crates/rolldown_common/src/import_record.rs

@@ -35,23 +35,19 @@ impl Display for ImportKind {
 pub struct RawImportRecord {
   // Module Request
   pub module_request: Atom,
-  // export * as ns from '...'
-  // import * as ns from '...'
-  pub is_import_namespace: bool,
   pub kind: ImportKind,
   pub namespace_ref: SymbolRef,
 }
 
 impl RawImportRecord {
   pub fn new(specifier: Atom, kind: ImportKind, namespace_ref: SymbolRef) -> Self {
-    Self { module_request: specifier, is_import_namespace: false, kind, namespace_ref }
+    Self { module_request: specifier, kind, namespace_ref }
   }
 
   pub fn into_import_record(self, resolved_module: ModuleId) -> ImportRecord {
     ImportRecord {
       module_request: self.module_request,
       resolved_module,
-      is_import_namespace: self.is_import_namespace,
       kind: self.kind,
       namespace_ref: self.namespace_ref,
     }
@@ -63,9 +59,6 @@ pub struct ImportRecord {
   // Module Request
   pub module_request: Atom,
   pub resolved_module: ModuleId,
-  // export * as ns from '...'
-  // import * as ns from '...'
-  pub is_import_namespace: bool,
   pub kind: ImportKind,
   pub namespace_ref: SymbolRef,
 }