Fix entry point check #25729
Fix entry point check #25729
Conversation
WalkthroughRefines chunk selection in Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
Can you add a test that fails with the system version of Bun and passes in the debug build in this branch?
I'll give it a try! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @test/bundler/bundler_html.test.ts:
- Around line 847-902: The test currently asserts the entry bundle contains
"index.js loaded" and that it does not contain "shared-value"; extend
onAfterBundle(api) to also verify that the shared module was emitted as a
separate chunk by scanning the output JS files (e.g., use api.readdir or list
out/*.js, skipping the index bundle found via jsMatch) and assert that at least
one other JS file contains "shared-value" (confirming a shared chunk was created
and the shared module wasn't dropped).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/bundler/bundler_html.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/bundler/bundler_html.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/bundler/bundler_html.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/bundler/bundler_html.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Learnt from: robobun
Repo: oven-sh/bun PR: 0
File: :0-0
Timestamp: 2025-10-13T06:54:58.043Z
Learning: Bun's HTML bundler only supports `.html` file extensions, not `.htm`. Adding `.htm` support would require a separate feature request.
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use string literal `require()` statements only; dynamic requires are not permitted
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never use `bun test` directly - always use `bun bd test` to run tests with debug build changes
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-12-16T00:21:32.179Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : For multi-file tests, prefer `tempDir` and `Bun.spawn` over single-file tests
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `-e` flag for single-file tests when spawning Bun processes
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer concurrent tests over sequential tests using `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` with files that end in `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Ensure V8 API tests compare identical C++ code output between Node.js and Bun through the test suite validation process
Applied to files:
test/bundler/bundler_html.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner
Applied to files:
test/bundler/bundler_html.test.ts
🧬 Code graph analysis (1)
test/bundler/bundler_html.test.ts (1)
test/bundler/expectBundled.ts (1)
itBundled(1763-1797)
| // Test HTML bundler referencing correct JS chunk with code splitting | ||
| itBundled("html/code-splitting-dynamic-imports-wrong-chunk", { | ||
| files: { | ||
| "/index.html": ` | ||
| <!DOCTYPE html> | ||
| <html> | ||
| <head> | ||
| <script src="./index.js"></script> | ||
| </head> | ||
| <body> | ||
| <h1>Dynamic Import Test</h1> | ||
| </body> | ||
| </html>`, | ||
| "/index.js": ` | ||
| // 9 lazy imports, with the last two sharing a common module | ||
| import('./lazy1.js'); | ||
| import('./lazy2.js'); | ||
| import('./lazy3.js'); | ||
| import('./lazy4.js'); | ||
| import('./lazy5.js'); | ||
| import('./lazy6.js'); | ||
| import('./lazy7.js'); | ||
| import('./lazy8.js'); | ||
| import('./lazy9.js'); | ||
| console.log('index.js loaded'); | ||
| `, | ||
| "/lazy1.js": `console.log('lazy1');`, | ||
| "/lazy2.js": `console.log('lazy2');`, | ||
| "/lazy3.js": `console.log('lazy3');`, | ||
| "/lazy4.js": `console.log('lazy4');`, | ||
| "/lazy5.js": `console.log('lazy5');`, | ||
| "/lazy6.js": `console.log('lazy6');`, | ||
| "/lazy7.js": `console.log('lazy7');`, | ||
| "/lazy8.js": ` | ||
| import { shared } from './shared.js'; | ||
| console.log('lazy8 with shared:', shared); | ||
| `, | ||
| "/lazy9.js": ` | ||
| import { shared } from './shared.js'; | ||
| console.log('lazy9 with shared:', shared); | ||
| `, | ||
| "/shared.js": ` | ||
| export const shared = 'shared-value'; | ||
| `, | ||
| }, | ||
| entryPoints: ["/index.html"], | ||
| splitting: true, | ||
| outdir: "/out", | ||
| onAfterBundle(api) { | ||
| const htmlContent = api.readFile("out/index.html"); | ||
| const jsMatch = htmlContent.match(/src="(.*\.js)"/); | ||
| const jsBundle = api.readFile("out/" + jsMatch![1]); | ||
| expect(jsBundle).toContain("index.js loaded"); | ||
| expect(jsBundle).not.toContain("shared-value"); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Well-structured regression test for the code-splitting bug.
The test correctly reproduces the scenario from issue #24344 where code splitting would cause index.html to reference an incorrect chunk. The assertions verify both that the entry point content is present and that shared module content was correctly split out.
Optional enhancement: Verify shared chunk creation
Consider also verifying that a separate chunk was created for the shared module, not just that it's absent from index.js:
onAfterBundle(api) {
const htmlContent = api.readFile("out/index.html");
const jsMatch = htmlContent.match(/src="(.*\.js)"/);
const jsBundle = api.readFile("out/" + jsMatch![1]);
expect(jsBundle).toContain("index.js loaded");
expect(jsBundle).not.toContain("shared-value");
+
+ // Verify shared module was split into a separate chunk
+ const files = api.readdir("out");
+ const sharedChunk = files.find(f => f.endsWith('.js') && f !== jsMatch![1]);
+ expect(sharedChunk).toBeDefined();
+ const sharedBundle = api.readFile("out/" + sharedChunk!);
+ expect(sharedBundle).toContain("shared-value");
},This would provide stronger verification that code splitting worked correctly, though the current assertions are sufficient for catching the reported bug.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @test/bundler/bundler_html.test.ts around lines 847-902, The test currently
asserts the entry bundle contains "index.js loaded" and that it does not contain
"shared-value"; extend onAfterBundle(api) to also verify that the shared module
was emitted as a separate chunk by scanning the output JS files (e.g., use
api.readdir or list out/*.js, skipping the index bundle found via jsMatch) and
assert that at least one other JS file contains "shared-value" (confirming a
shared chunk was created and the shared module wasn't dropped).
Test added, also, I created a minimal repro https://github.com/2hu12/minimal-bun-splitting-bug |
2 participants
What does this PR do?
How did you verify your code works?
Verified locally using the reproduction repository mentioned in the issue: https://github.com/david-arteaga/bun-bundler-splitting-bug. The fix was validated on a local bun-debug build.