From 497a4d4818a2f3e57564ea3952010511cd08499d Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Tue, 20 Jan 2026 12:40:33 -0800 Subject: [PATCH] Fix duplicate exports when two entrypoints share symbols (#26089) ### What does this PR do? Fixes #5344 Fixes #6356 ### How did you verify your code works? Some test coverage --------- Co-authored-by: Claude Co-authored-by: Claude Bot --- src/ast/P.zig | 4 +- src/ast/Parser.zig | 1 + src/bundler/ParseTask.zig | 1 + src/bundler/linker_context/computeChunks.zig | 17 ++++-- .../computeCrossChunkDependencies.zig | 7 ++- .../linker_context/scanImportsAndExports.zig | 18 ++++++- src/js_printer.zig | 14 +++++ test/bundler/esbuild/splitting.test.ts | 25 ++++++++- test/regression/issue/5344.test.ts | 53 +++++++++++++++++++ 9 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 test/regression/issue/5344.test.ts diff --git a/src/ast/P.zig b/src/ast/P.zig index 718dc2cf93..449bacb691 100644 --- a/src/ast/P.zig +++ b/src/ast/P.zig @@ -6527,7 +6527,9 @@ pub fn NewParser_( break :brk p.hmr_api_ref; } - if (p.options.bundle and p.needsWrapperRef(parts.items)) { + // When code splitting is enabled, always create wrapper_ref to match esbuild behavior. + // Otherwise, use needsWrapperRef() to optimize away unnecessary wrappers. + if (p.options.bundle and (p.options.code_splitting or p.needsWrapperRef(parts.items))) { break :brk p.newSymbol( .other, std.fmt.allocPrint( diff --git a/src/ast/Parser.zig b/src/ast/Parser.zig index fdb60c1ab9..d84d10f64c 100644 --- a/src/ast/Parser.zig +++ b/src/ast/Parser.zig @@ -19,6 +19,7 @@ pub const Parser = struct { tree_shaking: bool = false, bundle: bool = false, + code_splitting: bool = false, package_version: string = "", macro_context: *MacroContextType() = undefined, diff --git a/src/bundler/ParseTask.zig b/src/bundler/ParseTask.zig index 0240f9e03e..6264f735d0 100644 --- a/src/bundler/ParseTask.zig +++ b/src/bundler/ParseTask.zig @@ -1219,6 +1219,7 @@ fn runWithSourceCode( } opts.tree_shaking = if (source.index.isRuntime()) true else transpiler.options.tree_shaking; + opts.code_splitting = transpiler.options.code_splitting; opts.module_type = task.module_type; task.jsx.parse = loader.isJSX(); diff --git a/src/bundler/linker_context/computeChunks.zig b/src/bundler/linker_context/computeChunks.zig index 2abc07404b..c2bb9d7fda 100644 --- a/src/bundler/linker_context/computeChunks.zig +++ b/src/bundler/linker_context/computeChunks.zig @@ -39,9 +39,18 @@ pub noinline fn computeChunks( entry_bits.set(entry_bit); const has_html_chunk = loaders[source_index] == .html; + + // For code splitting, entry point chunks should be keyed by ONLY the entry point's + // own bit, not the full entry_bits. This ensures that if an entry point file is + // reachable from other entry points (e.g., via re-exports), its content goes into + // a shared chunk rather than staying in the entry point's chunk. + // https://github.com/evanw/esbuild/blob/cd832972927f1f67b6d2cc895c06a8759c1cf309/internal/linker/linker.go#L3882 + var entry_point_chunk_bits = try AutoBitSet.initEmpty(this.allocator(), this.graph.entry_points.len); + entry_point_chunk_bits.set(entry_bit); + const js_chunk_key = brk: { if (code_splitting) { - break :brk try temp_allocator.dupe(u8, entry_bits.bytes(this.graph.entry_points.len)); + break :brk try temp_allocator.dupe(u8, entry_point_chunk_bits.bytes(this.graph.entry_points.len)); } else { // Force HTML chunks to always be generated, even if there's an identical JS file. break :brk try std.fmt.allocPrint(temp_allocator, "{f}", .{JSChunkKeyFormatter{ @@ -61,7 +70,7 @@ pub noinline fn computeChunks( .source_index = source_index, .is_entry_point = true, }, - .entry_bits = entry_bits.*, + .entry_bits = entry_point_chunk_bits, .content = .html, .output_source_map = SourceMap.SourceMapPieces.init(this.allocator()), .flags = .{ .is_browser_chunk_from_server_build = could_be_browser_target_from_server_build and ast_targets[source_index] == .browser }, @@ -90,7 +99,7 @@ pub noinline fn computeChunks( .source_index = source_index, .is_entry_point = true, }, - .entry_bits = entry_bits.*, + .entry_bits = entry_point_chunk_bits, .content = .{ .css = .{ .imports_in_chunk_in_order = order, @@ -117,7 +126,7 @@ pub noinline fn computeChunks( .source_index = source_index, .is_entry_point = true, }, - .entry_bits = entry_bits.*, + .entry_bits = entry_point_chunk_bits, .content = .{ .javascript = .{}, }, diff --git a/src/bundler/linker_context/computeCrossChunkDependencies.zig b/src/bundler/linker_context/computeCrossChunkDependencies.zig index 13ef6ad6b2..a21a22132a 100644 --- a/src/bundler/linker_context/computeCrossChunkDependencies.zig +++ b/src/bundler/linker_context/computeCrossChunkDependencies.zig @@ -35,6 +35,7 @@ pub fn computeCrossChunkDependencies(c: *LinkerContext, chunks: []Chunk) bun.OOM .entry_point_chunk_indices = c.graph.files.items(.entry_point_chunk_index), .imports_to_bind = c.graph.meta.items(.imports_to_bind), .wrapper_refs = c.graph.ast.items(.wrapper_ref), + .exports_refs = c.graph.ast.items(.exports_ref), .sorted_and_filtered_export_aliases = c.graph.meta.items(.sorted_and_filtered_export_aliases), .resolved_exports = c.graph.meta.items(.resolved_exports), .ctx = c, @@ -61,6 +62,7 @@ const CrossChunkDependencies = struct { entry_point_chunk_indices: []Index.Int, imports_to_bind: []RefImportData, wrapper_refs: []const Ref, + exports_refs: []const Ref, sorted_and_filtered_export_aliases: []const []const string, resolved_exports: []const ResolvedExports, ctx: *LinkerContext, @@ -190,6 +192,7 @@ const CrossChunkDependencies = struct { if (deps.symbols.getConst(target_ref).?.namespace_alias) |namespace_alias| { target_ref = namespace_alias.namespace_ref; } + if (comptime Environment.allow_assert) debug("Cross-chunk export: {s}", .{deps.symbols.get(target_ref).?.original_name}); @@ -198,11 +201,13 @@ const CrossChunkDependencies = struct { } // Ensure "exports" is included if the current output format needs it + // https://github.com/evanw/esbuild/blob/v0.27.2/internal/linker/linker.go#L1049-L1051 if (flags.force_include_exports_for_entry_point) { - imports.put(deps.wrapper_refs[chunk.entry_point.source_index], {}) catch unreachable; + imports.put(deps.exports_refs[chunk.entry_point.source_index], {}) catch unreachable; } // Include the wrapper if present + // https://github.com/evanw/esbuild/blob/v0.27.2/internal/linker/linker.go#L1053-L1056 if (flags.wrap != .none) { imports.put(deps.wrapper_refs[chunk.entry_point.source_index], {}) catch unreachable; } diff --git a/src/bundler/linker_context/scanImportsAndExports.zig b/src/bundler/linker_context/scanImportsAndExports.zig index b214f9e17f..b6bdb6bf0a 100644 --- a/src/bundler/linker_context/scanImportsAndExports.zig +++ b/src/bundler/linker_context/scanImportsAndExports.zig @@ -698,8 +698,22 @@ pub fn scanImportsAndExports(this: *LinkerContext) ScanImportsAndExportsError!vo record.flags.contains_default_alias or record.flags.contains_es_module_alias)) { - record.flags.wrap_with_to_esm = true; - to_esm_uses += 1; + // 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 (record.source_index.isValid() and + this.isExternalDynamicImport(record, source_index) and + exports_kind[record.source_index.get()] == .cjs) + { + // Cross-chunk dynamic import to CJS - needs special handling in printer + record.flags.wrap_with_to_esm = true; + to_esm_uses += 1; + } else if (kind != .dynamic) { + // Static imports to external CJS modules need __toESM wrapping + record.flags.wrap_with_to_esm = true; + to_esm_uses += 1; + } + // Dynamic imports to truly external modules: no wrapping (preserve native format) } } } diff --git a/src/js_printer.zig b/src/js_printer.zig index 250aa4ddef..9ba58ad1c5 100644 --- a/src/js_printer.zig +++ b/src/js_printer.zig @@ -1789,6 +1789,9 @@ fn NewPrinter( p.printSpaceBeforeIdentifier(); + // Wrap with __toESM if importing a CommonJS module + const wrap_with_to_esm = record.flags.wrap_with_to_esm; + // Allow it to fail at runtime, if it should if (module_type != .internal_bake_dev) { p.print("import("); @@ -1807,6 +1810,17 @@ fn NewPrinter( p.print(")"); + // For CJS modules, unwrap the default export and convert to ESM + if (wrap_with_to_esm) { + p.print(".then((m)=>"); + p.printSymbol(p.options.to_esm_ref); + p.print("(m.default"); + if (p.options.input_module_type == .esm) { + p.print(",1"); + } + p.print("))"); + } + // if (leading_interior_comments.len > 0) { // p.printNewline(); // p.unindent(); diff --git a/test/bundler/esbuild/splitting.test.ts b/test/bundler/esbuild/splitting.test.ts index 2f31caf0e2..937f2cb26f 100644 --- a/test/bundler/esbuild/splitting.test.ts +++ b/test/bundler/esbuild/splitting.test.ts @@ -279,7 +279,6 @@ describe("bundler", () => { }, }); itBundled("splitting/ReExportESBuildIssue273", { - todo: true, files: { "/a.js": `export const a = { value: 1 }`, "/b.js": `export { a } from './a'`, @@ -609,4 +608,28 @@ describe("bundler", () => { stdout: "42 true 42", }, }); + // Test that CJS modules with dynamic imports to other CJS entry points work correctly + // when code splitting causes the dynamically imported module to be in a separate chunk. + // The dynamic import should properly unwrap the default export using __toESM. + // Regression test for: dynamic import of CJS chunk returns { default: { __esModule, ... } } + // and needs .then((m)=>__toESM(m.default)) to unwrap correctly. + // Note: __esModule is required because bun optimizes simple CJS to ESM otherwise. + itBundled("splitting/CJSDynamicImportOfCJSChunk", { + files: { + "/main.js": /* js */ ` + import("./impl.js").then(mod => console.log(mod.foo())); + `, + "/impl.js": /* js */ ` + Object.defineProperty(exports, "__esModule", { value: true }); + exports.foo = () => "success"; + `, + }, + entryPoints: ["/main.js", "/impl.js"], + splitting: true, + outdir: "/out", + run: { + file: "/out/main.js", + stdout: "success", + }, + }); }); diff --git a/test/regression/issue/5344.test.ts b/test/regression/issue/5344.test.ts new file mode 100644 index 0000000000..c84441ab93 --- /dev/null +++ b/test/regression/issue/5344.test.ts @@ -0,0 +1,53 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// https://github.com/oven-sh/bun/issues/5344 +// When one entry point re-exports from another entry point with code splitting, +// the bundler was producing duplicate export statements. +test("code splitting with re-exports between entry points should not produce duplicate exports", async () => { + using dir = tempDir("issue-5344", { + "entry-a.ts": `export { b } from "./entry-b.ts"; export function a() {}`, + "entry-b.ts": `export function b() {}`, + }); + + const result = await Bun.build({ + entrypoints: [`${dir}/entry-a.ts`, `${dir}/entry-b.ts`], + outdir: `${dir}/dist`, + splitting: true, + }); + + expect(result.success).toBe(true); + expect(result.outputs.length).toBe(3); // entry-a.js, entry-b.js, chunk-*.js + + const entryB = result.outputs.find(o => o.path.endsWith("entry-b.js")); + expect(entryB).toBeDefined(); + + const entryBContent = await entryB!.text(); + + const exportMatches = entryBContent.match(/^export\s*\{/gm); + expect(exportMatches?.length).toBe(1); + + const entryAUrl = Bun.pathToFileURL(`${dir}/dist/entry-a.js`); + const entryBUrl = Bun.pathToFileURL(`${dir}/dist/entry-b.js`); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { a, b } from "${entryAUrl}"; + import { b as b2 } from "${entryBUrl}"; + console.log(typeof a, typeof b, b === b2); + `, + ], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toBe(""); + expect(stdout.trim()).toBe("function function true"); + expect(exitCode).toBe(0); +});