Fix banner positioning with --format=cjs --target=bun (#22641)

## Summary
- Fixes incorrect banner positioning when using `--banner` with
`--format=cjs` and `--target=bun`
- Ensures Bun-specific comments (`// @bun @bun-cjs`) appear before user
banner content
- Properly extracts and positions hashbangs from banner content

## Problem
When using `--banner` with `--format=cjs --target=bun`, the banner was
incorrectly placed before the `// @bun @bun-cjs` comment and CJS wrapper
function, breaking the module format that Bun expects.

## Solution
Implemented proper ordering:
1. **Hashbang** (from source file or extracted from banner if it starts
with `#!`)
2. **@bun comments** (e.g., `// @bun`, `// @bun @bun-cjs`, `// @bun
@bytecode`)
3. **CJS wrapper** `(function(exports, require, module, __filename,
__dirname) {`
4. **Banner content** (excluding any extracted hashbang)

## Test plan
- [x] Added comprehensive tests for banner positioning with CJS/ESM and
Bun target
- [x] Tests cover hashbang extraction from banners
- [x] Tests verify proper ordering with bytecode generation
- [x] All existing tests pass

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Jarred Sumner
2025-09-14 01:01:22 -07:00
committed by GitHub
parent 3d8139dc27
commit ecd23df4ca
6 changed files with 236 additions and 42 deletions

View File

@@ -1,4 +1,4 @@
import { describe } from "bun:test";
import { describe, expect } from "bun:test";
import { itBundled } from "./expectBundled";
describe("bundler", () => {
@@ -33,4 +33,174 @@ describe("bundler", () => {
api.expectFile("out.js").toContain('"use client";');
},
});
itBundled("banner/BannerWithCJSAndTargetBun", {
banner: "// Copyright 2024 Example Corp",
format: "cjs",
target: "bun",
backend: "api",
outdir: "/out",
minifyWhitespace: true,
files: {
"a.js": `module.exports = 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"// @bun @bun-cjs
(function(exports, require, module, __filename, __dirname) {// Copyright 2024 Example Corp
module.exports=1;})
"
`);
},
});
itBundled("banner/HashbangBannerWithCJSAndTargetBun", {
banner: "#!/usr/bin/env -S node --enable-source-maps\n// Additional banner content",
format: "cjs",
target: "bun",
backend: "api",
outdir: "/out",
minifyWhitespace: true,
files: {
"/a.js": `module.exports = 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"#!/usr/bin/env -S node --enable-source-maps
// @bun @bun-cjs
(function(exports, require, module, __filename, __dirname) {// Additional banner content
module.exports=1;})
"
`);
},
});
itBundled("banner/SourceHashbangWithBannerAndCJSTargetBun", {
banner: "// Copyright 2024 Example Corp",
format: "cjs",
target: "bun",
outdir: "/out",
minifyWhitespace: true,
backend: "api",
files: {
"/a.js": `#!/usr/bin/env node
module.exports = 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"#!/usr/bin/env node
// @bun @bun-cjs
(function(exports, require, module, __filename, __dirname) {// Copyright 2024 Example Corp
module.exports=1;})
"
`);
},
});
itBundled("banner/BannerWithESMAndTargetBun", {
banner: "// Copyright 2024 Example Corp",
format: "esm",
target: "bun",
backend: "api",
minifyWhitespace: true,
files: {
"/a.js": `export default 1;`,
},
onAfterBundle(api) {
const content = api.readFile("out.js");
// @bun comment should come first, then banner
const bunCommentIndex = content.indexOf("// @bun");
const bannerIndex = content.indexOf("// Copyright 2024 Example Corp");
expect(bunCommentIndex).toBe(0);
expect(bannerIndex).toBeGreaterThan(bunCommentIndex);
// No CJS wrapper in ESM format
expect(content).not.toContain("(function(exports, require, module, __filename, __dirname)");
expect(content).toMatchInlineSnapshot(`
"// @bun
// Copyright 2024 Example Corp
var a_default=1;export{a_default as default};
"
`);
},
});
itBundled("banner/HashbangBannerWithESMAndTargetBun", {
banner: "#!/usr/bin/env -S node --enable-source-maps\n// Additional banner content",
format: "esm",
target: "bun",
backend: "api",
outdir: "/out",
minifyWhitespace: true,
files: {
"/a.js": `export default 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"#!/usr/bin/env -S node --enable-source-maps
// @bun
// Additional banner content
var a_default=1;export{a_default as default};
"
`);
},
});
itBundled("banner/BannerWithBytecodeAndCJSTargetBun", {
banner: "// Copyright 2024 Example Corp",
format: "cjs",
target: "bun",
backend: "api",
bytecode: true,
minifyWhitespace: true,
outdir: "/out",
files: {
"/a.js": `module.exports = 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"// @bun @bytecode @bun-cjs
(function(exports, require, module, __filename, __dirname) {// Copyright 2024 Example Corp
module.exports=1;})
"
`);
// @bun @bytecode @bun-cjs comment should come first, then CJS wrapper, then banner
const bunBytecodeIndex = content.indexOf("// @bun @bytecode @bun-cjs");
const wrapperIndex = content.indexOf("(function(exports, require, module, __filename, __dirname) {");
const bannerIndex = content.indexOf("// Copyright 2024 Example Corp");
expect(bunBytecodeIndex).toBe(0);
expect(wrapperIndex).toBeGreaterThan(bunBytecodeIndex);
expect(bannerIndex).toBeGreaterThan(wrapperIndex);
},
});
itBundled("banner/HashbangBannerWithBytecodeAndCJSTargetBun", {
banner: "#!/usr/bin/env bun\n// Production build",
format: "cjs",
target: "bun",
bytecode: true,
backend: "api",
outdir: "/out",
minifyWhitespace: true,
files: {
"/a.js": `module.exports = 1;`,
},
onAfterBundle(api) {
const content = api.readFile("/out/a.js");
expect(content).toMatchInlineSnapshot(`
"#!/usr/bin/env bun
// @bun @bytecode @bun-cjs
(function(exports, require, module, __filename, __dirname) {// Production build
module.exports=1;})
"
`);
},
});
});

