Skip to content

fix(bundler): prevent crash when asset file is both entrypoint and import #25062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(bundler): prevent crash when asset file is both entrypoint and import #25062

wants to merge 1 commit into from
+54 −4

Conversation

@robobun
Copy link
Collaborator

@robobun robobun commented Nov 25, 2025

Summary

Fixes #20278

When Bun.build() had entrypoints that included a file that was also imported by another entrypoint (e.g., a CSS file importing an SVG, and both being in the entrypoints array), it caused a segmentation fault.

Example

// style.css imports chevron-down.svg
// Both files are passed as entrypoints
const res = await Bun.build({
  entrypoints: ["./style.css", "./chevron-down.svg"],
  outdir: "./dist",
});

This would crash with:

panic(main thread): index out of bounds: index 1, len 1

Root Cause

The issue occurred in computeChunks.zig when iterating over files to determine which chunks they belong to. The code used entry point IDs as direct indices into the JS chunks array, but not all entry points create JS chunks:

  • CSS-only entrypoints create CSS chunks
  • Asset files (like SVG) don't create chunks at all

This caused an index out of bounds error when the code tried to access js_chunks[entry_point_id] where entry_point_id was larger than the number of JS chunks.

Changes

  1. Added a mapping from entry point IDs to JS chunk keys (entry_point_to_js_chunk_key) to properly handle cases where entry points don't create JS chunks
  2. Skip chunk creation for asset files (non-JavaScript-like loaders) since they're handled as external assets and don't need chunks

Test Plan

  • Created regression test at test/regression/issue/20278.test.ts
  • Test fails with USE_SYSTEM_BUN=1 (confirms it reproduces the bug)
  • Test passes with bun bd test (confirms the fix works)
  • The SVG is properly inlined as a data URL in the output CSS

🤖 Generated with Claude Code

@claude
fix(bundler): prevent crash when asset file is both entrypoint and im…
1458b45 
…port

Fixes #20278

When `Bun.build()` had entrypoints that included a file that was also
imported by another entrypoint (e.g., a CSS file importing an SVG, and
both being in the entrypoints array), it caused a segfault.

The issue occurred in `computeChunks.zig` when iterating over files to
determine which chunks they belong to. The code used entry point IDs as
direct indices into the JS chunks array, but not all entry points create
JS chunks (CSS-only entrypoints create CSS chunks, and asset files don't
create chunks at all). This caused an index out of bounds error.

The fix:
1. Create a mapping from entry point IDs to JS chunk keys to properly
   handle cases where entry points don't create JS chunks
2. Skip chunk creation for asset files (non-JavaScript-like loaders)
   since they're handled as external assets

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

Co-Authored-By: Claude <[email protected]>
@robobun
Copy link
Collaborator Author

robobun commented Nov 25, 2025 •
edited
Loading

Updated 2:00 AM PT - Nov 25th, 2025

❌ Your commit 1458b45e has 11 failures in Build #32392 (All Failures):

🧪   To try this PR locally:

bunx bun-pr 25062

That installs a local version of the PR into your bun-25062 executable, so you can run:

bun-25062 --bun

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Walkthrough

The pull request refactors the bundler's chunk computation logic to replace direct array-based chunk access with a JS chunks map and entry point-to-chunk-key mapping. A regression test is added for a crash scenario involving shared asset dependencies across entry points.

Changes

Cohort / File(s) Summary
Bundler chunk tracking refactor
src/bundler/linker_context/computeChunks.zig		 Replaces direct chunks array access in Handler struct with js_chunks map and entry_point_to_js_chunk_key mapping. Updates Handler.next() method signature to accept entry_point_id instead of chunk_id. Adds guards to skip chunk creation for non-JavaScript loaders (e.g., assets). Adjusts iteration, sorting, and finalization to consult the new structures before accessing chunk data.
Regression test
test/regression/issue/20278.test.ts		 Adds test verifying that bundling a CSS file importing an asset, when another entry point imports the same asset, completes without panic or segmentation fault.

