fix: hoisted top level scope declartion for wrapped esm (#387)

crates/rolldown/src/bundler/renderer/impl_visit.rs

@@ -8,7 +8,7 @@ use rolldown_utils::MagicStringExt;
 
 use crate::bundler::{module::Module, renderer::RenderControl};
 
-use super::AstRenderer;
+use super::{AstRenderer, RenderKind};
 
 impl<'ast, 'r> AstRenderer<'r> {
   fn visit_top_level_stmt(&mut self, stmt: &ast::Statement<'ast>) {
@@ -29,6 +29,11 @@ impl<'ast, 'r> AstRenderer<'r> {
       }
     }
     if self.current_stmt_info.get().is_included {
+      if matches!(self.kind, RenderKind::WrappedEsm) {
+        if let oxc::ast::ast::Statement::Declaration(decl) = stmt {
+          self.render_top_level_declaration_for_wrapped_esm(decl);
+        }
+      }
       self.visit_statement(stmt);
     } else {
       self.ctx.remove_stmt(stmt.span());
@@ -155,9 +160,9 @@ impl<'ast, 'r> Visit<'ast> for AstRenderer<'r> {
 
   fn visit_export_named_declaration(&mut self, decl: &oxc::ast::ast::ExportNamedDeclaration<'ast>) {
     let control = match &mut self.kind {
-      super::RenderKind::WrappedEsm => self.render_export_named_declaration_for_wrapped_esm(decl),
-      super::RenderKind::Cjs => RenderControl::Continue,
-      super::RenderKind::Esm => self.render_export_named_declaration_for_esm(decl),
+      RenderKind::WrappedEsm => self.render_export_named_declaration_for_wrapped_esm(decl),
+      RenderKind::Cjs => RenderControl::Continue,
+      RenderKind::Esm => self.render_export_named_declaration_for_esm(decl),
     };
 
     if control.is_skip() {
@@ -174,8 +179,8 @@ impl<'ast, 'r> Visit<'ast> for AstRenderer<'r> {
     decl: &oxc::ast::ast::ExportDefaultDeclaration<'ast>,
   ) {
     let control = match &mut self.kind {
-      super::RenderKind::WrappedEsm => self.render_export_default_declaration_for_wrapped_esm(decl),
-      super::RenderKind::Cjs | super::RenderKind::Esm => self.strip_export_keyword(decl),
+      RenderKind::WrappedEsm => self.render_export_default_declaration_for_wrapped_esm(decl),
+      RenderKind::Cjs | RenderKind::Esm => self.strip_export_keyword(decl),
     };
 
     if control.is_skip() {

crates/rolldown/src/bundler/renderer/render_wrapped_esm.rs

@@ -3,7 +3,7 @@ use crate::bundler::module::Module;
 use super::{AstRenderer, RenderControl};
 use oxc::{
   ast::ast::Declaration,
-  span::{GetSpan, Span},
+  span::{Atom, GetSpan, Span},
 };
 use rolldown_common::ExportsKind;
 use rolldown_oxc::BindingIdentifierExt;
@@ -25,25 +25,16 @@ impl<'r> AstRenderer<'r> {
       }
       oxc::ast::ast::ExportDefaultDeclarationKind::FunctionDeclaration(func) => {
         self.ctx.remove_node(Span::new(decl.span.start, func.span.start));
-        if let Some(ident) = &func.id {
-          self.render_binding_identifier(ident);
-        } else {
+        if func.id.is_none() {
           let default_symbol_name = self.ctx.default_ref_name.unwrap();
           self.ctx.source.append_right(func.params.span.start, format!(" {default_symbol_name}"));
         }
-        self.wrapped_esm_ctx.hoisted_functions.push(func.span);
+        self.hoisted_function_declaration(func);
       }
       oxc::ast::ast::ExportDefaultDeclarationKind::ClassDeclaration(class) => {
+        self.ctx.remove_node(Span::new(decl.span.start, class.span.start));
         let default_symbol_name = self.ctx.default_ref_name.unwrap();
-        self.wrapped_esm_ctx.hoisted_vars.push(default_symbol_name.clone());
-        self.ctx.source.overwrite(
-          decl.span.start,
-          class.span.start,
-          format!("{default_symbol_name} = "),
-        );
-        // avoid syntax error
-        // export default class Foo {} Foo.prop = 123 => var Foo = class Foo {} \n Foo.prop = 123
-        self.ctx.source.append_right(class.span.end, "\n");
+        self.hoisted_class_declaration(class, default_symbol_name);
       }
       _ => {}
     }
@@ -57,46 +48,18 @@ impl<'r> AstRenderer<'r> {
     if let Some(decl) = &named_decl.declaration {
       match decl {
         Declaration::VariableDeclaration(var_decl) => {
-          let names = var_decl
-            .declarations
-            .iter()
-            .map(|decl| match &decl.id.kind {
-              oxc::ast::ast::BindingPatternKind::BindingIdentifier(id) => self
-                .ctx
-                .canonical_name_for((self.ctx.module.id, id.symbol_id.get().unwrap()).into()),
-              _ => unimplemented!(),
-            })
-            .cloned();
-          self.wrapped_esm_ctx.hoisted_vars.extend(names);
-          self
-            .ctx
-            .remove_node(Span::new(named_decl.span.start, var_decl.declarations[0].span.start));
+          self.hoisted_variable_declaration(&var_decl.declarations, named_decl.span.start);
         }
         Declaration::FunctionDeclaration(func) => {
           self.ctx.remove_node(Span::new(named_decl.span.start, func.span.start));
-          let id = func.id.as_ref().unwrap();
-          let name =
-            self.ctx.canonical_name_for((self.ctx.module.id, id.expect_symbol_id()).into());
-          if id.name != name {
-            self.ctx.source.overwrite(id.span.start, id.span.end, name.to_string());
-          }
-          self.wrapped_esm_ctx.hoisted_functions.push(func.span);
+          self.hoisted_function_declaration(func);
         }
         Declaration::ClassDeclaration(class) => {
+          self.ctx.remove_node(Span::new(named_decl.span.start, class.span.start));
           let id = class.id.as_ref().unwrap();
-          if let Some(name) =
-            self.ctx.need_to_rename((self.ctx.module.id, id.expect_symbol_id()).into())
-          {
-            self.wrapped_esm_ctx.hoisted_vars.push(name.clone());
-            self.ctx.source.overwrite(
-              named_decl.span.start,
-              class.span.start,
-              format!("{name} = "),
-            );
-            // avoid syntax error
-            // export class Foo {} Foo.prop = 123 => var Foo = class Foo {} \n Foo.prop = 123
-            self.ctx.source.append_right(class.span.end, "\n");
-          }
+          let name =
+            self.ctx.canonical_name_for((self.ctx.module.id, id.expect_symbol_id()).into());
+          self.hoisted_class_declaration(class, name);
         }
         _ => {}
       }
@@ -122,4 +85,57 @@ impl<'r> AstRenderer<'r> {
     }
     RenderControl::Skip
   }
+
+  pub fn render_top_level_declaration_for_wrapped_esm(
+    &mut self,
+    decl: &oxc::ast::ast::Declaration,
+  ) {
+    match &decl {
+      oxc::ast::ast::Declaration::VariableDeclaration(var_decl) => {
+        self.hoisted_variable_declaration(&var_decl.declarations, var_decl.span.start);
+      }
+      oxc::ast::ast::Declaration::FunctionDeclaration(func) => {
+        self.hoisted_function_declaration(func);
+      }
+      oxc::ast::ast::Declaration::ClassDeclaration(class) => {
+        if let Some(id) = &class.id {
+          let name =
+            self.ctx.canonical_name_for((self.ctx.module.id, id.expect_symbol_id()).into());
+          self.hoisted_class_declaration(class, name);
+        }
+      }
+      _ => {}
+    }
+  }
+
+  fn hoisted_variable_declaration<'a>(
+    &mut self,
+    declarations: &oxc::allocator::Vec<'a, oxc::ast::ast::VariableDeclarator<'a>>,
+    decl_start: u32,
+  ) {
+    let names = declarations
+      .iter()
+      .map(|decl| match &decl.id.kind {
+        oxc::ast::ast::BindingPatternKind::BindingIdentifier(id) => {
+          self.ctx.canonical_name_for((self.ctx.module.id, id.symbol_id.get().unwrap()).into())
+        }
+        _ => unimplemented!(),
+      })
+      .cloned();
+    self.wrapped_esm_ctx.hoisted_vars.extend(names);
+    self.ctx.remove_node(Span::new(decl_start, declarations[0].span.start));
+  }
+
+  fn hoisted_function_declaration(&mut self, func: &oxc::ast::ast::Function) {
+    // binding_identifier will rename at visit children
+    self.wrapped_esm_ctx.hoisted_functions.push(func.span);
+  }
+
+  fn hoisted_class_declaration(&mut self, class: &oxc::ast::ast::Class, name: &Atom) {
+    self.wrapped_esm_ctx.hoisted_vars.push(name.clone());
+    self.ctx.source.append_left(class.span.start, format!("{name} = "));
+    // avoid syntax error
+    // export class Foo {} Foo.prop = 123 => var Foo = class Foo {} \n Foo.prop = 123
+    self.ctx.source.append_right(class.span.end, "\n");
+  }
 }

crates/rolldown/src/bundler/renderer/utils.rs

@@ -12,7 +12,7 @@ impl<'r> AstRenderContext<'r> {
     self.graph.symbols.canonical_name_for(symbol, self.canonical_names)
   }
 
-  pub fn need_to_rename(&self, symbol: SymbolRef) -> Option<&Atom> {
+  pub fn _need_to_rename(&self, symbol: SymbolRef) -> Option<&Atom> {
     let canonical_ref = self.graph.symbols.par_canonical_ref_for(symbol);
     self.canonical_names.get(&canonical_ref)
   }

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

@@ -67,7 +67,7 @@ var init_commonjs = __esm({
 		c = 234
 
 
-		Class = class Class {}
+Class = 		class Class {}
 
 
 
@@ -80,7 +80,7 @@ var c_ns = {
 };
 var init_c = __esm({
 	'c.js'() {
-		c_default = class {}
+c_default = 		class {}
 
 	}
 });
@@ -91,7 +91,7 @@ var d_ns = {
 };
 var init_d = __esm({
 	'd.js'() {
-		Foo = class Foo {}
+Foo = 		class Foo {}
  Foo.prop = 123
 	}
 });

crates/rolldown/tests/fixtures/basic_commonjs/artifacts.snap

@@ -44,21 +44,29 @@ var __toCommonJS = mod => __copyProps(__defProp({}, '__esModule', { value: true
 // esm.js
 function esm_default_fn() {}
 function esm_named_fn() {}
-var esm_named_var,esm_named_class;
+function hoisted_fn() {
+    const bar = 1 // shouldn't hoisted
+}
+var esm_named_var,esm_named_class,hoisted_var,hoisted_class;
 var esm_ns = {
-	get default() { return esm_default_fn },
-	get esm_named_class() { return esm_named_class },
-	get esm_named_fn() { return esm_named_fn },
-	get esm_named_var() { return esm_named_var }
+    get default() { return esm_default_fn },
+    get esm_named_class() { return esm_named_class },
+    get esm_named_fn() { return esm_named_fn },
+    get esm_named_var() { return esm_named_var }
 };
 var init_esm = __esm({
-	'esm.js'() {
+    'esm.js'() {
 
-		esm_named_var = 1;
+        esm_named_var = 1;
 
-		esm_named_class = class esm_named_class {}
+esm_named_class =         class esm_named_class {}
 
-	}
+
+        hoisted_var = 1;
+
+hoisted_class =         class hoisted_class {}
+
+    }
 });
 // commonjs.js
 var require_commonjs$1 = __commonJS({

crates/rolldown/tests/fixtures/basic_commonjs/esm.js

@@ -1,4 +1,10 @@
 export default function esm_default_fn() {}
 export const esm_named_var = 1;
 export function esm_named_fn() {}
-export class esm_named_class {}
+export class esm_named_class {}
+
+const hoisted_var = 1;
+function hoisted_fn() {
+    const bar = 1 // shouldn't hoisted
+}
+class hoisted_class {}

crates/rolldown/tests/fixtures/basic_commonjs/test.config.json

@@ -1 +1,5 @@
-{}
+{
+    "input": {
+        "treeshake": false
+    }
+}

crates/rolldown/tests/fixtures/deconflict/wrapped_esm_default_function/artifacts.snap

@@ -34,12 +34,13 @@ var __toCommonJS = mod => __copyProps(__defProp({}, '__esModule', { value: true
 function foo$1(a$1$1) {
     console.log(a$1$1, a$1)
 }
-        var foo_ns = {
+var a$1;
+var foo_ns = {
     get default() { return foo$1 }
 };
 var init_foo = __esm({
     'foo.js'() {
-const a$1 = 1;
+        a$1 = 1;
 
 
     }

crates/rolldown/tests/fixtures/deconflict/wrapped_esm_export_named_function/artifacts.snap

@@ -34,12 +34,13 @@ var __toCommonJS = mod => __copyProps(__defProp({}, '__esModule', { value: true
 function foo$1(a$1$1) {
     console.log(a$1$1, a$1)
 }
-        var foo_ns = {
+var a$1;
+var foo_ns = {
     get foo() { return foo$1 }
 };
 var init_foo = __esm({
     'foo.js'() {
-const a$1 = 1;
+        a$1 = 1;
 
 
     }
@@ -53,7 +54,7 @@ var bar_default = { foo: foo$1 }
 
 
 
- // make foo `a` conflict
+const a = 2; // make foo `a` conflict
 
 const { foo } = bar_default
 

crates/rolldown/tests/fixtures/deconflict/wrapped_esm_export_named_function/test.config.json

@@ -2,6 +2,7 @@
     "input": {
         "external": [
             "assert"
-        ]
+        ],
+        "treeshake": false
     }
 }