mirror of
https://github.com/oven-sh/bun
synced 2026-02-16 13:51:47 +00:00
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 <noreply@anthropic.com> Co-authored-by: Claude Bot <claude-bot@bun.sh>
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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 = .{},
|
||||
},
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
53
test/regression/issue/5344.test.ts
Normal file
53
test/regression/issue/5344.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user