bundler: keep the CommonJS wrapper when a file gives module or exports its own value

src/js_parser/fold.rs

@@ -293,7 +293,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         }
                     }
 
-                    if !p.is_control_flow_dead && id.ref_.eql(p.module_ref) {
+                    if !p.is_control_flow_dead
+                        && id.ref_.eql(p.module_ref)
+                        && !p.has_user_declared_module_or_exports
+                    {
                         // Rewrite "module.require()" to "require()" for Webpack compatibility.
                         // See https://github.com/webpack/webpack/pull/7750 for more info.
                         // This also makes correctness a little easier.
@@ -461,7 +464,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
 
                     if p.should_unwrap_common_js_to_esm() {
                         if !p.is_control_flow_dead && id.ref_.eql(p.exports_ref) {
-                            if !p.commonjs_named_exports_deoptimized {
+                            if !p.commonjs_named_exports_deoptimized
+                                && !p.has_user_declared_module_or_exports
+                            {
                                 if identifier_opts.is_delete_target() {
                                     p.deoptimize_common_js_named_exports();
                                     return None;

src/js_parser/p.rs

@@ -307,6 +307,9 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> {
     /// A module-scope `var` has the name of a top-level function, which module code rejects.
     pub(crate) has_top_level_function_merged_with_var: bool,
 
+    /// A module-scope `var module = ...` / `var exports = ...` replaces the CommonJS wrapper value.
+    pub(crate) has_user_declared_module_or_exports: bool,
+
     pub(crate) is_file_considered_to_have_esm_exports: bool,
 
     pub(crate) has_called_runtime: bool,
@@ -9749,6 +9752,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             hoisted_ref_for_sloppy_mode_block_fn: Default::default(),
             has_with_scope: false,
             has_top_level_function_merged_with_var: false,
+            has_user_declared_module_or_exports: false,
             is_file_considered_to_have_esm_exports: false,
             has_called_runtime: false,
             symbol_uses,

src/js_parser/parse/parse_entry.rs

@@ -1610,6 +1610,7 @@ impl<'a> Parser<'a> {
             && !p.has_top_level_return
             && !p.has_with_scope
             && !p.has_top_level_function_merged_with_var
+            && !p.has_user_declared_module_or_exports
             && p.symbols.as_slice()[p.module_ref.inner_index() as usize].use_count_estimate == 1
             && p.symbols.as_slice()[p.exports_ref.inner_index() as usize].use_count_estimate == 0
         {

src/js_parser/parse/parse_stmt.rs

@@ -150,6 +150,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         p.lexer.next()?;
         let decls = p.parse_and_declare_decls(js_ast::symbol::Kind::Hoisted, opts)?;
         p.lexer.expect_or_insert_semicolon()?;
+        p.note_var_shadowing_module_or_exports(decls.slice());
         Ok(p.s(
             S::Local {
                 kind: js_ast::s::Kind::KVar,
@@ -161,6 +162,54 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         ))
     }
 
+    /// `var exports = {...}` / `var module = {...}` hoisted to the module scope gives
+    /// the wrapper name a value of its own. `var exports;` and
+    /// `var exports = module.exports;` keep the wrapper value, so they are not counted.
+    fn note_var_shadowing_module_or_exports(&mut self, decls: &[G::Decl]) {
+        for decl in decls {
+            let Some(value) = decl.value else {
+                continue;
+            };
+            let js_ast::b::B::BIdentifier(id) = decl.binding.data else {
+                continue;
+            };
+            let name = self.symbols[id.r#ref.inner_index() as usize]
+                .original_name
+                .slice();
+            if name != b"module" && name != b"exports" {
+                continue;
+            }
+            if self.is_module_dot_exports_at_parse(value) {
+                continue;
+            }
+            let mut scope = self.current_scope_ref();
+            while !scope.kind_stops_hoisting() {
+                scope = scope.parent.unwrap();
+            }
+            if scope == self.module_scope_ref() {
+                self.has_user_declared_module_or_exports = true;
+            }
+        }
+    }
+
+    /// `module.exports` or `module.exports = ...` before the visit pass resolves names.
+    fn is_module_dot_exports_at_parse(&self, mut expr: Expr) -> bool {
+        if let Some(bin) = expr.data.e_binary()
+            && bin.op == js_ast::OpCode::BinAssign
+        {
+            expr = bin.left;
+        }
+        let Some(dot) = expr.data.e_dot() else {
+            return false;
+        };
+        dot.name == b"exports"
+            && dot
+                .target
+                .data
+                .e_identifier()
+                .is_some_and(|id| self.load_name_from_ref(id.ref_) == b"module")
+    }
+
     #[inline]
     fn t_const(
         p: &mut Self,

test/bundler/bundler_cjs2esm.test.ts

@@ -2404,4 +2404,67 @@ describe("bundler", () => {
       stdout: "var\nmain",
     },
   });
+  // A top-level `var exports` / `var module` merges with the CommonJS wrapper
+  // parameter in Node. When it has a value of its own, the file must keep its
+  // wrapper, keep the declaration, and leave `exports.x` / `module.exports.x` as
+  // plain property accesses. `var exports;` and `var exports = module.exports;`
+  // keep the wrapper value and still lift to ESM.
+  itBundled("cjs2esm/VarWithTheNameOfModuleOrExportsKeepsWrapper", {
+    files: {
+      "/entry.js": /* js */ `
+        import lib from "./lib.cjs";
+        import fake from "./fake-module.cjs";
+        import reassigned from "./reassigned.cjs";
+        import uninit from "./uninit.cjs";
+        import nested from "./nested.cjs";
+        import { a as aliasedA } from "./aliased.cjs";
+        import { b as chainedB } from "./chained.cjs";
+        console.log(JSON.stringify({ lib, fake, reassigned, uninit, nested, aliasedA, chainedB }));
+      `,
+      "/aliased.cjs": /* js */ `
+        var exports = module.exports;
+        exports.a = "aliased";
+      `,
+      "/chained.cjs": /* js */ `
+        var exports = module.exports = {};
+        exports.b = "chained";
+      `,
+      "/lib.cjs": /* js */ `
+        var exports = { local: true };
+        module.exports.answer = 42;
+        module.exports.kind = typeof exports.local;
+      `,
+      "/fake-module.cjs": /* js */ `
+        var module = { exports: { fake: true }, id: "user" };
+        module.exports.x = 1;
+        exports.viaParam = typeof module.exports.fake;
+        exports.id = module.id;
+      `,
+      "/reassigned.cjs": /* js */ `
+        exports.before = 1;
+        var exports = "shadow";
+        module.exports.after = typeof exports;
+      `,
+      "/uninit.cjs": /* js */ `
+        var exports;
+        module.exports.foo = 123;
+        exports.bar = exports.foo + 1;
+      `,
+      "/nested.cjs": /* js */ `
+        if (true) {
+          var module = { exports: { nested: true } };
+        }
+        module.exports.x = 1;
+        exports.viaParam = typeof module.exports.nested;
+      `,
+    },
+    cjs2esm: {
+      // chained.cjs keeps its wrapper because of the `module.exports = {}` assignment, as before.
+      unhandled: ["/lib.cjs", "/fake-module.cjs", "/reassigned.cjs", "/nested.cjs", "/chained.cjs"],
+    },
+    run: {
+      stdout:
+        '{"lib":{"answer":42,"kind":"boolean"},"fake":{"viaParam":"boolean","id":"user"},"reassigned":{"before":1,"after":"string"},"uninit":{"foo":123,"bar":124},"nested":{"viaParam":"boolean"},"aliasedA":"aliased","chainedB":"chained"}',
+    },
+  });
 });