fix(splitting): ensure correct __export symbol reference (rolldown#3289)

### Description close rolldown#3288
Adr1anN-dev · Jan 3, 2025 · 31cc7b8 · 31cc7b8
1 parent 82b4e30
commit 31cc7b8
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 18 deletions.
diff --git a/crates/rolldown/src/module_finalizers/scope_hoisting/mod.rs b/crates/rolldown/src/module_finalizers/scope_hoisting/mod.rs
@@ -65,6 +65,10 @@ impl<'me, 'ast> ScopeHoistingFinalizer<'me, 'ast> {
     self.ctx.runtime.resolve_symbol(name)
   }
 
+  pub fn finalized_expr_for_runtime_symbol(&self, name: &str) -> ast::Expression<'ast> {
+    self.finalized_expr_for_symbol_ref(self.ctx.runtime.resolve_symbol(name), false)
+  }
+
   /// If return true the import stmt should be removed,
   /// or transform the import stmt to target form.
   fn transform_or_remove_import_export_stmt(
@@ -85,8 +89,7 @@ impl<'me, 'ast> ScopeHoistingFinalizer<'me, 'ast> {
         // Replace the statement with something like `var import_foo = __toESM(require_foo())`
 
         // `__toESM`
-        let to_esm_fn_name =
-          self.finalized_expr_for_symbol_ref(self.canonical_ref_for_runtime("__toESM"), false);
+        let to_esm_fn_name = self.finalized_expr_for_runtime_symbol("__toESM");
 
         // `require_foo`
         let importee_wrapper_ref_name =
@@ -353,18 +356,17 @@ impl<'me, 'ast> ScopeHoistingFinalizer<'me, 'ast> {
     });
 
     // construct `__export(ns_name, { prop_name: () => returned, ... })`
-    let mut export_call_expr = self.snippet.call_expr(self.canonical_name_for_runtime("__export"));
-    export_call_expr.arguments.push(ast::Argument::from(self.snippet.id_ref_expr(var_name, SPAN)));
-    export_call_expr
-      .arguments
-      .push(ast::Argument::ObjectExpression(arg_obj_expr.into_in(self.alloc)));
-    let export_call_stmt = ast::Statement::ExpressionStatement(
-      ast::ExpressionStatement {
-        expression: ast::Expression::CallExpression(export_call_expr.into_in(self.alloc)),
-        ..TakeIn::dummy(self.alloc)
-      }
-      .into_in(self.alloc),
+    let export_call_expr = self.snippet.builder.expression_call(
+      SPAN,
+      self.finalized_expr_for_runtime_symbol("__export"),
+      NONE,
+      self.snippet.builder.vec_from_array([
+        ast::Argument::from(self.snippet.id_ref_expr(var_name, SPAN)),
+        ast::Argument::ObjectExpression(arg_obj_expr.into_in(self.alloc)),
+      ]),
+      false,
     );
+    let export_call_stmt = self.snippet.builder.statement_expression(SPAN, export_call_expr);
     let mut ret = vec![decl_stmt, export_call_stmt];
     ret.extend(re_export_external_stmts.unwrap_or_default());
 
@@ -618,7 +620,7 @@ impl<'me, 'ast> ScopeHoistingFinalizer<'me, 'ast> {
         // use `__require` instead of `require`
         if rec.meta.contains(ImportRecordMeta::CALL_RUNTIME_REQUIRE) {
           *call_expr.callee.get_inner_expression_mut() =
-            self.finalized_expr_for_symbol_ref(self.canonical_ref_for_runtime("__require"), false);
+            self.finalized_expr_for_runtime_symbol("__require");
         }
         let rewrite_ast = match &self.ctx.modules[rec.resolved_module] {
           Module::Normal(importee) => {
@@ -685,11 +687,8 @@ impl<'me, 'ast> ScopeHoistingFinalizer<'me, 'ast> {
                     // `xxx_exports`
                     let namespace_object_ref_expr =
                       self.finalized_expr_for_symbol_ref(importee.namespace_object_ref, false);
-                    let to_commonjs_ref = self.canonical_ref_for_runtime("__toCommonJS");
                     // `__toCommonJS`
-                    let to_commonjs_expr =
-                      self.finalized_expr_for_symbol_ref(to_commonjs_ref, false);
-
+                    let to_commonjs_expr = self.finalized_expr_for_runtime_symbol("__toCommonJS");
                     // `init_xxx()`
                     let wrap_ref_call_expr =
                       ast::Expression::CallExpression(self.snippet.builder.alloc_call_expression(

diff --git a/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/_config.json b/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/_config.json
@@ -0,0 +1,18 @@
+{
+  "config": {
+    "input": [
+      {
+        "name": "main1",
+        "import": "main1.js"
+      },
+      {
+        "name": "main2",
+        "import": "main2.js"
+      }
+    ],
+    "external": [
+      "node:assert"
+    ],
+    "format":"cjs"
+  }
+}
diff --git a/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/artifacts.snap b/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/artifacts.snap
@@ -0,0 +1,57 @@
+---
+source: crates/rolldown_testing/src/integration_test.rs
+snapshot_kind: text
+---
+# Assets
+
+## chunk.js
+
+```js
+"use strict";
+
+
+Object.defineProperty(exports, '__esm', {
+  enumerable: true,
+  get: function () {
+    return __esm;
+  }
+});
+Object.defineProperty(exports, '__export', {
+  enumerable: true,
+  get: function () {
+    return __export;
+  }
+});
+Object.defineProperty(exports, '__toESM', {
+  enumerable: true,
+  get: function () {
+    return __toESM;
+  }
+});
+```
+## main1.js
+
+```js
+const require_chunk = require('./chunk.js');
+require("node:assert");
+
+```
+## main2.js
+
+```js
+const require_chunk = require('./chunk.js');
+
+//#region esm.js
+var esm_exports = {};
+require_chunk.__export(esm_exports, { share: () => share });
+function share() {
+	return 1;
+}
+var init_esm = require_chunk.__esm({ "esm.js"() {} });
+
+//#endregion
+//#region main2.js
+init_esm();
+
+//#endregion
+```
diff --git a/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/esm.js b/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/esm.js
@@ -0,0 +1,3 @@
+export function share() {
+    return 1
+}
diff --git a/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/main1.js b/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/main1.js
@@ -0,0 +1 @@
+import assert from 'node:assert'
diff --git a/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/main2.js b/crates/rolldown/tests/rolldown/code_splitting/format_cjs_with_esm_require_esm/main2.js
@@ -0,0 +1 @@
+require('./esm');
diff --git a/crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap b/crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap
@@ -3800,6 +3800,12 @@ snapshot_kind: text
 - b-!~{001}~.js => b-C3H3Nr5B.js
 - shared-!~{002}~.js => shared-xwhduHDy.js
 
+# tests/rolldown/code_splitting/format_cjs_with_esm_require_esm
+
+- main1-!~{000}~.js => main1-BAlyLRtA.js
+- main2-!~{001}~.js => main2-CfVwQve5.js
+- chunk-!~{002}~.js => chunk-m_k7nO7a.js
+
 # tests/rolldown/code_splitting/format_cjs_with_module_cjs
 
 - main1-!~{000}~.js => main1-DfLRKT4o.js