From 74e951bacbdabb64ae1f95ba98f801f2d5c93fce Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 2 Sep 2026 23:26:30 +0000 Subject: [PATCH] bundler: wrap a split import() with __toESM when the target is CommonJS at link time With code splitting, a cross-chunk import() skipped the __toESM wrap when the target had FORCE_CJS_TO_ESM. The parser sets that flag on each file in the unwrap list and on each file whose exports.foo = ... it lifted. Such a file can still be CommonJS at link time: a require() of it wraps it, it assigns module.exports, or the target of its lifted module.exports = require() is CommonJS. Its chunk is then `export default require_x()`, so the importer saw the bare chunk namespace and named exports were undefined. Remove the skip. Since #41231 it did nothing else. The branch below already wraps a cross-chunk import() when the target is CommonJS at link time, and leaves every other target alone. --- .../linker_context/scanImportsAndExports.rs | 134 ++++++++---------- test/bundler/bundler_cjs2esm.test.ts | 95 +++++++++++++ 2 files changed, 157 insertions(+), 72 deletions(-) diff --git a/src/bundler/linker_context/scanImportsAndExports.rs b/src/bundler/linker_context/scanImportsAndExports.rs index 98004f399c02..3340219416c7 100644 --- a/src/bundler/linker_context/scanImportsAndExports.rs +++ b/src/bundler/linker_context/scanImportsAndExports.rs @@ -1113,88 +1113,78 @@ pub(crate) fn scan_imports_and_exports( || !output_format.keep_es6_import_export_syntax() || kind == ImportKind::Dynamic { - if rec_source_index.is_valid() - && kind == ImportKind::Dynamic - && col_ref!(ast_flags_list)[other_id] - .contains(AstFlags::FORCE_CJS_TO_ESM) + // We should use "__require" instead of "require" if we're not + // generating a CommonJS output file, since it won't exist otherwise. + // An `import()` is printed as-is and never becomes `__require()`, + // nor does a split `require()` (`import.meta.require`). + if kind != ImportKind::Dynamic + && !is_external_dyn + && should_call_runtime_require(output_format) { - // The chunk of a module converted to ESM provides `default` itself. - continue; - } else { - // We should use "__require" instead of "require" if we're not - // generating a CommonJS output file, since it won't exist otherwise. - // An `import()` is printed as-is and never becomes `__require()`, - // nor does a split `require()` (`import.meta.require`). - if kind != ImportKind::Dynamic - && !is_external_dyn - && should_call_runtime_require(output_format) - { - runtime_require_uses += 1; - } + runtime_require_uses += 1; + } + + // A split `require()` whose target is CommonJS at link + // time (for example a lifted `module.exports = require()` + // file the linker wrapped again): the chunk exports only + // `default: module.exports`, so the printed call reads + // `.default` to return `module.exports`, the same value + // an unsplit `require()` returns. + if kind == ImportKind::Require + && is_external_dyn + && rec_source_index.is_valid() + && col_ref!(exports_kind)[rec_source_index.get() as usize] + == ExportsKind::Cjs + { + col!(import_records_list)[id].as_mut_slice() + [import_record_index as usize] + .flags + .insert(ImportRecordFlags::CROSS_CHUNK_REQUIRE_DEFAULT); + } - // A split `require()` whose target is CommonJS at link - // time (for example a lifted `module.exports = require()` - // file the linker wrapped again): the chunk exports only - // `default: module.exports`, so the printed call reads - // `.default` to return `module.exports`, the same value - // an unsplit `require()` returns. - if kind == ImportKind::Require + // If this wasn't originally a "require()" call, then we may need + // to wrap this in a call to the "__toESM" wrapper to convert from + // CommonJS semantics to ESM semantics. + // + // Unfortunately this adds some additional code since the conversion + // is somewhat complex. As an optimization, we can avoid this if the + // following things are true: + // + // - The import is an ES module statement (e.g. not an "import()" expression) + // - The ES module namespace object must not be captured + // - The "default" and "__esModule" exports must not be accessed + // + if kind != ImportKind::Require + && (kind != ImportKind::Stmt + || rec_flags.contains(ImportRecordFlags::CONTAINS_IMPORT_STAR) + || rec_flags + .contains(ImportRecordFlags::CONTAINS_DEFAULT_ALIAS) + || rec_flags + .contains(ImportRecordFlags::CONTAINS_ES_MODULE_ALIAS)) + { + // For dynamic imports to cross-chunk CJS modules, we need extra + // unwrapping in js_printer (.then((m)=>__toESM(m.default))). + // For other cases (static imports, truly external), use standard wrapping. + if rec_source_index.is_valid() && is_external_dyn - && rec_source_index.is_valid() && col_ref!(exports_kind)[rec_source_index.get() as usize] == ExportsKind::Cjs { + // Cross-chunk dynamic import to CJS - needs special handling in printer col!(import_records_list)[id].as_mut_slice() [import_record_index as usize] .flags - .insert(ImportRecordFlags::CROSS_CHUNK_REQUIRE_DEFAULT); - } - - // If this wasn't originally a "require()" call, then we may need - // to wrap this in a call to the "__toESM" wrapper to convert from - // CommonJS semantics to ESM semantics. - // - // Unfortunately this adds some additional code since the conversion - // is somewhat complex. As an optimization, we can avoid this if the - // following things are true: - // - // - The import is an ES module statement (e.g. not an "import()" expression) - // - The ES module namespace object must not be captured - // - The "default" and "__esModule" exports must not be accessed - // - if kind != ImportKind::Require - && (kind != ImportKind::Stmt - || rec_flags - .contains(ImportRecordFlags::CONTAINS_IMPORT_STAR) - || rec_flags - .contains(ImportRecordFlags::CONTAINS_DEFAULT_ALIAS) - || rec_flags - .contains(ImportRecordFlags::CONTAINS_ES_MODULE_ALIAS)) - { - // For dynamic imports to cross-chunk CJS modules, we need extra - // unwrapping in js_printer (.then((m)=>__toESM(m.default))). - // 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] - == ExportsKind::Cjs - { - // Cross-chunk dynamic import to CJS - needs special handling in printer - col!(import_records_list)[id].as_mut_slice() - [import_record_index as usize] - .flags - .insert(ImportRecordFlags::WRAP_WITH_TO_ESM); - to_esm_uses += 1; - } else if kind != ImportKind::Dynamic { - // Static imports to external CJS modules need __toESM wrapping - col!(import_records_list)[id].as_mut_slice() - [import_record_index as usize] - .flags - .insert(ImportRecordFlags::WRAP_WITH_TO_ESM); - to_esm_uses += 1; - } - // Dynamic imports to truly external modules: no wrapping (preserve native format) + .insert(ImportRecordFlags::WRAP_WITH_TO_ESM); + to_esm_uses += 1; + } else if kind != ImportKind::Dynamic { + // Static imports to external CJS modules need __toESM wrapping + col!(import_records_list)[id].as_mut_slice() + [import_record_index as usize] + .flags + .insert(ImportRecordFlags::WRAP_WITH_TO_ESM); + to_esm_uses += 1; } + // Dynamic imports of an ES module chunk or a truly external module: no wrapping } } continue; diff --git a/test/bundler/bundler_cjs2esm.test.ts b/test/bundler/bundler_cjs2esm.test.ts index bf4285cfb127..5d5f1b8f52e4 100644 --- a/test/bundler/bundler_cjs2esm.test.ts +++ b/test/bundler/bundler_cjs2esm.test.ts @@ -1550,6 +1550,101 @@ describe("bundler", () => { }, run: { file: "/out/entry.js", stdout: "true true true patched patched" }, }); + // A module in the unwrap list that assigns `module.exports` is not lifted and + // keeps its `__commonJS` wrapper, so its chunk is `export default require_x()`. + // A cross-chunk `import()` of it needs the same `__toESM` as any other + // CommonJS chunk, or the named exports are `undefined`. + itBundled("cjs2esm/DynamicImportSplittingOfWrappedCommonJS", { + files: { + "/entry.js": /* js */ ` + import React from "react"; + const m = await import("react"); + console.log(m.useState(1)[0], m.version, m.default === React, typeof m.default.useState); + `, + "/node_modules/react/package.json": /* json */ ` + { "name": "react", "version": "19.0.0", "main": "index.js" } + `, + "/node_modules/react/index.js": /* js */ ` + 'use strict'; + if (globalThis.USE_PROD) { + module.exports = require('./cjs/react.production.js'); + } else { + module.exports = require('./cjs/react.development.js'); + } + `, + "/node_modules/react/cjs/react.production.js": /* js */ ` + 'use strict'; + function useState(initial) { + return [initial, function setState() {}]; + } + exports.useState = useState; + exports.version = "production"; + `, + "/node_modules/react/cjs/react.development.js": /* js */ ` + 'use strict'; + function useState(initial) { + return [initial, function setState() {}]; + } + exports.useState = useState; + exports.version = "development"; + `, + }, + outdir: "/out", + splitting: true, + onAfterBundle(api) { + api.expectFile("/out/entry.js").toContain("__toESM(m.default"); + }, + run: { + file: "/out/entry.js", + stdout: "1 development true function", + }, + }); + // The same for a lifted module that the linker wraps again, because the + // target of its `module.exports = require()` stays CommonJS. + itBundled("cjs2esm/DynamicImportSplittingOfRewrappedLiftedCommonJS", { + files: { + "/entry.js": /* js */ ` + const m = await import("react-dom"); + console.log(m.version, m.default.version); + `, + "/node_modules/react-dom/index.js": /* js */ ` + console.log('side effect'); + module.exports = require('./impl'); + `, + "/node_modules/react-dom/impl.js": /* js */ ` + module.exports = function render() { return "rendered"; }; + module.exports.version = "19.0.0"; + `, + }, + outdir: "/out", + splitting: true, + minifySyntax: true, + run: { + file: "/out/entry.js", + stdout: "side effect\n19.0.0 19.0.0", + }, + }); + // Outside the unwrap list too: the `exports.foo = ...` of "./lib.js" are + // lifted, but a `require()` of it makes it CommonJS again. + itBundled("cjs2esm/DynamicImportSplittingOfRequiredLiftedCommonJS", { + files: { + "/entry.js": /* js */ ` + const lib = require("./lib.js"); + const m = await import("./lib.js"); + console.log(lib.foo, m.foo, m.default === lib); + `, + "/lib.js": /* js */ ` + exports.foo = "foo"; + exports.bar = "bar"; + `, + }, + outdir: "/out", + splitting: true, + run: { + file: "/out/entry.js", + stdout: "foo foo true", + }, + }); // `import()` of a lifted CommonJS module resolves to a view of its namespace // whose `default` is the namespace itself (`module.exports`), as in Node. // https://github.com/oven-sh/bun/issues/14061