diff --git a/src/js_parser/fold.rs b/src/js_parser/fold.rs index 1e01b7280c76..c13ba1a874ba 100644 --- a/src/js_parser/fold.rs +++ b/src/js_parser/fold.rs @@ -313,7 +313,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O 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 + && !p.has_user_declared_module + // 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 +479,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; diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index d981eb71862d..fbba37810a61 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -302,6 +302,13 @@ 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, + /// A module-scope `var exports = module.exports`, which keeps the wrapper value if `module` is the wrapper's. + pub(crate) has_var_exports_from_module_exports: bool, + pub(crate) is_file_considered_to_have_esm_exports: bool, pub(crate) has_called_runtime: bool, @@ -3343,6 +3350,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"exports")?; self.module_ref = self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"module")?; + if self.has_var_exports_from_module_exports + && self + .module_scope() + .get_member_with_hash(b"module", Scope::get_member_hash(b"module")) + .is_some_and(|member| !member.ref_.eql(self.module_ref)) + { + self.has_user_declared_exports = true; + } self.require_ref = self.declare_common_js_symbol(js_ast::symbol::Kind::Unbound, b"require")?; @@ -9732,6 +9747,9 @@ 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, + has_var_exports_from_module_exports: false, is_file_considered_to_have_esm_exports: false, has_called_runtime: false, symbol_uses, diff --git a/src/js_parser/parse/parse_entry.rs b/src/js_parser/parse/parse_entry.rs index d1cf892e5f49..5068fcebfdc3 100644 --- a/src/js_parser/parse/parse_entry.rs +++ b/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 { diff --git a/src/js_parser/parse/parse_stmt.rs b/src/js_parser/parse/parse_stmt.rs index 856983870baf..ad8bdfce98cf 100644 --- a/src/js_parser/parse/parse_stmt.rs +++ b/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,101 @@ 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)) + { + self.has_var_exports_from_module_exports |= + self.var_hoists_to_module_scope(); + 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 +756,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 +778,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 +796,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 { diff --git a/src/js_parser/visit/visit_expr.rs b/src/js_parser/visit/visit_expr.rs index 61e97b7a79a0..9cb3ef304581 100644 --- a/src/js_parser/visit/visit_expr.rs +++ b/src/js_parser/visit/visit_expr.rs @@ -258,9 +258,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O ), ), } - } else if p.exports_ref.eql(e_.ref_) { - // Assigning to `exports` in a CommonJS module must be tracked to undo the - // `module.exports` -> `exports` optimization. + } else if p.exports_ref.eql(e_.ref_) || p.module_ref.eql(e_.ref_) { + // An assignment to `exports` or `module` undoes the `module.exports` -> `exports` rewrite. p.commonjs_module_exports_assigned_deoptimized = true; } diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 57e31a43c888..d76cb80f2bd3 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -2620,4 +2620,228 @@ 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. A default import used as a + // whole value keeps the wrapper of any CommonJS file, so the files that are + // read through named imports are the ones that show which file is lifted. + 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"; + import { lost as namedExportsLost, kept as namedExportsKept } from "./named-exports.cjs"; + import { leak as namedModuleLeak, real as namedModuleReal } from "./named-module.cjs"; + import { kind as forInModuleKind } from "./for-in-module.cjs"; + import { foo as blockShadowFoo } from "./block-shadow.cjs"; + import { foo as functionModuleFoo } from "./function-module.cjs"; + import { leak as bareModuleLeak, real as bareModuleReal } from "./bare-module.cjs"; + console.log( + JSON.stringify({ + lib, + fake, + reassigned, + uninit: { foo: uninitFoo, bar: uninitBar }, + nested, + aliasedA, + chainedB, + destructured, + forHeader, + forOf, + moduleRequire, + ambientOk, + namedExports: [typeof namedExportsLost, namedExportsKept], + namedModule: [typeof namedModuleLeak, namedModuleReal], + forInModuleKind, + blockShadowFoo: typeof blockShadowFoo, + functionModuleFoo: typeof functionModuleFoo, + bareModule: [typeof bareModuleLeak, bareModuleReal], + }), + ); + `, + "/named-exports.cjs": /* js */ ` + var exports = {}; + exports.lost = 1; + module.exports.kept = 2; + `, + "/named-module.cjs": /* js */ ` + var module = { exports: {} }; + module.exports.leak = 1; + exports.real = 2; + `, + "/for-in-module.cjs": /* js */ ` + for (var [module] in { m: 1 }) {} + exports.kind = typeof module.exports; + `, + "/block-shadow.cjs": /* js */ ` + { + let module = { exports: { fake: true } }; + var exports = module.exports; + } + exports.foo = 1; + `, + "/function-module.cjs": /* js */ ` + function module() {} + module.exports = { fake: true }; + var exports = module.exports; + exports.foo = 1; + `, + "/bare-module.cjs": /* js */ ` + module = { exports: { fake: true } }; + module.exports.leak = 1; + exports.real = 2; + `, + "/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 } }; + module.exports.x = 1; + exports.viaParam = typeof module.exports.fake; + `, + "/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", + "/named-exports.cjs", + "/named-module.cjs", + "/for-in-module.cjs", + "/block-shadow.cjs", + "/function-module.cjs", + "/bare-module.cjs", + ], + }, + run: { + stdout: + '{"lib":{"answer":42,"kind":"boolean"},"fake":{"viaParam":"boolean"},"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,"namedExports":["undefined",2],"namedModule":["undefined",2],"forInModuleKind":"undefined","blockShadowFoo":"undefined","functionModuleFoo":"undefined","bareModule":["undefined",2]}', + }, + }); + // In an unwrapped package `module.exports = require()` becomes `export * from`. + // Not beside a `module` of its own: react assigns to that object and exports + // nothing. Not beside an `exports` of its own either: in a lifted file that + // `var` is the namespace object, and its initializer would replace it. + itBundled("cjs2esm/ReactSpecificUnwrappingWithOwnVarModuleOrExportsKeepsWrapper", { + files: { + "/entry.js": /* js */ ` + import { value as ownModule } from "react"; + import * as ownExports from "react-is"; + const key = "value"; + console.log(typeof ownModule, ownExports[key]); + `, + "/node_modules/react/index.js": /* js */ ` + var module = { exports: {} }; + module.exports = require('./main'); + `, + "/node_modules/react/main.js": /* js */ ` + exports.value = "react"; + `, + "/node_modules/react-is/index.js": /* js */ ` + var exports = { local: true }; + module.exports = require('./main'); + `, + "/node_modules/react-is/main.js": /* js */ ` + exports.value = "react-is"; + `, + }, + cjs2esm: { unhandled: ["/node_modules/react/index.js", "/node_modules/react-is/index.js"] }, + run: { + stdout: "undefined react-is", + }, + }); + // After `var module = module || {}` or a conditional `var module`, `module` is + // still the wrapper's object at run time. In a bundle that object has no + // `require`, so these rewrites stay beside any `var module`. + itBundled("cjs2esm/VarModuleShimKeepsModuleRequireAndRequireMain", { + files: { + "/entry.cjs": /* js */ ` + var module = module || {}; + console.log(require.main === module, module.require("./conditional.cjs").value); + `, + "/conditional.cjs": /* js */ ` + if (typeof module === "undefined") { + var module = { exports: {} }; + } + module.exports.value = module.require("./dep.cjs").value; + `, + "/dep.cjs": /* js */ ` + exports.value = "dep"; + `, + }, + run: { stdout: "true dep" }, + }); });