View File

@@ -237,6 +237,7 @@ describe("bundler", () => {
},
});
itBundled("naming/NonexistantRoot", ({ root }) => ({
backend: "cli",
files: {
"/src/entry.js": /* js */ `
import asset1 from "./asset1.file";

View File

@@ -820,12 +820,13 @@ describe("bundler", () => {
},
external: ["esbuild"],
entryPoints: ["./index.ts"],
backend: "api",
plugins(build) {
const opts = (build as any).initialOptions;
expect(opts.bundle).toEqual(true);
expect(opts.entryPoints).toEqual([join(root, "index.ts")]);
expect(opts.external).toEqual(["esbuild"]);
expect(opts.format).toEqual(undefined);
expect(opts.format).toEqual("esm");
expect(opts.minify).toEqual(false);
expect(opts.minifyIdentifiers).toEqual(undefined);
expect(opts.minifySyntax).toEqual(undefined);

View File

@@ -1043,7 +1043,12 @@ function expectBundled(
const buildConfig: BuildConfig = {
entrypoints: [...entryPaths, ...(entryPointsRaw ?? [])],
external,
banner,
format,
footer,
root: outbase,
packages,
loader,
minify: {
whitespace: minifyWhitespace,
identifiers: minifyIdentifiers,

View File

@@ -152,6 +152,7 @@ console.log(favicon);
// Test manifest with multiple HTML imports
itBundled("html-import/multiple-manifests", {
outdir: "out/",
backend: "cli",
files: {
"/server.js": `
import homeHtml from "./home.html";
@@ -301,6 +302,7 @@ console.log("About manifest:", aboutHtml);
// Test that import with {type: 'file'} still works as a file import
itBundled("html-import/with-type-file-attribute", {
outdir: "out/",
backend: "cli",
files: {
"/entry.js": `
import htmlUrl from "./page.html" with { type: 'file' };