Pre-merge checks

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: preventing a crash when an asset file is both an entrypoint and imported by another entrypoint.
Description check ✅ Passed The description covers both required template sections: 'What does this PR do?' (via Summary explaining the fix and root cause) and 'How did you verify your code works?' (via Test Plan with regression test details).
Linked Issues check ✅ Passed The PR fully addresses issue #20278 by adding entry_point_to_js_chunk_key mapping to handle cases where not all entrypoints create JS chunks, and skipping chunk creation for asset files.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the segmentation fault issue: modifications to computeChunks.zig implement the fix, and the regression test validates the specific crash scenario from #20278.

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Nov 25, 2025
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 123ac9d and 1458b45.

📒 Files selected for processing (2)
  • src/bundler/linker_context/computeChunks.zig (5 hunks)
  • test/regression/issue/20278.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
test/**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test, describe, expect) and import from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/regression/issue/20278.test.ts
test/regression/issue/**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

When a test is for a specific numbered GitHub Issue, place it in test/regression/issue/${issueNumber}.test.ts with a REAL issue number, not a placeholder

Files:

  • test/regression/issue/20278.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: For single-file tests, prefer using -e flag over tempDir
For multi-file tests, prefer using tempDir from harness and Bun.spawn over other temporary directory creation methods
Always use port: 0 for network tests and do not hardcode ports or use custom random port number functions
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test
Use tempDir from harness to create temporary directories, do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, check stdout/stderr expectations BEFORE checking exit code to get more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests, instead await the condition to be met as you are testing the CONDITION not the TIME PASSING
Verify your test fails with USE_SYSTEM_BUN=1 bun test <file> and passes with bun bd test <file> - your test is NOT VALID if it passes with USE_SYSTEM_BUN=1

Files:

  • test/regression/issue/20278.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with 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 use port: 0 to get a random port
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
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
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() 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, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to 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
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/regression/issue/20278.test.ts
test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

  • test/regression/issue/20278.test.ts
src/**/*.{cpp,zig}

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

src/**/*.{cpp,zig}: Use bun bd or bun run build:debug to build debug versions for C++ and Zig source files; creates debug build at ./build/debug/bun-debug
Run tests using bun bd test <test-file> with the debug build; never use bun test directly as it will not include your changes
Execute files using bun bd <file> <...args>; never use bun <file> directly as it will not include your changes
Enable debug logs for specific scopes using BUN_DEBUG_$(SCOPE)=1 environment variable
Code generation happens automatically as part of the build process; no manual code generation commands are required

Files:

  • src/bundler/linker_context/computeChunks.zig
src/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)

Use bun.Output.scoped(.${SCOPE}, .hidden) for creating debug logs in Zig code

Implement core functionality in Zig, typically in its own directory in src/

src/**/*.zig: Zig source code should be organized in src/*.zig for core runtime, JavaScript bindings, and package manager
In Zig code, be careful with allocators and use defer for cleanup to manage memory properly
Use BUN_DEBUG_QUIET_LOGS=1 to disable debug logging, or BUN_DEBUG_<scopeName>=1 to enable specific Output.scoped calls in debug builds
Run bun run zig:check-all to compile the Zig code on all platforms when making platform-specific changes

src/**/*.zig: Private fields in Zig are fully supported using the # prefix: struct { #foo: u32 };
Use decl literals in Zig for declaration initialization: const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer @import at the bottom of the file (auto formatter will move them automatically)

Files:

  • src/bundler/linker_context/computeChunks.zig
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

