From a3c6633562b548dd647a0262ee46e6c7a542746b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 7 Sep 2026 05:53:53 +0000 Subject: [PATCH 1/7] bundler: give `import *` of a lifted CommonJS module its own namespace object The linker used `exports_foo` of a lifted CommonJS module as both its `module.exports` (what a default import binds) and its `import *` namespace. So `ns === d`, `ns.default` was the lifted `exports.default` instead of `module.exports`, and `Object.keys(ns)` had no `default`. `exports_foo` stays the `module.exports` object. `import * as ns` now binds to `var import_foo = __toESM(exports_foo, 1)`, declared in a part of its own that prints with the namespace export part and is dropped unless the namespace is used as a value. `ns.default` and `ns.x` still bind to `exports_foo` and the lifted bindings directly. A `require()` that `unwrap_commonjs_to_esm` turned into an import star keeps binding `module.exports`. A star import from an importer that is not an ES module by type keeps the CommonJS wrapper of a module that sets both `exports.__esModule` and `exports.default`, as a default import does. --- docs/bundler/index.mdx | 4 +- src/bundler/LinkerContext.rs | 250 +++++++++++++----- src/bundler/LinkerGraph.rs | 24 +- src/bundler/linker_context/doStep5.rs | 4 +- .../findAllImportedPartsInJSOrder.rs | 5 + .../generateCodeForFileInChunkJS.rs | 32 +++ .../linker_context/scanImportsAndExports.rs | 42 ++- test/bundler/bundler_cjs.test.ts | 17 +- test/bundler/bundler_cjs2esm.test.ts | 209 ++++++++++++--- test/bundler/esbuild/dce.test.ts | 2 +- test/bundler/esbuild/importstar.test.ts | 2 +- test/bundler/esbuild/importstar_ts.test.ts | 2 +- 12 files changed, 467 insertions(+), 126 deletions(-) diff --git a/docs/bundler/index.mdx b/docs/bundler/index.mdx index c8a89aed92f6..eebf767317b8 100644 --- a/docs/bundler/index.mdx +++ b/docs/bundler/index.mdx @@ -1415,11 +1415,11 @@ In each case the member access compiles to a direct reference to `object`, the ` Assignments (`z.x = 1`), optional chains, and non-literal computed keys (`z[key]`) are left as property accesses; `z["object"]` is treated like `z.object`. `export default someImport` is followed only when it ends at a namespace; a default that snapshots a `let` export keeps snapshot semantics. -The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports`, which is that module's namespace, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The namespace object is created only when `React` itself is used as a value, and it lists the exports in assignment order, like `module.exports` does. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default`, and `ns.default` on `import * as ns`, is the lifted `default` export when the module has one, and otherwise the namespace itself, as `module.exports` is in Node. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import then depends on that flag at run time. An `import()` of a module that does not set both resolves to that same namespace object as `default`, with or without code splitting. +The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` of a module that does not set both resolves to that same `module.exports` object as `default`, with or without code splitting. ### deprecatedNamespaceObjectSetters -Default `true`. When a namespace object does have to be created, each property currently gets a getter and a setter; the setter accepts `ns.foo = value` without throwing (reads still return the module's binding). Set this to `false` to emit getter-only namespace objects, which is what a future Bun release will do unconditionally. The namespace of a lifted CommonJS module is not affected: it stands in for `module.exports`, so its setters assign the lifted bindings either way. +Default `true`. When a namespace object does have to be created, each property currently gets a getter and a setter; the setter accepts `ns.foo = value` without throwing (reads still return the module's binding). Set this to `false` to emit getter-only namespace objects, which is what a future Bun release will do unconditionally. The `module.exports` object of a lifted CommonJS module is not affected: its setters assign the lifted bindings either way. diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index 5d8576f64d36..3acd0a03cca8 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -3661,8 +3661,9 @@ impl<'a> LinkerContext<'a> { let ast_flags = self.graph.ast.items_flags(); let is_import_stmt = first_hop.is_none(); - let (other_source_index, alias, alias_is_star, is_exported) = match first_hop { - Some((source, alias)) => (source, Some(alias), false, false), + let (other_source_index, alias, alias_is_star, is_exported, namespace_ref) = match first_hop + { + Some((source, alias)) => (source, Some(alias), false, false, Ref::NONE), None => { let named_import: &NamedImport = match self.graph.ast.items_named_imports() [id as usize] @@ -3704,6 +3705,7 @@ impl<'a> LinkerContext<'a> { named_import.alias, named_import.alias_is_star, named_import.is_exported, + named_import.namespace_ref, ) } }; @@ -3764,36 +3766,32 @@ impl<'a> LinkerContext<'a> { }; } - // The default import of a lifted CommonJS module is `module.exports`, - // which is its namespace: bind it like `import * as X`. `ns.default` on - // `import * as ns` (a generated item) reads the namespace object's own - // `default` key when the module exports one. + // The default import of a lifted CommonJS module is `module.exports`, which + // `exports_foo` stands in for. So are `ns.default` on `import * as ns` (a + // generated item) and a `require()` that `unwrap_commonjs_to_esm` turned + // into an import star: bind them to that object. A `.default` read off + // such a `require()` is `exports.default`, an ordinary export. if is_import_stmt - && !alias_is_star && flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) - && alias.is_some_and(|a| a.slice() == b"default") - && !Self::lifted_default_import_needs_wrapper( - self.graph.ast.items_module_type()[id as usize], - &self.graph.ast.items_named_exports()[other_id as usize], - ) - && !(self - .graph - .symbols - .get_const(tracker.import_ref) - .is_some_and(|s| s.import_item_status == ImportItemStatus::Generated) - && self.graph.meta.items_resolved_exports()[other_id as usize] - .get(b"default") - .is_some()) + && if alias_is_star { + self.import_star_was_require_call(id, tracker.import_ref) + } else { + alias.is_some_and(|a| a.slice() == b"default") + && !Self::lifted_default_import_needs_wrapper( + self.graph.ast.items_module_type()[id as usize], + &self.graph.ast.items_named_exports()[other_id as usize], + ) + && !(namespace_ref.is_valid() + && self.import_star_was_require_call(id, namespace_ref)) + } { - let matching_export = &self.graph.meta.items_resolved_export_star()[other_id as usize]; return ImportTrackerIterator { - value: matching_export.data, + value: ImportTracker { + source_index: crate::Index::init(other_source_index), + import_ref: self.graph.ast.items_exports_ref()[other_id as usize], + ..Default::default() + }, status: ImportTrackerStatus::Found, - import_data: bun_ptr::BackRef::new( - matching_export - .potentially_ambiguous_export_star_refs - .slice(), - ), ..Default::default() }; } @@ -4381,15 +4379,30 @@ impl<'a> LinkerContext<'a> { fn is_esm_namespace_ref(&self, source_index: crate::IndexInt, ref_: Ref) -> bool { let id = source_index as usize; - id < self.graph.ast.len() - && ref_ == self.graph.ast.items_exports_ref()[id] - && matches!( + if id >= self.graph.ast.len() { + return false; + } + if ref_ == self.graph.ast.items_exports_ref()[id] { + return matches!( self.graph.ast.items_exports_kind()[id], ExportsKind::Esm | ExportsKind::EsmWithDynamicFallback | ExportsKind::EsmWithDynamicFallbackFromCjs - ) - && self.graph.meta.items_flags()[id].wrap != WrapKind::Cjs + ) && self.graph.meta.items_flags()[id].wrap != WrapKind::Cjs; + } + // The `import *` namespace of a lifted CommonJS module. + ref_.is_valid() && ref_ == self.lifted_namespace_ref(source_index) + } + + /// Is `namespace_ref` of file `source_index` the `import * as ns` that the + /// parser made out of a `require()` call (`unwrap_commonjs_to_esm`)? Such a + /// call returns `module.exports`, not the namespace. + fn import_star_was_require_call(&self, source_index: crate::IndexInt, namespace_ref: Ref) -> bool { + let parts = self.graph.ast.items_parts()[source_index as usize].as_slice(); + self.graph + .top_level_symbol_to_parts(source_index, namespace_ref) + .iter() + .any(|&part| parts[part as usize].tag == bun_ast::PartTag::ImportToConvertFromRequire) } /// The default import of a lifted module that sets `__esModule` and exports @@ -4719,6 +4732,9 @@ impl<'a> LinkerContext<'a> { name: bun_ast::StoreStr, count: u32, is_call_target: bool, + /// `X.default` where `X` is the `import *` namespace of a lifted CommonJS + /// module: `module.exports`, which `exports_foo` stands in for. + is_module_exports: bool, } let id = source_index as usize; @@ -4738,34 +4754,19 @@ impl<'a> LinkerContext<'a> { if !self.is_esm_namespace_ref(target_source, target.import_ref) { continue; } + let lifted_namespace = self.lifted_namespace_ref(target_source); + let base_is_lifted_namespace = + lifted_namespace.is_valid() && target.import_ref == lifted_namespace; let resolved_exports = &self.graph.meta.items_resolved_exports()[target_source as usize]; for (name, prop_use) in properties.iter() { + let is_module_exports = base_is_lifted_namespace && &**name == b"default"; // Not a static export of the target (missing, or only reachable // through `export *` from CommonJS): keep the property access. - let name = if let Some(index) = resolved_exports.get_index(name) { + let name = if is_module_exports { + bun_ast::StoreStr::new(b"default") + } else if let Some(index) = resolved_exports.get_index(name) { bun_ast::StoreStr::new(&resolved_exports.keys()[index]) - } else if &**name == b"default" - && self.graph.ast.items_flags()[target_source as usize] - .contains(AstFlags::COMMONJS_LIFTED_TO_ESM) - && !Self::lifted_default_import_needs_wrapper( - self.graph.ast.items_module_type()[id], - &self.graph.ast.items_named_exports()[target_source as usize], - ) - { - // `default` of a lifted CommonJS module is `module.exports`, the - // namespace itself, the same as `ns.default` on `import * as ns`. - let name = bun_ast::StoreStr::new(b"default"); - member_resolutions - .entry((target_source, name)) - .or_insert_with(|| { - Some(ImportMemberResolution { - source_index: target_source, - r#ref: target.import_ref, - re_exports: Vec::new(), - }) - }); - name } else { continue; }; @@ -4776,14 +4777,20 @@ impl<'a> LinkerContext<'a> { name, count: prop_use.count_estimate, is_call_target: prop_use.is_call_target, + is_module_exports, }); } } } + let module_exports_of = |this: &Self, access: &PropertyAccess| ImportMemberResolution { + source_index: access.target_source, + r#ref: this.graph.ast.items_exports_ref()[access.target_source as usize], + re_exports: Vec::new(), + }; for access in &accesses { let key = (access.target_source, access.name); - if member_resolutions.contains_key(&key) { + if access.is_module_exports || member_resolutions.contains_key(&key) { continue; } self.cycle_detector.clear(); @@ -4813,12 +4820,22 @@ impl<'a> LinkerContext<'a> { // call that needs `X` as `this` keeps every `X.name` of the file. let mut keeps_this: Vec<(Ref, bun_ast::StoreStr)> = Vec::new(); for access in &accesses { - if access.is_call_target - && let Some(resolved) = member_resolutions - .get(&(access.target_source, access.name)) - .unwrap() - && self.method_call_needs_this(resolved.source_index, resolved.r#ref) + if !access.is_call_target { + continue; + } + let module_exports; + let resolved = if access.is_module_exports { + module_exports = module_exports_of(self, access); + &module_exports + } else if let Some(resolved) = member_resolutions + .get(&(access.target_source, access.name)) + .unwrap() { + resolved + } else { + continue; + }; + if self.method_call_needs_this(resolved.source_index, resolved.r#ref) { keeps_this.push((access.base, access.name)); } } @@ -4834,10 +4851,16 @@ impl<'a> LinkerContext<'a> { if keeps_this.contains(&(base, name)) { continue; } - let Some(resolved) = member_resolutions + let module_exports; + let resolved = if access.is_module_exports { + module_exports = module_exports_of(self, access); + &module_exports + } else if let Some(resolved) = member_resolutions .get(&(access.target_source, name)) .unwrap() - else { + { + resolved + } else { continue; }; @@ -5016,6 +5039,109 @@ impl<'a> LinkerContext<'a> { Ok((r#ref, part_index)) } + /// The `import *` namespace of a lifted CommonJS module (see `LiftedNamespace`). + pub(crate) fn lifted_namespace_ref(&self, source_index: crate::IndexInt) -> Ref { + let id = source_index as usize; + if id < self.graph.meta.len() { + self.graph.meta.items_lifted_namespace()[id].ref_ + } else { + Ref::NONE + } + } + + /// Declares that namespace, `var import_foo = __toESM(exports_foo, 1)`, in a + /// part of its own, so it is dropped unless an importer uses the namespace + /// as a value. Mode `1` makes `default` the `module.exports` object: an + /// importer that is not an ES module by type got a CommonJS wrapper instead + /// when `__esModule` would change that (`lifted_default_import_needs_wrapper`). + pub(crate) fn create_lifted_namespace_part( + &mut self, + source_index: crate::IndexInt, + ) -> Result<(), AllocError> { + let id = source_index as usize; + let exports_ref = self.graph.ast.items_exports_ref()[id]; + let loc = Loc::EMPTY; + + let mut name: Vec = Vec::new(); + core::fmt::Write::write_fmt( + &mut bun_core::fmt::VecWriter(&mut name), + format_args!( + "import_{}", + self.parse_graph().input_files.items_source()[id].fmt_identifier() + ), + ) + .expect("infallible: VecWriter never errors"); + // SAFETY: `LinkerContext::arena()` returns a stable `&Arena` valid for the + // link pass; detach so it does not borrow `self` across the `&mut self` calls. + let arena: &Bump = unsafe { bun_ptr::detach_lifetime_ref::(self.arena()) }; + let namespace_ref = self.graph.generate_new_symbol( + source_index, + bun_ast::symbol::Kind::Other, + arena.alloc_slice_copy(&name), + ); + + let to_esm_ref = self.runtime_function(b"__toESM"); + let value = Expr::init( + E::Call { + target: Expr::init_identifier(to_esm_ref, loc), + args: bun_ast::ExprNodeList::from_slice(&[ + Expr::init_identifier(exports_ref, loc), + Expr::init(E::Number::new(1.0), loc), + ]), + ..Default::default() + }, + loc, + ); + let stmts: &mut [Stmt] = arena.alloc_slice_fill_iter(core::iter::once(Stmt::alloc( + S::Local { + decls: G::DeclList::from_slice(&[G::Decl { + binding: Binding::alloc( + arena, + bun_ast::b::Identifier { + r#ref: namespace_ref, + }, + loc, + ), + value: Some(value), + }]), + ..Default::default() + }, + loc, + ))); + let part_index = self.graph.add_part_to_file( + source_index, + Part { + stmts: bun_ast::StoreSlice::new_mut(stmts), + declared_symbols: DeclaredSymbolList::from_slice(&[DeclaredSymbol { + ref_: namespace_ref, + is_top_level: true, + }])?, + can_be_removed_if_unused: true, + force_tree_shaking: true, + ..Default::default() + }, + )?; + self.graph.generate_symbol_import_and_use( + source_index, + part_index, + exports_ref, + 1, + crate::Index::init(source_index), + )?; + self.graph.generate_symbol_import_and_use( + source_index, + part_index, + to_esm_ref, + 1, + crate::Index::RUNTIME, + )?; + self.graph.meta.items_lifted_namespace_mut()[id] = crate::js_meta::LiftedNamespace { + ref_: namespace_ref, + part_index, + }; + Ok(()) + } + pub(crate) fn break_output_into_pieces( &self, _alloc: *const Bump, diff --git a/src/bundler/LinkerGraph.rs b/src/bundler/LinkerGraph.rs index 8cddb13f50fc..b2e5adc41871 100644 --- a/src/bundler/LinkerGraph.rs +++ b/src/bundler/LinkerGraph.rs @@ -154,6 +154,25 @@ pub mod js_meta { } pub use crate::WrapKind as Wrap; + /// The `import *` namespace of a CommonJS module whose exports were lifted + /// to ESM. `exports_foo` stands in for `module.exports`, so the namespace is + /// a second object, `var import_foo = __toESM(exports_foo, 1)`, whose + /// `default` is `exports_foo`. Unset (`ref_` is `Ref::NONE`) for other files. + #[derive(Clone, Copy)] + pub struct LiftedNamespace { + pub ref_: Ref, + /// The part that declares `ref_`. It prints with the namespace export part. + pub part_index: u32, + } + impl Default for LiftedNamespace { + fn default() -> Self { + Self { + ref_: Ref::NONE, + part_index: u32::MAX, + } + } + } + pub struct JSMeta { pub probably_typescript_type: ProbablyTypescriptType, pub imports_to_bind: RefImportData, @@ -164,9 +183,10 @@ pub mod js_meta { pub cjs_export_copies: CjsExportCopies, pub wrapper_part_index: Index, pub dynamic_import_referenced_aliases: DynamicImportReferencedAliases, - /// The parameter of the setters on a lifted CommonJS module's namespace + /// The parameter of the setters on a lifted CommonJS module's exports /// object (`set: (value) => $foo = value`). `Ref::NONE` for other files. pub lifted_setter_param: Ref, + pub lifted_namespace: LiftedNamespace, pub flags: Flags, } @@ -183,6 +203,7 @@ pub mod js_meta { wrapper_part_index: Index::default(), dynamic_import_referenced_aliases: DynamicImportReferencedAliases::default(), lifted_setter_param: Ref::NONE, + lifted_namespace: LiftedNamespace::default(), flags: Flags::default(), } } @@ -200,6 +221,7 @@ pub mod js_meta { wrapper_part_index: Index, dynamic_import_referenced_aliases: DynamicImportReferencedAliases, lifted_setter_param: Ref, + lifted_namespace: LiftedNamespace, flags: Flags, } } diff --git a/src/bundler/linker_context/doStep5.rs b/src/bundler/linker_context/doStep5.rs index 431b71a14ee5..7156927d9593 100644 --- a/src/bundler/linker_context/doStep5.rs +++ b/src/bundler/linker_context/doStep5.rs @@ -213,7 +213,7 @@ impl LinkerContext<'_> { // and only store a count instead of an array // // A module namespace object lists its exports in code unit order. The - // namespace of a CommonJS module whose `exports.foo = ...` assignments + // exports object of a CommonJS module whose `exports.foo = ...` assignments // were lifted to ES module exports stands in for `module.exports`, so // it keeps the assignment order (the order of `resolved_exports`). if !is_lifted_commonjs { @@ -458,7 +458,7 @@ impl LinkerContext<'_> { // 1 property per export let mut properties = bun_alloc::ArenaVec::::with_capacity_in(export_aliases.len(), arena); - // A lifted CommonJS module's namespace stands in for `module.exports`: + // A lifted CommonJS module's exports object stands in for `module.exports`: // writes through it assign the lifted bindings, so every local export // also gets a setter. let mut setter_properties = bun_alloc::ArenaVec::::with_capacity_in( diff --git a/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs b/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs index 5465f739e421..9dcfbf839f14 100644 --- a/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs +++ b/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs @@ -306,8 +306,13 @@ impl<'a, 'ctx> FindImportedPartsVisitor<'a, 'ctx> { can_be_split, } => { let part = &self.parts[source_index as usize].as_slice()[part_index as usize]; + // The namespace export part was added on `Enter`, and the lifted + // namespace part prints with it. if can_be_split && part_index != bun_ast::NAMESPACE_EXPORT_PART_INDEX + && part_index + != self.c.graph.meta.items_lifted_namespace()[source_index as usize] + .part_index && self.c.should_include_part(source_index, part) { self.append_or_extend_range( diff --git a/src/bundler/linker_context/generateCodeForFileInChunkJS.rs b/src/bundler/linker_context/generateCodeForFileInChunkJS.rs index f7ba8e07ca3c..9078e5311ef0 100644 --- a/src/bundler/linker_context/generateCodeForFileInChunkJS.rs +++ b/src/bundler/linker_context/generateCodeForFileInChunkJS.rs @@ -56,6 +56,9 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>( } else { Index::INVALID }; + // Printed with the namespace export part (see `LiftedNamespace`). + let lifted_namespace_part_index: u32 = + c.graph.meta.items_lifted_namespace()[source_index].part_index; // referencing everything by array makes the code a lot more annoying :( // @@ -310,6 +313,30 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>( return PrintResult::Err(err.into()); } + // `var import_foo = __toESM(exports_foo, 1)` of a lifted CommonJS module goes + // right after `__exportCjs(exports_foo, ...)`: outside the wrapper, and ahead + // of the module's dependencies, which may read it back in a cycle. + if lifted_namespace_part_index != u32::MAX + && parts_live.is_set(lifted_namespace_part_index as usize) + { + let lifted_namespace_stmts: &[Stmt] = c.graph.ast.items_parts()[source_index] + .as_slice()[lifted_namespace_part_index as usize] + .stmts + .slice(); + if let Err(err) = convert_stmts_for_chunk( + c, + source_index as u32, + stmts, + lifted_namespace_stmts, + chunk, + temp_arena, + flags.wrap, + &ast, + ) { + return PrintResult::Err(err.into()); + } + } + match flags.wrap { WrapKind::Esm => { // Borrowck: `append_slice` borrows `stmts` mutably while @@ -354,6 +381,11 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>( continue; } + if index == lifted_namespace_part_index { + // Printed with the namespace export part above + continue; + } + let mut single_stmts_list: [Stmt; 1] = [Stmt::empty()]; let mut part_stmts: &[Stmt] = part.stmts.slice(); diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index aa807c0dbc0d..e80a7c5dcc8a 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -318,17 +318,23 @@ pub(crate) fn scan_imports_and_exports( } // A default import of a lifted CommonJS module binds to its - // namespace (`advance_import_tracker`) unless `__esModule` - // has to be checked at run time. - if record + // `module.exports` object (`advance_import_tracker`), and so does + // `default` of its `import *` namespace, unless `__esModule` has + // to be checked at run time. + let is_lifted = other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM); + let has_default_alias = record .flags - .contains(ImportRecordFlags::CONTAINS_DEFAULT_ALIAS) - && other_flags.contains(AstFlags::FORCE_CJS_TO_ESM) - && (!other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) - || LinkerContext::lifted_default_import_needs_wrapper( - col_ref!(module_types)[id], - &col_ref!(named_exports)[other_file], - )) + .contains(ImportRecordFlags::CONTAINS_DEFAULT_ALIAS); + let has_import_star = + record.flags.contains(ImportRecordFlags::CONTAINS_IMPORT_STAR); + if other_flags.contains(AstFlags::FORCE_CJS_TO_ESM) + && ((has_default_alias && !is_lifted) + || ((has_default_alias || has_import_star) + && is_lifted + && LinkerContext::lifted_default_import_needs_wrapper( + col_ref!(module_types)[id], + &col_ref!(named_exports)[other_file], + ))) { col!(exports_kind)[other_file] = ExportsKind::Cjs; col!(flags)[other_file].wrap = WrapKind::Cjs; @@ -518,10 +524,24 @@ pub(crate) fn scan_imports_and_exports( // Also add a special export so import stars can bind to it. This must be // done in this step because it must come after CommonJS module discovery // but before matching imports with exports. + // + // `exports_foo` of a lifted CommonJS module stands in for `module.exports`, + // which default imports bind to. Import stars bind to a second object whose + // `default` is `exports_foo`, like the namespace Node gives such a module. + let mut namespace_ref = col_ref!(exports_refs)[id]; + if id < col_ref!(import_records_list).len() + && col_ref!(css_asts)[id].is_none() + && col_ref!(ast_flags_list)[id].contains(AstFlags::COMMONJS_LIFTED_TO_ESM) + && col_ref!(exports_kind)[id] != ExportsKind::Cjs + && output_format != Format::InternalBakeDev + { + this.create_lifted_namespace_part(source_index)?; + namespace_ref = this.lifted_namespace_ref(source_index); + } col!(resolved_export_stars)[id] = ExportData { data: ImportTracker { source_index: Index::source(source_index), - import_ref: col_ref!(exports_refs)[id], + import_ref: namespace_ref, ..Default::default() }, ..Default::default() diff --git a/test/bundler/bundler_cjs.test.ts b/test/bundler/bundler_cjs.test.ts index 957c4269f83d..7befa8e95ed5 100644 --- a/test/bundler/bundler_cjs.test.ts +++ b/test/bundler/bundler_cjs.test.ts @@ -125,10 +125,10 @@ describe("bundler", () => { `, }, run: { - // Namespace import only gets the CJS exports as-is, no default wrapper. - // The lifted module keeps the order of its `exports.x = ...` assignments, - // like `module.exports` would. - stdout: '{"foo":"foo","bar":"bar"}', + // The namespace of the lifted module has `default` set to its + // `module.exports` object, then the named exports in the order of the + // `exports.x = ...` assignments, as `bun run` and esbuild print it. + stdout: '{"default":{"foo":"foo","bar":"bar"},"foo":"foo","bar":"bar"}', }, }); @@ -373,10 +373,11 @@ describe("bundler", () => { `, }, run: { - // The default import is `module.exports`, which for a lifted CommonJS - // module is the namespace object itself, so the namespace has no - // separate `default` key (the same as a lone `import *`, Test 6). - stdout: '{"default":{"foo":"foo","bar":"bar"},"named":"foo","namespace":{"foo":"foo","bar":"bar"}}', + // The default import is `module.exports`. The namespace is a second + // object whose `default` is that `module.exports` (the same as a lone + // `import *`, Test 6). + stdout: + '{"default":{"foo":"foo","bar":"bar"},"named":"foo","namespace":{"default":{"foo":"foo","bar":"bar"},"foo":"foo","bar":"bar"}}', }, }); diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 7c5964726e11..9cb819d8c0d4 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -485,8 +485,8 @@ describe("bundler", () => { stdout: "checkDCE\nproduction root", }, }); - // A default import of the converted file binds to its namespace, which - // holds the re-exported names (the export star resolves at link time). + // A default import of the converted file binds to its `module.exports` object, + // which holds the re-exported names (the export star resolves at link time). itBundled("cjs2esm/ReactSpecificUnwrappingSideEffectDefaultImport", { files: { "/entry.js": /* js */ ` @@ -505,14 +505,15 @@ describe("bundler", () => { cjs2esm: true, minifySyntax: true, run: { - stdout: "side effect\nrendered rendered 19.0.0 object", + // `impl.js` exports no `default`, so `module.exports.default` is undefined + stdout: "side effect\nrendered rendered 19.0.0 undefined", }, }); itBundled("cjs2esm/ReactSpecificUnwrappingSideEffectNamespaceImport", { files: { "/entry.js": /* js */ ` import * as ReactDOM from "react-dom"; - console.log(ReactDOM.render(), Object.keys(ReactDOM).sort().join(","), typeof ReactDOM.default); + console.log(ReactDOM.render(), Object.keys(ReactDOM).sort().join(","), typeof ReactDOM.default, ReactDOM.default.render === ReactDOM.render); `, "/node_modules/react-dom/index.js": /* js */ ` console.log('side effect'); @@ -526,18 +527,21 @@ describe("bundler", () => { cjs2esm: true, minifySyntax: true, run: { - stdout: "side effect\nrendered render,version object", + // the namespace lists `default` (the `module.exports` object) and the names + stdout: "side effect\nrendered default,render,version object true", }, }); // `module.exports = ns` re-exports the whole namespace, so `default` of an - // ES module target comes through too (a plain `export *` would drop it). + // ES module target comes through too (a plain `export *` would drop it): + // it is `module.exports.default`, which the default import, `ns.default` + // and `require()` all reach. itBundled("cjs2esm/ReactSpecificUnwrappingSideEffectTargetHasDefault", { files: { "/entry.js": /* js */ ` import ReactDOM, { version } from "react-dom"; import * as ns from "react-dom"; const m = require("react-dom"); - console.log(version, typeof ReactDOM.default, ReactDOM.default(), typeof ns.default, ns.default(), m.default(), Object.keys(ns).sort().join(",")); + console.log(version, typeof ReactDOM.default, ReactDOM.default(), ns.default === ReactDOM, ns.default.default(), m === ReactDOM, m.default(), Object.keys(ns).sort().join(",")); `, "/node_modules/react-dom/index.js": /* js */ ` console.log('side effect'); @@ -551,7 +555,7 @@ describe("bundler", () => { cjs2esm: true, minifySyntax: true, run: { - stdout: "side effect\n19.0.0 function rendered function rendered rendered default,version", + stdout: "side effect\n19.0.0 function rendered true rendered true rendered default,version", }, }); itBundled("cjs2esm/ReactSpecificUnwrappingSideEffectTargetHasDefaultRequireOnly", { @@ -1152,7 +1156,9 @@ describe("bundler", () => { }, cjs2esm: true, run: { - stdout: "true true true true true", + // `R` is the namespace and `React` its `default`, the `module.exports` + // object: two objects over the same bindings, as in Node + stdout: "true true true true false", }, }); itBundled("cjs2esm/DefaultImportComputedMemberKeepsNamespace", { @@ -1191,20 +1197,20 @@ describe("bundler", () => { stdout: '{"zeta":1,"alpha":2} object 1', }, }); - itBundled("cjs2esm/DefaultImportDotDefaultIsNamespace", { + itBundled("cjs2esm/StarImportDotDefaultIsModuleExports", { files: { "/entry.js": /* js */ ` import React from "react"; import * as R from "react"; - console.log(React.default === React, R.default === React, React.default.useState === React.useState); + console.log(React.default, R.default === React, R.default.useState === React.useState, "default" in R, "default" in React); `, ...liftedReact, }, cjs2esm: true, run: { - // the module has no `default` export, so `.default` is `module.exports` - // through both import forms - stdout: "true true true", + // the module has no `default` export: `module.exports.default` is + // undefined and `ns.default` is `module.exports`, as in Node + stdout: "undefined true true true false", }, }); itBundled("cjs2esm/DefaultImportDotDefaultOfExportsDefault", { @@ -1212,7 +1218,7 @@ describe("bundler", () => { "/entry.js": /* js */ ` import lib from "./lib.js"; import * as ns from "./lib.js"; - console.log(lib.default(), ns.default(), ns.default === lib.default, lib.foo, Object.keys(lib).join(",")); + console.log(lib.default(), ns.default === lib, ns.default.default === lib.default, lib.foo, Object.keys(lib).join(",")); `, "/lib.js": /* js */ ` exports.default = function def() { return "def"; }; @@ -1221,14 +1227,16 @@ describe("bundler", () => { }, cjs2esm: true, onAfterBundle(api) { - // `lib.default` and `ns.default` bind to the lifted export; only - // `Object.keys(lib)` materializes the namespace object - api.expectFile("/out.js").toContain("$default()"); + // `lib.default` binds to the lifted export and `ns.default` to the + // `module.exports` object; the namespace object of `ns` is never made + const out = api.readFile("/out.js"); + expect(out).toContain("$default()"); + expect(out).not.toContain("__toESM"); }, run: { // without `__esModule`, the default import is the whole `module.exports`, - // and `ns.default` is its own `default` key - stdout: "def def true 1 default,foo", + // and so is `ns.default` + stdout: "def true true 1 default,foo", }, }); itBundled("cjs2esm/DefaultImportWithEsModuleKeepsWrapper", { @@ -1269,9 +1277,9 @@ describe("bundler", () => { stdout: "d 1 true 1", }, }); - // `.default` of a lifted module that sets `__esModule` but exports no - // `default` is the namespace for every importer, as `__toESM` and `bun run` - // give `module.exports`. Both routes to the namespace, `import * as ns` and + // `ns.default` of a lifted module that sets `__esModule` but exports no + // `default` is `module.exports` for every importer, as `__toESM` and `bun run` + // give it. Both routes to the namespace, `import * as ns` and // `export * as Lib`, must agree. const esModuleNoDefault = { "/lib.js": /* js */ ` @@ -1287,28 +1295,28 @@ describe("bundler", () => { "/entry.js": /* js */ ` import * as ns from "./lib.js"; import { Lib } from "./mid.js"; - console.log(Lib === ns, Lib.default === ns, ns.default === ns, Lib.foo); + console.log(Lib === ns, Lib.default === ns.default, ns.default === ns, ns.default.foo, Lib.foo, Lib.default.__esModule); `, ...esModuleNoDefault, }, cjs2esm: true, run: { - stdout: "true true true 1", + stdout: "true true false 1 1 true", }, }); - // The default import of that module is the namespace too, so the module + // The default import of that module is `module.exports`, so the module // stays lifted for a `.js` importer itBundled("cjs2esm/DefaultImportWithEsModuleNoDefaultFromCjsImporter", { files: { "/entry.js": /* js */ ` import lib, * as ns from "./lib.js"; - console.log(lib === ns, lib.default === ns, lib.__esModule, lib.foo); + console.log(lib === ns.default, lib === ns, lib.default, lib.__esModule, lib.foo); `, ...esModuleNoDefault, }, cjs2esm: true, run: { - stdout: "true true true 1", + stdout: "true false undefined true 1", }, }); itBundled("cjs2esm/DotDefaultWithEsModuleNoDefaultFromEsmImporter", { @@ -1316,13 +1324,13 @@ describe("bundler", () => { "/entry.mjs": /* js */ ` import * as ns from "./lib.js"; import { Lib } from "./mid.js"; - console.log(Lib === ns, Lib.default === ns, ns.default === ns, Lib.foo); + console.log(Lib === ns, Lib.default === ns.default, ns.default === ns, ns.default.foo, Lib.foo, Lib.default.__esModule); `, ...esModuleNoDefault, }, cjs2esm: true, run: { - stdout: "true true true 1", + stdout: "true true false 1 1 true", }, }); itBundled("cjs2esm/ReExportDefaultAsNameFromLiftedCommonJS", { @@ -1698,7 +1706,7 @@ describe("bundler", () => { "/b.js": /* js */ ` import React from "react"; import * as R from "react"; - console.log("b", React.useState(2)[0], React === R, Object.keys(R).length); + console.log("b", React.useState(2)[0], React === R.default, Object.keys(R).join(",")); `, ...liftedReact, }, @@ -1707,7 +1715,7 @@ describe("bundler", () => { splitting: true, run: [ { file: "/out/a.js", stdout: "a a id" }, - { file: "/out/b.js", stdout: "b 2 true 4" }, + { file: "/out/b.js", stdout: "b 2 true default,createElement,useState,useId,version" }, ], }); // The chunk a split `import()` of a lifted CommonJS module loads exports the @@ -1727,7 +1735,7 @@ describe("bundler", () => { const m = await import("./lib.cjs"); lib.expando = 1; m.default.version = "patched"; - console.log(m.default === lib, m.default === ns, m.default.expando, lib.version, m.version, Object.keys(m.default).join(",")); + console.log(m.default === lib, m.default === ns.default, m.default.expando, lib.version, m.version, ns.version, Object.keys(m.default).join(",")); `, "/lib.cjs": liftedLib, }, @@ -1738,7 +1746,7 @@ describe("bundler", () => { expect(splitChunk(api, "lib")).toContain("export default exports_lib;"); expect(splitChunk(api, "lib")).not.toContain("get createElement()"); }, - run: { file: "/out/entry.js", stdout: "true true 1 patched patched createElement,version,expando" }, + run: { file: "/out/entry.js", stdout: "true true 1 patched patched patched createElement,version,expando" }, }); itBundled("cjs2esm/SplitDynamicImportOnlyOfLiftedCommonJS", { files: { @@ -2118,12 +2126,12 @@ describe("bundler", () => { }, }); // A lone `import * as ns` of a lifted CommonJS module: `ns.default` is - // `module.exports`, which is the namespace itself. + // `module.exports`, and the namespace is a separate object that lists it. itBundled("cjs2esm/ImportStarOfLiftedCommonJSHasDefault", { files: { "/entry.mjs": /* js */ ` import * as ns from "./c.cjs"; - console.log(typeof ns.default, ns.default.n, ns.default === ns, Object.keys(ns).join(",")); + console.log(typeof ns.default, ns.default.n, ns.default === ns, Object.keys(ns).join(","), Object.keys(ns.default).join(",")); `, "/c.cjs": /* js */ ` exports.n = 7; @@ -2131,8 +2139,135 @@ describe("bundler", () => { }, cjs2esm: true, run: { - stdout: "object 7 true n", + stdout: "object 7 false default,n n", + }, + }); + + // `ns.default` on `import * as ns` of a lifted CommonJS module is the same + // object a default import of that module binds, `module.exports`, even when + // the module assigns `exports.default`. The namespace is a separate object, + // so a computed `ns["default"]` agrees with the static read. Node, `bun run` + // and esbuild print the same. + const liftedWithDefault = { + "/dep.cjs": /* js */ ` + exports.default = { m: "exports.default" }; + exports.zz = 1; + `, + }; + const starAndDefaultImport = /* js */ ` + import * as ns from "./dep.cjs"; + import d, { default as d2 } from "./dep.cjs"; + const key = "def" + "ault"; + console.log(JSON.stringify([ + d === ns.default, + d2 === d, + ns === d, + ns[key] === d, + ns.default.default === d.default, + d.default.m, + ns.zz, + Object.keys(ns), + Object.keys(d), + JSON.stringify(ns), + ])); + `; + const starAndDefaultImportStdout = + '[true,true,false,true,true,"exports.default",1,["default","zz"],["default","zz"],"{\\"default\\":{\\"default\\":{\\"m\\":\\"exports.default\\"},\\"zz\\":1},\\"zz\\":1}"]'; + itBundled("cjs2esm/StarImportDefaultIsTheDefaultImport", { + files: { + "/entry.mjs": starAndDefaultImport, + ...liftedWithDefault, + }, + cjs2esm: true, + onAfterBundle(api) { + api.expectFile("/out.js").toContain("import_dep = __toESM(exports_dep, 1)"); + }, + run: { stdout: starAndDefaultImportStdout }, + }); + itBundled("cjs2esm/StarImportDefaultIsTheDefaultImportMinified", { + files: { + "/entry.mjs": starAndDefaultImport, + ...liftedWithDefault, + }, + minifySyntax: true, + minifyIdentifiers: true, + run: { stdout: starAndDefaultImportStdout }, + }); + // The lifted module ends up in a lazy ESM wrapper here (a wrapped file imports + // it). Its namespace object is made next to `exports_dep`, outside `init_dep`. + itBundled("cjs2esm/StarImportDefaultIsTheDefaultImportEsmWrapper", { + files: { + "/entry.mjs": /* js */ ` + const { run } = await import("./mid.mjs"); + run(); + `, + "/mid.mjs": /* js */ ` + ${starAndDefaultImport} + export function run() {} + `, + ...liftedWithDefault, + }, + cjs2esm: true, + onAfterBundle(api) { + const out = api.readFile("/out.js"); + expect(out).toContain("init_dep = __esm("); + expect(out).toContain("import_dep = __toESM(exports_dep, 1)"); + }, + run: { stdout: starAndDefaultImportStdout }, + }); + // The namespace reached through another module is that same object. + itBundled("cjs2esm/StarReExportDefaultIsTheDefaultImport", { + files: { + "/entry.mjs": /* js */ ` + import d from "./dep.cjs"; + import * as ns from "./dep.cjs"; + import lib, { inner, outer, zz } from "./mid.mjs"; + console.log(inner === ns, outer === ns, lib === ns, inner.default === d, outer.default === d, lib.default === d, inner.zz, zz, typeof lib.default.default.m); + `, + "/mid.mjs": /* js */ ` + export * from "./dep.cjs"; + import * as inner from "./dep.cjs"; + export { inner }; + export * as outer from "./dep.cjs"; + export default inner; + `, + ...liftedWithDefault, + }, + cjs2esm: true, + run: { stdout: "true true true true true true 1 1 string" }, + }); + // `exports.__esModule = true` by assignment is lifted too. An importer that is + // an ES module by type ignores the flag, as Node does: the default import and + // `ns.default` are both the whole `module.exports`. + itBundled("cjs2esm/StarImportDefaultWithEsModuleFromEsmImporter", { + files: { + "/entry.mjs": /* js */ ` + import d from "./p.cjs"; + import * as ns from "./p.cjs"; + console.log(d === ns.default, ns === d, JSON.stringify(ns["def" + "ault"]), JSON.stringify(ns.default.default), Object.keys(ns).join(",")); + `, + "/p.cjs": /* js */ ` + exports.__esModule = true; exports.default = { m: "D" }; exports.a = "a"; + `, + }, + cjs2esm: true, + run: { stdout: 'true false {"__esModule":true,"default":{"m":"D"},"a":"a"} {"m":"D"} default,__esModule,a' }, + }); + // For any other importer `ns.default` depends on the flag at run time, like a + // default import does, so a star import keeps the CommonJS wrapper too. + itBundled("cjs2esm/StarImportWithEsModuleFromCjsImporterKeepsWrapper", { + files: { + "/entry.js": /* js */ ` + import * as ns from "./p.cjs"; + import { a } from "./p.cjs"; + console.log(JSON.stringify(ns.default), ns["def" + "ault"] === ns.default, a, Object.keys(ns).join(",")); + `, + "/p.cjs": /* js */ ` + exports.__esModule = true; exports.default = { m: "D" }; exports.a = "a"; + `, }, + cjs2esm: { unhandled: ["/p.cjs"] }, + run: { stdout: '{"m":"D"} true a __esModule,default,a' }, }); // A write through the namespace of a lifted CommonJS module assigns the diff --git a/test/bundler/esbuild/dce.test.ts b/test/bundler/esbuild/dce.test.ts index f3f159d484c3..74159a82a424 100644 --- a/test/bundler/esbuild/dce.test.ts +++ b/test/bundler/esbuild/dce.test.ts @@ -103,7 +103,7 @@ describe("bundler", () => { `, }, run: { - stdout: 'hello\n{"foo":123}', + stdout: 'hello\n{"default":{"foo":123},"foo":123}', }, }); itBundled("dce/PackageJsonSideEffectsTrueKeepES6", { diff --git a/test/bundler/esbuild/importstar.test.ts b/test/bundler/esbuild/importstar.test.ts index 79c29d6074c7..d8dc8a88ec22 100644 --- a/test/bundler/esbuild/importstar.test.ts +++ b/test/bundler/esbuild/importstar.test.ts @@ -224,7 +224,7 @@ describe.concurrent("bundler", () => { "/foo.js": `exports.foo = 123`, }, run: { - stdout: '{"foo":123} 123 234', + stdout: '{"default":{"foo":123},"foo":123} 123 234', }, }); itBundled("importstar/ImportStarCommonJSNoCapture", { diff --git a/test/bundler/esbuild/importstar_ts.test.ts b/test/bundler/esbuild/importstar_ts.test.ts index 35ce54499d81..28dc5c44c8d2 100644 --- a/test/bundler/esbuild/importstar_ts.test.ts +++ b/test/bundler/esbuild/importstar_ts.test.ts @@ -175,7 +175,7 @@ describe("bundler", () => { `, "/foo.ts": `exports.foo = 123`, }, - run: { stdout: '{"foo":123} 123 234' }, + run: { stdout: '{"default":{"foo":123},"foo":123} 123 234' }, }); itBundled("importstar_ts/CommonJSNoCapture", { files: { From e5021eb9974fc5a9135aee48e91822cd3d5996b3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 06:03:40 +0000 Subject: [PATCH 2/7] [autofix.ci] apply automated fixes --- src/bundler/LinkerContext.rs | 6 +++++- src/bundler/linker_context/scanImportsAndExports.rs | 5 +++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index 3acd0a03cca8..eb8cee6190a8 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -4397,7 +4397,11 @@ impl<'a> LinkerContext<'a> { /// Is `namespace_ref` of file `source_index` the `import * as ns` that the /// parser made out of a `require()` call (`unwrap_commonjs_to_esm`)? Such a /// call returns `module.exports`, not the namespace. - fn import_star_was_require_call(&self, source_index: crate::IndexInt, namespace_ref: Ref) -> bool { + fn import_star_was_require_call( + &self, + source_index: crate::IndexInt, + namespace_ref: Ref, + ) -> bool { let parts = self.graph.ast.items_parts()[source_index as usize].as_slice(); self.graph .top_level_symbol_to_parts(source_index, namespace_ref) diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index e80a7c5dcc8a..8b3889fd21b8 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -325,8 +325,9 @@ pub(crate) fn scan_imports_and_exports( let has_default_alias = record .flags .contains(ImportRecordFlags::CONTAINS_DEFAULT_ALIAS); - let has_import_star = - record.flags.contains(ImportRecordFlags::CONTAINS_IMPORT_STAR); + let has_import_star = record + .flags + .contains(ImportRecordFlags::CONTAINS_IMPORT_STAR); if other_flags.contains(AstFlags::FORCE_CJS_TO_ESM) && ((has_default_alias && !is_lifted) || ((has_default_alias || has_import_star) From 95469320f478dc8bb8b57983805047c365be6f72 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 8 Sep 2026 10:13:49 +0000 Subject: [PATCH 3/7] bundler: a split import() of a lifted module with `exports.__esModule` resolves `default` to `module.exports` The chunk for a split `import()` of a lifted CommonJS module exported the lifted `exports.default` as `default` when the module also assigns `exports.__esModule`, for every importer. Node, esbuild and the build without `--splitting` give `module.exports` to an importer that is an ES module by type. The chunk now always exports `module.exports` as `default`, and an importer that is not an ES module by type unwraps `exports.default` from it through `__toESM(m.default)`, the same per importer rule the unsplit build applies. Also shortens the comments the previous commit added. --- docs/bundler/index.mdx | 2 +- src/bundler/LinkerContext.rs | 36 +++++++++------- src/bundler/LinkerGraph.rs | 7 +--- .../findAllImportedPartsInJSOrder.rs | 3 +- .../generateCodeForFileInChunkJS.rs | 4 +- .../linker_context/scanImportsAndExports.rs | 41 ++++++++++-------- test/bundler/bundler_cjs2esm.test.ts | 42 ++++++++++++++++--- 7 files changed, 86 insertions(+), 49 deletions(-) diff --git a/docs/bundler/index.mdx b/docs/bundler/index.mdx index eebf767317b8..900d8e4473ee 100644 --- a/docs/bundler/index.mdx +++ b/docs/bundler/index.mdx @@ -1415,7 +1415,7 @@ In each case the member access compiles to a direct reference to `object`, the ` Assignments (`z.x = 1`), optional chains, and non-literal computed keys (`z[key]`) are left as property accesses; `z["object"]` is treated like `z.object`. `export default someImport` is followed only when it ends at a namespace; a default that snapshots a `let` export keeps snapshot semantics. -The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` of a module that does not set both resolves to that same `module.exports` object as `default`, with or without code splitting. +The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. ### deprecatedNamespaceObjectSetters diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index eb8cee6190a8..ac9c5da77395 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -3766,11 +3766,7 @@ impl<'a> LinkerContext<'a> { }; } - // The default import of a lifted CommonJS module is `module.exports`, which - // `exports_foo` stands in for. So are `ns.default` on `import * as ns` (a - // generated item) and a `require()` that `unwrap_commonjs_to_esm` turned - // into an import star: bind them to that object. A `.default` read off - // such a `require()` is `exports.default`, an ordinary export. + // A default import, `ns.default` and an unwrapped `require()` are `module.exports`. if is_import_stmt && flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) && if alias_is_star { @@ -4394,9 +4390,7 @@ impl<'a> LinkerContext<'a> { ref_.is_valid() && ref_ == self.lifted_namespace_ref(source_index) } - /// Is `namespace_ref` of file `source_index` the `import * as ns` that the - /// parser made out of a `require()` call (`unwrap_commonjs_to_esm`)? Such a - /// call returns `module.exports`, not the namespace. + /// An `import *` that `unwrap_commonjs_to_esm` made out of a `require()` call. fn import_star_was_require_call( &self, source_index: crate::IndexInt, @@ -4409,6 +4403,23 @@ impl<'a> LinkerContext<'a> { .any(|&part| parts[part as usize].tag == bun_ast::PartTag::ImportToConvertFromRequire) } + /// A split `import()` whose chunk exports `module.exports` as `default`: unwrap `exports.default`. + pub(crate) fn split_import_of_lifted_module_needs_to_esm( + &self, + importer: crate::IndexInt, + source_index: crate::IndexInt, + ) -> bool { + let id = source_index as usize; + self.graph.ast.items_flags()[id].contains(AstFlags::COMMONJS_LIFTED_TO_ESM) + && self.graph.ast.items_exports_kind()[id] != ExportsKind::Cjs + && self.graph.files.items_entry_point_kind()[id] + != crate::EntryPoint::Kind::UserSpecified + && Self::lifted_default_import_needs_wrapper( + self.graph.ast.items_module_type()[importer as usize], + &self.graph.ast.items_named_exports()[id], + ) + } + /// The default import of a lifted module that sets `__esModule` and exports /// `default` depends on the flag's run-time value, unless the importer is an /// ES module by type (Node ignores the flag). @@ -4736,8 +4747,7 @@ impl<'a> LinkerContext<'a> { name: bun_ast::StoreStr, count: u32, is_call_target: bool, - /// `X.default` where `X` is the `import *` namespace of a lifted CommonJS - /// module: `module.exports`, which `exports_foo` stands in for. + /// `X.default` on the `import *` namespace of a lifted CommonJS module. is_module_exports: bool, } @@ -5053,11 +5063,7 @@ impl<'a> LinkerContext<'a> { } } - /// Declares that namespace, `var import_foo = __toESM(exports_foo, 1)`, in a - /// part of its own, so it is dropped unless an importer uses the namespace - /// as a value. Mode `1` makes `default` the `module.exports` object: an - /// importer that is not an ES module by type got a CommonJS wrapper instead - /// when `__esModule` would change that (`lifted_default_import_needs_wrapper`). + /// Declares `var import_foo = __toESM(exports_foo, 1)` in a removable part of its own. pub(crate) fn create_lifted_namespace_part( &mut self, source_index: crate::IndexInt, diff --git a/src/bundler/LinkerGraph.rs b/src/bundler/LinkerGraph.rs index b2e5adc41871..da23a36dfe49 100644 --- a/src/bundler/LinkerGraph.rs +++ b/src/bundler/LinkerGraph.rs @@ -154,14 +154,11 @@ pub mod js_meta { } pub use crate::WrapKind as Wrap; - /// The `import *` namespace of a CommonJS module whose exports were lifted - /// to ESM. `exports_foo` stands in for `module.exports`, so the namespace is - /// a second object, `var import_foo = __toESM(exports_foo, 1)`, whose - /// `default` is `exports_foo`. Unset (`ref_` is `Ref::NONE`) for other files. + /// The `import *` namespace of a lifted CommonJS module, `__toESM(exports_foo, 1)`. #[derive(Clone, Copy)] pub struct LiftedNamespace { pub ref_: Ref, - /// The part that declares `ref_`. It prints with the namespace export part. + /// Declares `ref_`; prints with the namespace export part. `u32::MAX` when unset. pub part_index: u32, } impl Default for LiftedNamespace { diff --git a/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs b/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs index 9dcfbf839f14..37db9f3df78d 100644 --- a/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs +++ b/src/bundler/linker_context/findAllImportedPartsInJSOrder.rs @@ -306,8 +306,7 @@ impl<'a, 'ctx> FindImportedPartsVisitor<'a, 'ctx> { can_be_split, } => { let part = &self.parts[source_index as usize].as_slice()[part_index as usize]; - // The namespace export part was added on `Enter`, and the lifted - // namespace part prints with it. + // Both were handled on `Enter`: the lifted namespace part prints with part 0. if can_be_split && part_index != bun_ast::NAMESPACE_EXPORT_PART_INDEX && part_index diff --git a/src/bundler/linker_context/generateCodeForFileInChunkJS.rs b/src/bundler/linker_context/generateCodeForFileInChunkJS.rs index 9078e5311ef0..0aa263438f5e 100644 --- a/src/bundler/linker_context/generateCodeForFileInChunkJS.rs +++ b/src/bundler/linker_context/generateCodeForFileInChunkJS.rs @@ -313,9 +313,7 @@ pub fn generate_code_for_file_in_chunk_js<'r, 'src>( return PrintResult::Err(err.into()); } - // `var import_foo = __toESM(exports_foo, 1)` of a lifted CommonJS module goes - // right after `__exportCjs(exports_foo, ...)`: outside the wrapper, and ahead - // of the module's dependencies, which may read it back in a cycle. + // Right after `__exportCjs(...)`: outside the wrapper, before dependencies in a cycle. if lifted_namespace_part_index != u32::MAX && parts_live.is_set(lifted_namespace_part_index as usize) { diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index 8b3889fd21b8..45cd59ffdc99 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -247,16 +247,20 @@ pub(crate) fn scan_imports_and_exports( .get(&(import_record_index as u32)) { None => col!(dyn_ref_aliases)[other_file].merge_all(), - // `default` of a lifted CommonJS module is its namespace. + // `default` of a lifted CommonJS module is its `module.exports`. Some(dynamic_use) if record.kind == ImportKind::Dynamic && other_flags .contains(AstFlags::COMMONJS_LIFTED_TO_ESM) - && dynamic_use + && (dynamic_use .aliases .slice() .iter() - .any(|alias| alias.slice() == b"default") => + .any(|alias| alias.slice() == b"default") + || LinkerContext::lifted_default_import_needs_wrapper( + col_ref!(module_types)[id], + &col_ref!(named_exports)[other_file], + )) => { col!(dyn_ref_aliases)[other_file].merge_all() } @@ -317,10 +321,7 @@ pub(crate) fn scan_imports_and_exports( col!(flags)[other_file].wrap = WrapKind::Cjs; } - // A default import of a lifted CommonJS module binds to its - // `module.exports` object (`advance_import_tracker`), and so does - // `default` of its `import *` namespace, unless `__esModule` has - // to be checked at run time. + // Keep the wrapper when `__esModule` decides `default` at run time (`advance_import_tracker`). let is_lifted = other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM); let has_default_alias = record .flags @@ -373,12 +374,17 @@ pub(crate) fn scan_imports_and_exports( let exports = &col_ref!(named_exports)[other_file]; let user_entry = col_ref!(entry_point_kinds)[other_file] == EntryPoint::Kind::UserSpecified; - // `__esModule` makes `exports.default` the `default`, as in `bun run`. - let default_is_module_exports = !exports.contains(b"default") - || (other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) - && !user_entry - && !exports.contains(b"__esModule")); + let is_lifted = other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) + && !user_entry; + let default_is_module_exports = + !exports.contains(b"default") || is_lifted; + // `__toESM(m.default)` reads it when `__esModule` decides (step 6). let default_is_read = user_entry + || (is_lifted + && LinkerContext::lifted_default_import_needs_wrapper( + col_ref!(module_types)[id], + exports, + )) || col_ref!(dynamic_import_aliases)[id] .get(&(import_record_index as u32)) .is_none_or(|dynamic_use| { @@ -525,10 +531,7 @@ pub(crate) fn scan_imports_and_exports( // Also add a special export so import stars can bind to it. This must be // done in this step because it must come after CommonJS module discovery // but before matching imports with exports. - // - // `exports_foo` of a lifted CommonJS module stands in for `module.exports`, - // which default imports bind to. Import stars bind to a second object whose - // `default` is `exports_foo`, like the namespace Node gives such a module. + // For a lifted CommonJS module: `import_foo`, not `exports_foo` (`module.exports`). let mut namespace_ref = col_ref!(exports_refs)[id]; if id < col_ref!(import_records_list).len() && col_ref!(css_asts)[id].is_none() @@ -1187,8 +1190,12 @@ pub(crate) fn scan_imports_and_exports( // For other cases (static imports, truly external), use standard wrapping. if rec_source_index.is_valid() && is_external_dyn - && col_ref!(exports_kind)[rec_source_index.get() as usize] + && (col_ref!(exports_kind)[rec_source_index.get() as usize] == ExportsKind::Cjs + || this.split_import_of_lifted_module_needs_to_esm( + source_index, + rec_source_index.get(), + )) { // Cross-chunk dynamic import to CJS - needs special handling in printer col!(import_records_list)[id].as_mut_slice() diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 9cb819d8c0d4..fc34df7f3184 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -1835,22 +1835,52 @@ describe("bundler", () => { }, run: { file: "/out/entry.js", stdout: "true d d 1" }, }); + // With `exports.__esModule` set, the chunk still exports `module.exports` as + // `default`. An importer that is an ES module by type reads it as is, as Node + // does. Any other importer unwraps `exports.default` from it through `__toESM`, + // as `bun run` does. Both match the build without `--splitting`. + const esModuleLiftedLib = { + "/lib.cjs": /* js */ ` + exports.__esModule = true; + exports.default = "d"; + exports.x = 1; + `, + }; itBundled("cjs2esm/SplitDynamicImportOfLiftedCommonJSWithEsModuleAndDefault", { files: { "/entry.mjs": /* js */ ` + import lib from "./lib.cjs"; const m = await import("./lib.cjs"); - console.log(m.default, m.x); + console.log(typeof m.default, m.default === lib, m.default.default, m.x, m.__esModule, Object.keys(m).join(",")); `, - "/lib.cjs": /* js */ ` - exports.__esModule = true; - exports.default = "d"; - exports.x = 1; + ...esModuleLiftedLib, + }, + outdir: "/out", + outputPaths: ["/out/entry.js"], + splitting: true, + onAfterBundle(api) { + expect(splitChunk(api, "lib")).toContain("export default exports_lib;"); + api.expectFile("/out/entry.js").not.toContain("__toESM"); + }, + run: { file: "/out/entry.js", stdout: "object true d 1 true __esModule,default,x" }, + }); + itBundled("cjs2esm/SplitDynamicImportOfLiftedCommonJSWithEsModuleAndDefaultFromCjsImporter", { + files: { + "/entry.js": /* js */ ` + import { x } from "./lib.cjs"; + const m = await import("./lib.cjs"); + console.log(m.default, m.x, x, m.__esModule, Object.keys(m).join(",")); `, + ...esModuleLiftedLib, }, outdir: "/out", outputPaths: ["/out/entry.js"], splitting: true, - run: { file: "/out/entry.js", stdout: "d 1" }, + onAfterBundle(api) { + expect(splitChunk(api, "lib")).toContain("export default exports_lib;"); + api.expectFile("/out/entry.js").toContain(".then((m)=>__toESM(m.default))"); + }, + run: { file: "/out/entry.js", stdout: "d 1 1 true __esModule,default,x" }, }); // Static imports of a lifted module bind its exports directly, whether or // not a split `import()` of the module reads `default`. Only a read of From 5cb06b758f0b45194fb373e220b90b09e1cb8de0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 9 Sep 2026 05:26:47 +0000 Subject: [PATCH 4/7] test: check lifted CommonJS namespaces against bun run across importer shapes and flags; pin the user entry point shape (#12463) --- docs/bundler/index.mdx | 2 +- src/bundler/LinkerContext.rs | 2 +- .../linker_context/scanImportsAndExports.rs | 1 + test/bundler/bundler_cjs2esm.test.ts | 133 +++++++++++++++++- 4 files changed, 134 insertions(+), 4 deletions(-) diff --git a/docs/bundler/index.mdx b/docs/bundler/index.mdx index 900d8e4473ee..6a648e952c50 100644 --- a/docs/bundler/index.mdx +++ b/docs/bundler/index.mdx @@ -1415,7 +1415,7 @@ In each case the member access compiles to a direct reference to `object`, the ` Assignments (`z.x = 1`), optional chains, and non-literal computed keys (`z[key]`) are left as property accesses; `z["object"]` is treated like `z.object`. `export default someImport` is followed only when it ends at a namespace; a default that snapshots a `let` export keeps snapshot semantics. -The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. +The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. A lifted CommonJS file that is itself an entry point of the build is the other exception: its output file exports its own `exports.default` as `default`, and so does an `import()` of it. ### deprecatedNamespaceObjectSetters diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index ac9c5da77395..5031cdba6209 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -4403,7 +4403,7 @@ impl<'a> LinkerContext<'a> { .any(|&part| parts[part as usize].tag == bun_ast::PartTag::ImportToConvertFromRequire) } - /// A split `import()` whose chunk exports `module.exports` as `default`: unwrap `exports.default`. + /// A split `import()` whose chunk exports `module.exports` as `default` (not a user entry, #12463). pub(crate) fn split_import_of_lifted_module_needs_to_esm( &self, importer: crate::IndexInt, diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index 45cd59ffdc99..16d185ef0a25 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -372,6 +372,7 @@ pub(crate) fn scan_imports_and_exports( && this.is_external_dynamic_import(record, id as u32) { let exports = &col_ref!(named_exports)[other_file]; + // A user entry point keeps its own export list, `exports.default` as `default` (#12463). let user_entry = col_ref!(entry_point_kinds)[other_file] == EntryPoint::Kind::UserSpecified; let is_lifted = other_flags.contains(AstFlags::COMMONJS_LIFTED_TO_ESM) diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index fc34df7f3184..8a8a3344c238 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -1,5 +1,7 @@ -import { describe, expect } from "bun:test"; -import { readdirSync } from "node:fs"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; import { itBundled, type BundlerTestBundleAPI } from "./expectBundled"; const fakeReactNodeModules = { @@ -1882,6 +1884,31 @@ describe("bundler", () => { }, run: { file: "/out/entry.js", stdout: "d 1 1 true __esModule,default,x" }, }); + // A lifted CommonJS file that is itself an entry point of the build keeps its + // own export list: `default` is its `exports.default`, for the output file and + // so for a split `import()` of it too. Node would give `module.exports`; that + // shape is #12463's to change, so this pins today's output. + itBundled("cjs2esm/SplitDynamicImportOfLiftedCommonJSUserEntryPoint", { + files: { + "/entry.mjs": /* js */ ` + import lib from "./lib.cjs"; + const m = await import("./lib.cjs"); + console.log(typeof m.default, m.default === lib, lib.default, m.x, Object.keys(m).join(",")); + `, + "/lib.cjs": /* js */ ` + exports.default = "d"; + exports.x = 1; + `, + }, + entryPoints: ["/entry.mjs", "/lib.cjs"], + outdir: "/out", + outputPaths: ["/out/entry.js", "/out/lib.js"], + splitting: true, + onAfterBundle(api) { + api.expectFile("/out/lib.js").toContain("$default as default"); + }, + run: { file: "/out/entry.js", stdout: "string false d 1 default,x" }, + }); // Static imports of a lifted module bind its exports directly, whether or // not a split `import()` of the module reads `default`. Only a read of // `default` creates the namespace object. @@ -2570,3 +2597,105 @@ describe("bundler", () => { }, }); }); + +// Every way an ES module reaches a lifted CommonJS module must print what +// `bun run` prints for the unbundled sources (which is also what Node prints, +// `exports.__esModule` aside). Each cell bundles one module shape, one importer +// and one set of flags and compares the two runs, so no expectation here is +// hand-written. `lifted` says whether that cell keeps the module lifted; an +// unsplit `import()` puts it back into a CommonJS wrapper. +describe("cjs2esm/LiftedNamespaceMatchesBunRun", () => { + const fn = `function () { return typeof this + ":" + (this && this.named); }`; + const libs: Record = { + ExportsDefault: `exports.default = { tag: "D" };\nexports.named = "n";\nexports.fn = ${fn};\n`, + NoDefault: `exports.named = "n";\nexports.fn = ${fn};\n`, + ModuleExportsProps: `module.exports.named = "n";\nmodule.exports.default = "d";\nmodule.exports.fn = ${fn};\n`, + }; + const mid = + `import * as inner from "./lib.cjs";\nexport { inner };\n` + + `export * as outer from "./lib.cjs";\nexport { default as viaMid } from "./lib.cjs";\n`; + const probe = (dynamic: boolean) => /* js */ ` + import d, { default as d2, named } from "./lib.cjs"; + import * as ns from "./lib.cjs"; + import { inner, outer, viaMid } from "./mid.mjs"; + const dyn = ${dynamic ? `await import("./lib.cjs")` : `null`}; + const key = "def" + "ault"; + const stable = (v) => + JSON.stringify(v, (k, val) => + val && typeof val === "object" && !Array.isArray(val) + ? Object.fromEntries(Object.keys(val).sort().map(k2 => [k2, val[k2]])) + : val, + ); + console.log( + stable({ + dIsNsDefault: d === ns.default, + d2IsD: d2 === d, + viaMidIsD: viaMid === d, + nsIsD: ns === d, + computedIsD: ns[key] === d, + innerIsNs: inner === ns, + outerIsNs: outer === ns, + innerDefaultIsD: inner.default === d, + typeofDDefault: typeof d.default, + typeofNsDefault: typeof ns.default, + nsDefaultDefaultIsDDefault: ns.default.default === d.default, + nsKeys: Object.keys(ns).sort(), + dKeys: Object.keys(d), + ns: stable(ns), + d: JSON.stringify(d), + named, + nsNamed: ns.named, + outerNamed: outer.named, + nsFn: ns.fn(), + dFn: d.fn(), + hasDefault: "default" in ns, + dyn: dyn && { + defaultIsD: dyn.default === d, + keys: Object.keys(dyn).sort(), + json: stable(dyn), + named: dyn.named, + }, + }), + ); + `; + const run = (cwd: string, ...args: string[]) => { + const { stdout, stderr, exitCode } = Bun.spawnSync({ + cmd: [bunExe(), ...args], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + return { stdout: stdout.toString(), stderr: stderr.toString(), exitCode }; + }; + const cells: { dynamic: boolean; flags: string[]; lifted: boolean }[] = [ + { dynamic: false, flags: [], lifted: true }, + { dynamic: false, flags: ["--minify"], lifted: true }, + { dynamic: false, flags: ["--splitting"], lifted: true }, + { dynamic: true, flags: ["--splitting"], lifted: true }, + { dynamic: true, flags: [], lifted: false }, + ]; + for (const [libName, lib] of Object.entries(libs)) { + for (const { dynamic, flags, lifted } of cells) { + const name = `${libName} ${dynamic ? "static+dynamic" : "static"} ${flags.join(" ")}`.trim(); + test.concurrent(name, () => { + using dir = tempDir("cjs2esm-oracle", { "lib.cjs": lib, "mid.mjs": mid, "entry.mjs": probe(dynamic) }); + const cwd = String(dir); + const expected = run(cwd, "entry.mjs"); + expect(expected.stderr).toBe(""); + expect(expected.exitCode).toBe(0); + const build = run(cwd, "build", "./entry.mjs", "--outdir=out", "--entry-naming=[name].mjs", ...flags); + expect(build.stderr).not.toContain("error"); + expect(build.exitCode).toBe(0); + const out = readdirSync(join(cwd, "out")) + .map(f => readFileSync(join(cwd, "out", f), "utf8")) + .join("\n"); + expect(out.includes("__commonJS(")).toBe(!lifted); + const actual = run(cwd, join("out", "entry.mjs")); + expect(actual.stderr).toBe(""); + expect(JSON.parse(actual.stdout)).toEqual(JSON.parse(expected.stdout)); + expect(actual.exitCode).toBe(0); + }); + } + } +}); From d93da0bedda98c171f97a8521fa6ca5b7cf0d176 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:40:33 +0000 Subject: [PATCH 5/7] docs: table of what default means for a lifted CommonJS module --- docs/bundler/index.mdx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/bundler/index.mdx b/docs/bundler/index.mdx index 6a648e952c50..4ecae9422f60 100644 --- a/docs/bundler/index.mdx +++ b/docs/bundler/index.mdx @@ -1417,6 +1417,17 @@ Assignments (`z.x = 1`), optional chains, and non-literal computed keys (`z[key] The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. A lifted CommonJS file that is itself an entry point of the build is the other exception: its output file exports its own `exports.default` as `default`, and so does an `import()` of it. +In short, for a lifted CommonJS module `dep`: + +| How `dep` is reached | What you get | +| -------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | +| `import d from "./dep"`, `import { default as d }`, `export { default } from` | `module.exports` | +| `import * as ns` / `export * as ns from` | a namespace object: `ns.default` is `module.exports`, then the named exports; `ns.x` binds the lifted `x` | +| `import("./dep")`, same chunk or split chunk | that namespace shape: `.default` is `module.exports` | +| `require("./dep")` | `module.exports` | +| any of the above from an importer that is not an ES module by type, when `dep` assigns both `exports.__esModule` and `exports.default` | `default` is `exports.default`, as `bun run` gives it: static imports keep the CommonJS wrapper, a split `import()` unwraps it | +| `dep` is itself an entry point of the build | its output file, and an `import()` of it, export `exports.default` as `default` (#12463) | + ### deprecatedNamespaceObjectSetters Default `true`. When a namespace object does have to be created, each property currently gets a getter and a setter; the setter accepts `ns.foo = value` without throwing (reads still return the module's binding). Set this to `false` to emit getter-only namespace objects, which is what a future Bun release will do unconditionally. The `module.exports` object of a lifted CommonJS module is not affected: its setters assign the lifted bindings either way. From f41ef2018d90004fdd9e44ddc857a26e0e05e894 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:52:27 +0000 Subject: [PATCH 6/7] docs: spell out which importers are ES modules by type --- docs/bundler/index.mdx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/bundler/index.mdx b/docs/bundler/index.mdx index 4ecae9422f60..863cdae5e30e 100644 --- a/docs/bundler/index.mdx +++ b/docs/bundler/index.mdx @@ -1415,18 +1415,18 @@ In each case the member access compiles to a direct reference to `object`, the ` Assignments (`z.x = 1`), optional chains, and non-literal computed keys (`z[key]`) are left as property accesses; `z["object"]` is treated like `z.object`. `export default someImport` is followed only when it ends at a namespace; a default that snapshots a `let` export keeps snapshot semantics. -The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (`.mjs`, `.mts`, or `"type": "module"`), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. A lifted CommonJS file that is itself an entry point of the build is the other exception: its output file exports its own `exports.default` as `default`, and so does an `import()` of it. +The same applies to the default import of a CommonJS module whose `exports.x = ...` assignments the bundler lifted to ES module exports, such as `react` and `scheduler`. The default import of a CommonJS module is its `module.exports` object, so `import React from "react"; React.useState()` compiles to a direct call of the lifted `useState` binding. The `module.exports` object is created only when `React` itself is used as a value, and it lists the exports in assignment order. A write through it, `React.useLayoutEffect = React.useEffect`, assigns the lifted binding, so every importer sees the new value, the same as a write to `module.exports`. `React.default` is the lifted `default` export when the module has one, and otherwise `undefined`. `import * as ns` gives a separate namespace object, as in Node: `ns.default` is that `module.exports` object (the value a default import binds), followed by the named exports, and `ns.useState` still compiles to the lifted binding. A module that sets both `exports.__esModule` and `exports.default` keeps its CommonJS wrapper when the importer is not an ES module by type (that is, not a `.mjs` or `.mts` file and not in a `"type": "module"` package), because the default import and `ns.default` then depend on that flag at run time. An `import()` resolves to that same `module.exports` object as `default`, with or without code splitting, except that an importer that is not an ES module by type gets `exports.default` from a module that sets both. A lifted CommonJS file that is itself an entry point of the build is the other exception: its output file exports its own `exports.default` as `default`, and so does an `import()` of it. In short, for a lifted CommonJS module `dep`: -| How `dep` is reached | What you get | -| -------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `import d from "./dep"`, `import { default as d }`, `export { default } from` | `module.exports` | -| `import * as ns` / `export * as ns from` | a namespace object: `ns.default` is `module.exports`, then the named exports; `ns.x` binds the lifted `x` | -| `import("./dep")`, same chunk or split chunk | that namespace shape: `.default` is `module.exports` | -| `require("./dep")` | `module.exports` | -| any of the above from an importer that is not an ES module by type, when `dep` assigns both `exports.__esModule` and `exports.default` | `default` is `exports.default`, as `bun run` gives it: static imports keep the CommonJS wrapper, a split `import()` unwraps it | -| `dep` is itself an entry point of the build | its output file, and an `import()` of it, export `exports.default` as `default` (#12463) | +| How `dep` is reached | What you get | +| ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | +| `import d from "./dep"`, `import { default as d }`, `export { default } from` | `module.exports` | +| `import * as ns` / `export * as ns from` | a namespace object: `ns.default` is `module.exports`, then the named exports; `ns.x` binds the lifted `x` | +| `import("./dep")`, same chunk or split chunk | that namespace shape: `.default` is `module.exports` | +| `require("./dep")` | `module.exports` | +| any of the above from an importer that is not an ES module by type (not `.mjs`, `.mts`, or `"type": "module"`), when `dep` assigns both `exports.__esModule` and `exports.default` | `default` is `exports.default`, as `bun run` gives it: static imports keep the CommonJS wrapper, a split `import()` unwraps it | +| `dep` is itself an entry point of the build | its output file, and an `import()` of it, export `exports.default` as `default` (#12463) | ### deprecatedNamespaceObjectSetters From ab35eebc5e55bd4968b2fadd039a7c3819c4a639 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:36:40 +0000 Subject: [PATCH 7/7] bundler: an unwrapped require() of a lifted module keeps it lifted whatever exports.__esModule says; run the oracle cells with async spawns --- src/bundler/LinkerContext.rs | 19 ++++++++++ .../linker_context/scanImportsAndExports.rs | 8 +++- test/bundler/bundler_cjs2esm.test.ts | 38 +++++++++++++++---- 3 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index c63158611633..9b7a5677095c 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -4405,6 +4405,25 @@ impl<'a> LinkerContext<'a> { .any(|&part| parts[part as usize].tag == bun_ast::PartTag::ImportToConvertFromRequire) } + /// The same question, asked of the import record instead of its namespace symbol. + pub(crate) fn record_is_unwrapped_require( + &self, + source_index: crate::IndexInt, + record_index: u32, + ) -> bool { + self.graph.ast.items_parts()[source_index as usize] + .as_slice() + .iter() + .any(|part| { + part.tag == bun_ast::PartTag::ImportToConvertFromRequire + && part.stmts.slice().iter().any(|stmt| { + stmt.data + .s_import() + .is_some_and(|import| import.import_record_index == record_index) + }) + }) + } + /// A split `import()` whose chunk exports `module.exports` as `default` (not a user entry, #12463). pub(crate) fn split_import_of_lifted_module_needs_to_esm( &self, diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index 16d185ef0a25..9cfea310ea4d 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -336,7 +336,13 @@ pub(crate) fn scan_imports_and_exports( && LinkerContext::lifted_default_import_needs_wrapper( col_ref!(module_types)[id], &col_ref!(named_exports)[other_file], - ))) + ) + // `require()` returns `module.exports` whatever `__esModule` says. + && (has_default_alias + || !this.record_is_unwrapped_require( + id as u32, + import_record_index as u32, + )))) { col!(exports_kind)[other_file] = ExportsKind::Cjs; col!(flags)[other_file].wrap = WrapKind::Cjs; diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index 8a8a3344c238..381fcd8c751d 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -2310,6 +2310,27 @@ describe("bundler", () => { cjs2esm: true, run: { stdout: 'true false {"__esModule":true,"default":{"m":"D"},"a":"a"} {"m":"D"} default,__esModule,a' }, }); + // A `require()` that the parser turned into an import (the module is in the + // `react` family) returns `module.exports` whatever `__esModule` says, so it + // keeps the module lifted; `.default` on it is `exports.default`. + itBundled("cjs2esm/UnwrappedRequireWithEsModuleStaysLifted", { + files: { + "/entry.js": /* js */ ` + const React = require("react"); + console.log(React.default.tag, React.a, React.__esModule, React === require("react"), Object.keys(React).join(",")); + `, + "/node_modules/react/package.json": /* json */ ` + { "name": "react", "version": "19.0.0", "main": "index.js" } + `, + "/node_modules/react/index.js": /* js */ ` + exports.__esModule = true; + exports.default = { tag: "D" }; + exports.a = "a"; + `, + }, + cjs2esm: true, + run: { stdout: "D a true true __esModule,default,a" }, + }); // For any other importer `ns.default` depends on the flag at run time, like a // default import does, so a star import keeps the CommonJS wrapper too. itBundled("cjs2esm/StarImportWithEsModuleFromCjsImporterKeepsWrapper", { @@ -2658,15 +2679,16 @@ describe("cjs2esm/LiftedNamespaceMatchesBunRun", () => { }), ); `; - const run = (cwd: string, ...args: string[]) => { - const { stdout, stderr, exitCode } = Bun.spawnSync({ + const run = async (cwd: string, ...args: string[]) => { + await using proc = Bun.spawn({ cmd: [bunExe(), ...args], cwd, env: bunEnv, stdout: "pipe", stderr: "pipe", }); - return { stdout: stdout.toString(), stderr: stderr.toString(), exitCode }; + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; }; const cells: { dynamic: boolean; flags: string[]; lifted: boolean }[] = [ { dynamic: false, flags: [], lifted: true }, @@ -2678,20 +2700,22 @@ describe("cjs2esm/LiftedNamespaceMatchesBunRun", () => { for (const [libName, lib] of Object.entries(libs)) { for (const { dynamic, flags, lifted } of cells) { const name = `${libName} ${dynamic ? "static+dynamic" : "static"} ${flags.join(" ")}`.trim(); - test.concurrent(name, () => { + test.concurrent(name, async () => { using dir = tempDir("cjs2esm-oracle", { "lib.cjs": lib, "mid.mjs": mid, "entry.mjs": probe(dynamic) }); const cwd = String(dir); - const expected = run(cwd, "entry.mjs"); + const [expected, build] = await Promise.all([ + run(cwd, "entry.mjs"), + run(cwd, "build", "./entry.mjs", "--outdir=out", "--entry-naming=[name].mjs", ...flags), + ]); expect(expected.stderr).toBe(""); expect(expected.exitCode).toBe(0); - const build = run(cwd, "build", "./entry.mjs", "--outdir=out", "--entry-naming=[name].mjs", ...flags); expect(build.stderr).not.toContain("error"); expect(build.exitCode).toBe(0); const out = readdirSync(join(cwd, "out")) .map(f => readFileSync(join(cwd, "out", f), "utf8")) .join("\n"); expect(out.includes("__commonJS(")).toBe(!lifted); - const actual = run(cwd, join("out", "entry.mjs")); + const actual = await run(cwd, join("out", "entry.mjs")); expect(actual.stderr).toBe(""); expect(JSON.parse(actual.stdout)).toEqual(JSON.parse(expected.stdout)); expect(actual.exitCode).toBe(0);