fix: generate local symbol for esm import cjs instead of reference cj…

…s namespace symbol (#141)

crates/rolldown/src/bundler/chunk/render_chunk_exports.rs

@@ -35,10 +35,8 @@ impl Chunk {
         let canonical_name = &self.canonical_names[&canonical_ref];
         if let Some(ns_alias) = &symbol.namespace_alias {
           let canonical_ns_name = &self.canonical_names[&ns_alias.namespace_ref];
-          s.append(format!(
-            "var {canonical_name} = {canonical_ns_name}.{};\n",
-            ns_alias.property_name
-          ));
+          let property_name = &ns_alias.property_name;
+          s.append(format!("var {canonical_name} = {canonical_ns_name}.{property_name};\n"));
         }
         if canonical_name == exported_name {
           format!("{canonical_name}")

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

@@ -169,7 +169,7 @@ impl<'graph> Linker<'graph> {
 
     // Linking the module imports to resolved exports
     self.graph.sorted_modules.clone().into_iter().for_each(|id| {
-      self.match_imports_with_exports(id, &mut symbols, &mut linking_infos, &self.graph.modules);
+      self.match_imports_with_exports(id, &mut symbols, &linking_infos, &self.graph.modules);
     });
 
     // Exclude ambiguous from resolved exports
@@ -233,29 +233,23 @@ impl<'graph> Linker<'graph> {
             };
             if let Some(importee_warp_symbol) = importee_linking_info.wrap_symbol {
               let importer_linking_info = &mut linking_infos[importer.id];
-              importer.reference_symbol_in_facade_stmt_infos(
-                importee_warp_symbol,
-                importer_linking_info,
-                symbols,
-              );
-              importer.reference_symbol_in_facade_stmt_infos(
-                importee.namespace_symbol,
-                importer_linking_info,
-                symbols,
-              );
+              importer_linking_info.reference_symbol_in_facade_stmt_infos(importee_warp_symbol);
               match (importer.exports_kind, importee.exports_kind) {
                 (ExportsKind::Esm, ExportsKind::CommonJs) => {
-                  importer.reference_symbol_in_facade_stmt_infos(
-                    self.graph.runtime.resolve_symbol(&"__toESM".into()),
+                  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".into()),
+                  );
                 }
                 (_, ExportsKind::Esm) => {
-                  importer.reference_symbol_in_facade_stmt_infos(
+                  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".into()),
-                    importer_linking_info,
-                    symbols,
                   );
                 }
                 _ => {}
@@ -265,10 +259,9 @@ impl<'graph> Linker<'graph> {
           importer.star_export_modules().for_each(|id| match &self.graph.modules[id] {
             Module::Normal(importee) => {
               if importee.exports_kind == ExportsKind::CommonJs {
-                importer.reference_symbol_in_facade_stmt_infos(
+                let importee_linking_info = &mut linking_infos[importer.id];
+                importee_linking_info.reference_symbol_in_facade_stmt_infos(
                   self.graph.runtime.resolve_symbol(&"__reExport".into()),
-                  &mut linking_infos[importer.id],
-                  symbols,
                 );
               }
             }
@@ -306,7 +299,7 @@ impl<'graph> Linker<'graph> {
           "__esm".into()
         };
         let runtime_symbol = self.graph.runtime.resolve_symbol(&name);
-        module.reference_symbol_in_facade_stmt_infos(runtime_symbol, linking_info, symbols);
+        linking_info.reference_symbol_in_facade_stmt_infos(runtime_symbol);
         module.import_records.iter().for_each(|record| {
           self.wrap_module(record.resolved_module, symbols, linking_infos);
         });
@@ -369,13 +362,12 @@ impl<'graph> Linker<'graph> {
     &self,
     id: ModuleId,
     symbols: &mut Symbols,
-    linking_infos: &mut LinkingInfoVec,
+    linking_infos: &LinkingInfoVec,
     modules: &ModuleVec,
   ) {
     let importer = &self.graph.modules[id];
     match importer {
       Module::Normal(importer) => {
-        let mut local_symbols = vec![];
         importer
           .named_imports
           .values()
@@ -389,6 +381,7 @@ impl<'graph> Linker<'graph> {
                   modules,
                   importee,
                   &linking_infos[importee.id],
+                  &linking_infos[importer.id],
                   info,
                 ) {
                   MatchImportKind::NotFound => panic!(""),
@@ -408,12 +401,15 @@ impl<'graph> Linker<'graph> {
                   MatchImportKind::Found(symbol_ref) => {
                     symbols.union(info.imported_as, symbol_ref);
                   }
-                  MatchImportKind::NameSpace => {
-                    symbols.get_mut(info.imported_as).namespace_alias = Some(NamespaceAlias {
-                      property_name: info.imported.clone(),
-                      namespace_ref: importee.namespace_symbol,
-                    });
-                    local_symbols.push(importee.namespace_symbol);
+                  MatchImportKind::NameSpace(symbol_ref) => {
+                    if info.is_imported_star {
+                      symbols.union(info.imported_as, symbol_ref);
+                    } else {
+                      symbols.get_mut(info.imported_as).namespace_alias = Some(NamespaceAlias {
+                        property_name: info.imported.clone(),
+                        namespace_ref: symbol_ref,
+                      });
+                    }
                   }
                 }
               }
@@ -423,14 +419,6 @@ impl<'graph> Linker<'graph> {
               }
             }
           });
-
-        local_symbols.into_iter().for_each(|symbol_ref| {
-          importer.reference_symbol_in_facade_stmt_infos(
-            symbol_ref,
-            &mut linking_infos[importer.id],
-            symbols,
-          );
-        });
       }
       Module::External(_) => {
         // It's meaningless to be a importer for a external module.
@@ -459,6 +447,7 @@ impl<'graph> Linker<'graph> {
                   modules,
                   importee,
                   &linking_infos[importee_id],
+                  module_linking_info,
                   info,
                 ));
               }
@@ -472,6 +461,7 @@ impl<'graph> Linker<'graph> {
                   modules,
                   importee,
                   &linking_infos[importee_id],
+                  module_linking_info,
                   info,
                 ));
               }
@@ -502,15 +492,19 @@ impl<'graph> Linker<'graph> {
     modules: &ModuleVec,
     importee: &NormalModule,
     importee_linking_info: &LinkingInfo,
+    importer_linking_info: &LinkingInfo,
     info: &NamedImport,
   ) -> MatchImportKind {
-    if info.is_imported_star {
-      return MatchImportKind::Found(importee.namespace_symbol);
-    }
-
     // If importee module is commonjs module, it will generate property access to namespace symbol
+    // The namespace symbols should be importer created local symbol.
     if importee.exports_kind == ExportsKind::CommonJs {
-      return MatchImportKind::NameSpace;
+      return MatchImportKind::NameSpace(
+        importer_linking_info.local_symbol_for_import_cjs[&importee.id],
+      );
+    }
+
+    if info.is_imported_star {
+      return MatchImportKind::Found(importee.namespace_symbol);
     }
 
     if let Some(resolved_export) = importee_linking_info.resolved_exports.get(&info.imported) {
@@ -526,7 +520,9 @@ impl<'graph> Linker<'graph> {
         match module {
           Module::Normal(module) => {
             if module.exports_kind == ExportsKind::CommonJs {
-              return MatchImportKind::NameSpace;
+              return MatchImportKind::NameSpace(
+                importer_linking_info.local_symbol_for_import_cjs[&module.id],
+              );
             }
           }
           Module::External(_) => {}
@@ -536,8 +532,9 @@ impl<'graph> Linker<'graph> {
     }
 
     // If the module has dynamic exports, the unknown export name will be resolved at runtime.
+    // The namespace symbol should be importee namespace symbol.
     if importee_linking_info.has_dynamic_exports {
-      return MatchImportKind::NameSpace;
+      return MatchImportKind::NameSpace(importee.namespace_symbol);
     }
 
     MatchImportKind::NotFound
@@ -563,7 +560,10 @@ impl<'graph> Linker<'graph> {
         }
         for id in module.star_export_modules() {
           if self.mark_dynamic_exports_due_to_export_star(id, linking_infos, visited_modules) {
-            linking_infos[target].has_dynamic_exports = true;
+            let module_linking_info = &mut linking_infos[target];
+            module_linking_info.has_dynamic_exports = true;
+            // Dynamic exports will generate `__reExport(ns, xx)`, here should reference self namespace symbol
+            module_linking_info.reference_symbol_in_facade_stmt_infos(module.namespace_symbol);
             return true;
           }
         }
@@ -578,7 +578,7 @@ impl<'graph> Linker<'graph> {
 pub enum MatchImportKind {
   NotFound,
   // The import symbol will generate property access to namespace symbol
-  NameSpace,
+  NameSpace(SymbolRef),
   // External,
   PotentiallyAmbiguous(SymbolRef, Vec<SymbolRef>),
   Found(SymbolRef),

crates/rolldown/src/bundler/graph/linker_info.rs

@@ -25,6 +25,8 @@ pub struct LinkingInfo {
   // The unknown export name will be resolved at runtime.
   // esbuild add it to `ExportKind`, but the linker shouldn't mutate the module.
   pub has_dynamic_exports: bool,
+  // Store the local symbol for esm import cjs. eg. `var import_ns = __toESM(require_cjs())`
+  pub local_symbol_for_import_cjs: FxHashMap<ModuleId, SymbolRef>,
 }
 
 impl LinkingInfo {
@@ -56,6 +58,16 @@ impl LinkingInfo {
     export_names.sort_unstable_by(|a, b| a.cmp(b));
     self.exclude_ambiguous_sorted_resolved_exports = export_names;
   }
+
+  pub fn reference_symbol_in_facade_stmt_infos(&mut self, symbol_ref: SymbolRef) {
+    self.facade_stmt_infos.push(StmtInfo {
+      declared_symbols: vec![],
+      // Since the facade symbol is used, it should be referenced. This will be used to
+      // create correct cross-chunk links
+      referenced_symbols: vec![symbol_ref],
+      ..Default::default()
+    });
+  }
 }
 
 pub type LinkingInfoVec = IndexVec<ModuleId, LinkingInfo>;

crates/rolldown/src/bundler/module/normal_module.rs

@@ -32,6 +32,7 @@ pub struct NormalModule {
   pub id: ModuleId,
   pub is_entry: bool,
   pub resource_id: ResourceId,
+  pub unique_name: String,
   pub module_type: ModuleType,
   pub namespace_symbol: SymbolRef,
   pub ast: OxcProgram,
@@ -222,7 +223,7 @@ impl NormalModule {
       let name = format!(
         "{}_{}",
         if self.exports_kind == ExportsKind::CommonJs { "require" } else { "init" },
-        self.resource_id.generate_unique_name()
+        self.unique_name
       )
       .into();
       let symbol_ref = self.create_local_symbol(name, self_linking_info, symbols);
@@ -243,21 +244,18 @@ impl NormalModule {
     symbol_ref
   }
 
-  pub fn reference_symbol_in_facade_stmt_infos(
+  #[allow(clippy::map_entry, clippy::use_self)]
+  pub fn create_local_symbol_for_import_cjs(
     &self,
-    other_module_symbol_ref: SymbolRef,
+    importee: &NormalModule,
     self_linking_info: &mut LinkingInfo,
-    _symbols: &mut Symbols,
+    symbols: &mut Symbols,
   ) {
-    debug_assert!(other_module_symbol_ref.owner != self.id);
-
-    self_linking_info.facade_stmt_infos.push(StmtInfo {
-      declared_symbols: vec![],
-      // Since the facade symbol is used, it should be referenced. This will be used to
-      // create correct cross-chunk links
-      referenced_symbols: vec![other_module_symbol_ref],
-      ..Default::default()
-    });
+    if !self_linking_info.local_symbol_for_import_cjs.contains_key(&importee.id) {
+      let name = format!("import_{}", importee.unique_name).into();
+      let symbol_ref = self.create_local_symbol(name, self_linking_info, symbols);
+      self_linking_info.local_symbol_for_import_cjs.insert(importee.id, symbol_ref);
+    }
   }
 
   pub fn star_export_modules(&self) -> impl Iterator<Item = ModuleId> + '_ {

crates/rolldown/src/bundler/module/normal_module_builder.rs

@@ -15,6 +15,7 @@ use super::NormalModule;
 #[derive(Debug, Default)]
 pub struct NormalModuleBuilder {
   pub id: Option<ModuleId>,
+  pub unique_name: Option<String>,
   pub path: Option<ResourceId>,
   pub ast: Option<OxcProgram>,
   pub named_imports: Option<FxHashMap<SymbolId, NamedImport>>,
@@ -36,6 +37,7 @@ impl NormalModuleBuilder {
     NormalModule {
       exec_order: u32::MAX,
       id: self.id.unwrap(),
+      unique_name: self.unique_name.unwrap(),
       resource_id: self.path.unwrap(),
       ast: self.ast.unwrap(),
       named_imports: self.named_imports.unwrap(),

crates/rolldown/src/bundler/module_loader/normal_module_task.rs

@@ -76,10 +76,12 @@ impl NormalModuleTask {
       export_default_symbol_id,
       imports,
       exports_kind,
+      unique_name,
     } = scan_result;
 
     builder.id = Some(self.module_id);
     builder.ast = Some(ast);
+    builder.unique_name = Some(unique_name);
     builder.path = Some(self.path);
     builder.named_imports = Some(named_imports);
     builder.named_exports = Some(named_exports);
@@ -121,7 +123,7 @@ impl NormalModuleTask {
       self.module_id,
       &mut scope,
       &mut symbol_table,
-      &unique_name,
+      unique_name,
       self.module_type,
     );
     scanner.visit_program(program.program_mut());

crates/rolldown/src/bundler/module_loader/runtime_normal_module_task.rs

@@ -60,10 +60,12 @@ impl RuntimeNormalModuleTask {
       export_default_symbol_id,
       imports,
       exports_kind,
+      unique_name,
     } = scan_result;
 
     builder.id = Some(self.module_id);
     builder.ast = Some(ast);
+    builder.unique_name = Some(unique_name);
     builder.path = Some(ResourceId::new(RUNTIME_PATH.to_string().into(), self.resolver.cwd()));
     builder.named_imports = Some(named_imports);
     builder.named_exports = Some(named_exports);
@@ -102,7 +104,7 @@ impl RuntimeNormalModuleTask {
       self.module_id,
       &mut scope,
       &mut symbol_table,
-      "should be unreachable for runtime module",
+      RUNTIME_PATH.to_string(),
       self.module_type,
     );
     let namespace_symbol = scanner.namespace_symbol;

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

@@ -21,9 +21,7 @@ impl<'ast> CjsRenderer<'ast> {
       "var {wrap_symbol_name} = {commonjs_runtime_symbol_name}({{\n'{module_path}'(exports, module) {{\n",
     ));
     self.base.source.append("\n}\n});");
-    if let Some(s) = self.base.generate_namespace_variable_declaration() {
-      self.base.source.prepend(s);
-    }
+    assert!(!self.base.module.is_namespace_referenced());
   }
 }
 

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

@@ -50,16 +50,14 @@ impl<'ast> Visit<'ast> for EsmRenderer<'ast> {
       match self.base.get_importee_by_span(named_decl.span) {
         Module::Normal(importee) => {
           if importee.exports_kind == ExportsKind::CommonJs {
-            self.base.overwrite(
+            self.base.hoisted_module_declaration(
               named_decl.span.start,
-              named_decl.span.end,
               self.base.generate_import_commonjs_module(
                 importee,
                 &self.base.graph.linking_infos[importee.id],
                 true,
               ),
             );
-            return;
           }
         }
         Module::External(_) => {} // TODO