**/*.zig: Expose generated bindings in Zig structs using pub const js = JSC.Codegen.JS<ClassName> with trait conversion methods: toJS, fromJS, and fromJSDirect
Use consistent parameter name globalObject instead of ctx in Zig constructor and method implementations
Use bun.JSError!JSValue return type for Zig methods and constructors to enable proper error handling and exception propagation
Implement resource cleanup using deinit() method that releases resources, followed by finalize() called by the GC that invokes deinit() and frees the pointer
Use JSC.markBinding(@src()) in finalize methods for debugging purposes before calling deinit()
For methods returning cached properties in Zig, declare external C++ functions using extern fn and callconv(JSC.conv) calling convention
Implement getter functions with naming pattern get<PropertyName> in Zig that accept this and globalObject parameters and return JSC.JSValue
Access JavaScript CallFrame arguments using callFrame.argument(i), check argument count with callFrame.argumentCount(), and get this with callFrame.thisValue()
For reference-counted objects, use .deref() in finalize instead of destroy() to release references to other JS objects

Files:

  • src/bundler/linker_context/computeChunks.zig
src/**/*.{ts,zig,cpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Always use absolute paths in file operations

Files:

  • src/bundler/linker_context/computeChunks.zig
src/**/*.{ts,zig}

📄 CodeRabbit inference engine (CLAUDE.md)

Avoid shell commands in code - don't use find or grep; use Bun's Glob and built-in tools instead

Files:

  • src/bundler/linker_context/computeChunks.zig
🧠 Learnings (20)
📓 Common learnings 
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.

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: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/css.test.ts : Organize CSS tests in css.test.ts for tests concerning bundling bugs with CSS files

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.
📚 Learning: 2025-11-24T18:35:08.605Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/css.test.ts : Organize CSS tests in css.test.ts for tests concerning bundling bugs with CSS files

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:08.605Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/bundle.test.ts : Organize bundle tests in bundle.test.ts for tests concerning bundling bugs that only occur in DevServer

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:37:30.249Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.249Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:36:59.675Z 
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.675Z
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/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:08.605Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : Organize source-map tests in sourcemap.test.ts for tests verifying source-maps are correct

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:08.605Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/html.test.ts : Organize HTML tests in html.test.ts for tests relating to HTML files themselves

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:36:33.049Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.049Z
Learning: Applies to test/bundler/**/*.test.{ts,tsx} : For bundler and transpiler tests, use the `test/bundler/` directory and the `itBundled` helper

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:36:33.049Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.049Z
Learning: Applies to **/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - your test is NOT VALID if it passes with `USE_SYSTEM_BUN=1`

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:08.605Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.605Z
Learning: Applies to test/bake/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:36:33.049Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:33.049Z
Learning: Applies to test/js/bun/**/*.test.{ts,tsx} : For Bun-specific API tests, use the `test/js/bun/` directory (for http, crypto, ffi, shell, etc.)

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:37:30.249Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.249Z
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/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:50.414Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.414Z
Learning: Applies to test/cli/**/*.{js,ts,jsx,tsx} : When testing Bun as a CLI, use the `spawn` API from `bun` with the `bunExe()` and `bunEnv` from `harness` to execute Bun commands and validate exit codes, stdout, and stderr

Applied to files:

  • test/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:37:30.249Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.249Z
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/regression/issue/20278.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/regression/issue/20278.test.ts
📚 Learning: 2025-11-24T18:35:39.196Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.196Z
Learning: Applies to **/js_*.zig : Implement proper memory management with reference counting using `ref()`/`deref()` in JavaScript bindings

Applied to files:

  • src/bundler/linker_context/computeChunks.zig
📚 Learning: 2025-11-24T18:36:08.548Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.548Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Include new class bindings in `src/bun.js/bindings/generated_classes_list.zig` to register them with the code generator

Applied to files:

  • src/bundler/linker_context/computeChunks.zig
📚 Learning: 2025-09-06T03:37:41.154Z 
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 22229
File: src/bundler/LinkerGraph.zig:0-0
Timestamp: 2025-09-06T03:37:41.154Z
Learning: In Bun's codebase, when checking import record source indices in src/bundler/LinkerGraph.zig, prefer using `if (import_index >= self.import_records.len)` bounds checking over `isValid()` checks, as the bounds check is more robust and `isValid()` is a strict subset of this condition.

Applied to files:

  • src/bundler/linker_context/computeChunks.zig
📚 Learning: 2025-11-24T18:35:39.196Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.196Z
Learning: Applies to src/**/index.zig : Create an `index.zig` entry point in your module directory that exports all module functionality

Applied to files:

  • src/bundler/linker_context/computeChunks.zig
📚 Learning: 2025-11-24T18:36:08.548Z 
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-11-24T18:36:08.548Z
Learning: Applies to **/*.zig : Expose generated bindings in Zig structs using `pub const js = JSC.Codegen.JS<ClassName>` with trait conversion methods: `toJS`, `fromJS`, and `fromJSDirect`

Applied to files:

  • src/bundler/linker_context/computeChunks.zig
🧬 Code graph analysis (1)
test/regression/issue/20278.test.ts (1)
test/harness.ts (2)
  • tempDir (277-284)
  • bunExe (102-105)
🔇 Additional comments (5)
src/bundler/linker_context/computeChunks.zig (5)

34-37: LGTM!

The mapping is correctly initialized with temp_allocator (arena-backed) and properly reserves capacity for all entry points. Using ensureUnusedCapacity is appropriate since not all entry points will create JS chunks.

114-118: LGTM! Core fix for the crash.

This correctly skips JS chunk creation for asset files (SVG, images, fonts) which don't produce JS chunks. Assets are handled externally and don't need chunk entries, preventing the out-of-bounds access that caused the segfault.

138-139: LGTM!

Correctly tracks entry point to JS chunk key mapping using putAssumeCapacity (capacity was ensured at line 37). This mapping enables the Handler to only process entry points that actually created JS chunks.

200-215: LGTM!

The Handler now correctly:

  1. Holds references to both maps for lookup
  2. Only processes entry points that created JS chunks (via entry_point_to_js_chunk_key.get())
  3. Uses the same catch unreachable pattern for getOrPut as existing code (line 246)

This prevents the original crash by ensuring we never access a chunk for an entry point that didn't create one.

248-254: LGTM!

Handler instantiation correctly passes references to both maps, enabling proper chunk lookup in the non-code-splitting path.

test/regression/issue/20278.test.ts
Comment on lines +25 to +27
expect(stderr).not.toContain("panic");
expect(stderr).not.toContain("Segmentation fault");
expect(exitCode).toBe(0);
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove panic/segfault checks and verify actual output instead.

Per coding guidelines: "Never write tests that check for no 'panic' or 'uncaught exception' or similar in the test output - that is NOT a valid test."

The PR description mentions "Confirmed the SVG is inlined as a data URL in the output CSS" - verify this behavior instead.

-  expect(stderr).not.toContain("panic");
-  expect(stderr).not.toContain("Segmentation fault");
   expect(exitCode).toBe(0);
+
+  // Verify output files exist and SVG is inlined as data URL in CSS
+  const outputCss = await Bun.file(`${dir}/out/style.css`).text();
+  expect(outputCss).toContain("data:image/svg+xml");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(stderr).not.toContain("panic");
expect(stderr).not.toContain("Segmentation fault");
expect(exitCode).toBe(0);
expect(exitCode).toBe(0);
// Verify output files exist and SVG is inlined as data URL in CSS
const outputCss = await Bun.file(`${dir}/out/style.css`).text();
expect(outputCss).toContain("data:image/svg+xml");
🤖 Prompt for AI Agents 
In test/regression/issue/20278.test.ts around lines 25 to 27, remove the
assertions that check stdout/stderr do not contain "panic" or "Segmentation
fault" and instead assert the actual expected behavior: confirm the generated
CSS contains the inlined SVG as a data URL (e.g., assert output CSS includes
"data:image/svg+xml" or the specific inlined SVG string described in the PR),
and keep or assert exitCode is 0 if desired; update the test to look for the
concrete CSS output rather than negative panic/segfault checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

claude slop

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bun.build() crashes when entrypoints includes a file, that is also imported by another entrypoint

3 participants

@robobun @Jarred-Sumner