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

src/js_parser/fold.rs

@@ -306,14 +306,21 @@ 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
+                    {
                         // 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.
                         if identifier_opts.is_call_target() && name == b"require" {
                             p.ignore_usage(p.module_ref);
                             return Some(p.value_for_require(name_loc));
-                        } else if !p.commonjs_named_exports_deoptimized && name == b"exports" {
+                        } else if !p.commonjs_named_exports_deoptimized
+                            // The rewrite prints through `exports_ref`, which is the user's binding here.
+                            && !p.has_user_declared_exports
+                            && name == b"exports"
+                        {
                             if identifier_opts.assign_target() != js_ast::AssignTarget::None {
                                 p.commonjs_module_exports_assigned_deoptimized = true;
                             }
@@ -474,7 +481,10 @@ 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
+                                && !p.has_user_declared_exports
+                            {
                                 if identifier_opts.is_delete_target() {
                                     p.deoptimize_common_js_named_exports();
                                     return None;

src/js_parser/p.rs

@@ -302,6 +302,11 @@ 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 = ...` replaces the CommonJS wrapper value.
+    pub(crate) has_user_declared_module: bool,
+    /// A module-scope `var exports = ...` replaces the CommonJS wrapper value.
+    pub(crate) has_user_declared_exports: bool,
+
     pub(crate) is_file_considered_to_have_esm_exports: bool,
 
     pub(crate) has_called_runtime: bool,
@@ -9732,6 +9737,8 @@ 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: false,
+            has_user_declared_exports: false,
             is_file_considered_to_have_esm_exports: false,
             has_called_runtime: false,
             symbol_uses,

src/js_parser/parse/parse_entry.rs

@@ -1740,6 +1740,8 @@ 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
+            && !p.has_user_declared_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,9 @@ 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()?;
+        if !opts.is_typescript_declare {
+            p.note_var_shadowing_module_or_exports(decls.slice(), false);
+        }
         Ok(p.s(
             S::Local {
                 kind: js_ast::s::Kind::KVar,
@@ -161,6 +164,99 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         ))
     }
 
+    /// `var exports;` and `var exports = module.exports;` keep the wrapper value and do not count.
+    fn note_var_shadowing_module_or_exports(&mut self, decls: &[G::Decl], is_loop_target: bool) {
+        for decl in decls {
+            if decl.value.is_none() && !is_loop_target {
+                continue;
+            }
+            match decl.binding.data {
+                js_ast::b::B::BIdentifier(id) => {
+                    let name = self.load_name_from_ref(id.r#ref);
+                    if name != b"module" && name != b"exports" {
+                        continue;
+                    }
+                    if name == b"exports"
+                        && !is_loop_target
+                        && decl
+                            .value
+                            .is_some_and(|value| self.is_module_dot_exports_at_parse(value))
+                    {
+                        continue;
+                    }
+                    if !self.var_hoists_to_module_scope() {
+                        return;
+                    }
+                    if name == b"module" {
+                        self.has_user_declared_module = true;
+                    } else {
+                        self.has_user_declared_exports = true;
+                    }
+                }
+                js_ast::b::B::BArray(_) | js_ast::b::B::BObject(_) => {
+                    let (module, exports) = self.binding_names_module_or_exports(&decl.binding);
+                    if (module || exports) && self.var_hoists_to_module_scope() {
+                        self.has_user_declared_module |= module;
+                        self.has_user_declared_exports |= exports;
+                    }
+                }
+                js_ast::b::B::BMissing(_) => {}
+            }
+        }
+    }
+
+    /// (`module`, `exports`) found anywhere in a destructuring pattern.
+    fn binding_names_module_or_exports(&self, binding: &js_ast::Binding) -> (bool, bool) {
+        match binding.data {
+            js_ast::b::B::BIdentifier(id) => {
+                let name = self.load_name_from_ref(id.r#ref);
+                (name == b"module", name == b"exports")
+            }
+            js_ast::b::B::BArray(array) => {
+                array.items().iter().fold((false, false), |acc, item| {
+                    let (m, e) = self.binding_names_module_or_exports(&item.binding);
+                    (acc.0 || m, acc.1 || e)
+                })
+            }
+            js_ast::b::B::BObject(object) => {
+                object
+                    .properties()
+                    .iter()
+                    .fold((false, false), |acc, prop| {
+                        let (m, e) = self.binding_names_module_or_exports(&prop.value);
+                        (acc.0 || m, acc.1 || e)
+                    })
+            }
+            js_ast::b::B::BMissing(_) => (false, false),
+        }
+    }
+
+    fn var_hoists_to_module_scope(&self) -> bool {
+        let mut scope = self.current_scope_ref();
+        while !scope.kind_stops_hoisting() {
+            scope = scope.parent.unwrap();
+        }
+        scope == self.module_scope_ref()
+    }
+
+    /// `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,
@@ -658,6 +754,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 }
 
                 p.forbid_initializers(decls_ptr.slice(), "of", false)?;
+                if is_var {
+                    p.note_var_shadowing_module_or_exports(decls_ptr.slice(), true);
+                }
                 p.lexer.next()?;
                 let value = p.parse_expr(Level::Comma)?;
                 p.lexer.expect(T::TCloseParen)?;
@@ -677,6 +776,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             // Detect for-in loops
             if p.lexer.token == T::TIn {
                 p.forbid_initializers(decls_ptr.slice(), "in", is_var)?;
+                if is_var {
+                    p.note_var_shadowing_module_or_exports(decls_ptr.slice(), true);
+                }
                 p.lexer.next()?;
                 let value = p.parse_expr(Level::Lowest)?;
                 p.lexer.expect(T::TCloseParen)?;
@@ -692,6 +794,10 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 ));
             }
 
+            if is_var {
+                p.note_var_shadowing_module_or_exports(decls_ptr.slice(), false);
+            }
+
             // Only require "const" statement initializers when we know we're a normal for loop
             if let Some(init_stmt) = &init_ {
                 match &init_stmt.data {

test/bundler/bundler_cjs2esm.test.ts

@@ -2620,4 +2620,126 @@ 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. Those files are read through
+  // named imports: a default import used as a whole value keeps the wrapper of
+  // any CommonJS file.
+  itBundled("cjs2esm/VarWithTheNameOfModuleOrExportsKeepsWrapper", {
+    files: {
+      "/entry.js": /* js */ `
+        import lib from "./lib.cjs";
+        import fake from "./fake-module.cjs";
+        import reassigned from "./reassigned.cjs";
+        import { foo as uninitFoo, bar as uninitBar } from "./uninit.cjs";
+        import nested from "./nested.cjs";
+        import { a as aliasedA } from "./aliased.cjs";
+        import { b as chainedB } from "./chained.cjs";
+        import destructured from "./destructured.cjs";
+        import forHeader from "./for-header.cjs";
+        import forOf from "./for-of.cjs";
+        import moduleRequire from "./module-require.cjs";
+        import { ok as ambientOk } from "./ambient.cts";
+        console.log(
+          JSON.stringify({
+            lib,
+            fake,
+            reassigned,
+            uninit: { foo: uninitFoo, bar: uninitBar },
+            nested,
+            aliasedA,
+            chainedB,
+            destructured,
+            forHeader,
+            forOf,
+            moduleRequire,
+            ambientOk,
+          }),
+        );
+      `,
+      "/destructured.cjs": /* js */ `
+        var { exports } = { exports: { local: true } };
+        module.exports.kind = typeof exports.local;
+      `,
+      "/for-header.cjs": /* js */ `
+        for (var exports = { local: true }; false; ) {}
+        module.exports.kind = typeof exports.local;
+      `,
+      "/for-of.cjs": /* js */ `
+        for (var exports of [{ local: true }]) {}
+        for (var [module] in { m: 1 }) {}
+        exports.kind = typeof exports.local + module;
+      `,
+      "/module-require.cjs": /* js */ `
+        var exports = { local: true };
+        module.exports.dep = module.require("./dep.cjs").value;
+        module.exports.kind = typeof exports.local;
+      `,
+      "/dep.cjs": /* js */ `
+        exports.value = "dep";
+      `,
+      "/ambient.cts": /* ts */ `
+        declare var exports = {};
+        module.exports.ok = 1;
+      `,
+      "/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",
+        "/destructured.cjs",
+        "/for-header.cjs",
+        "/for-of.cjs",
+        "/module-require.cjs",
+        // dep.cjs is loaded through require(), which always keeps the wrapper.
+        "/dep.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","destructured":{"kind":"boolean"},"forHeader":{"kind":"boolean"},"forOf":{},"moduleRequire":{"dep":"dep","kind":"boolean"},"ambientOk":1}',
+    },
+  });
 });