Repository navigation
Conversation
…ylesheet compute_chunks keys each CSS chunk by a hash of its import order. CssImportOrder::hash leaves the import conditions out, and it writes the layer names "x.y, z" and "x, yz" as the same bytes. Two entry points whose import orders differ only there got one stylesheet. With CSS chunking on, a CSS entry point with the hash of an earlier chunk got no output file. css_chunk_key now compares the import order of the chunk that has the hash. If the orders differ, it takes the next free key. Entry points with equal import orders share one chunk as before.
|
Status: ready for review. The diff is green. Reproduced on Bun 1.4.3-canary.1+367d939d9 with the three new cases of
With this branch the 28 tests of the file pass on a debug build. The three new cases fail without the change and pass with it. CI: build 130197 has 180 of 181 jobs green. The one red job is |
|
Updated 2:56 PM PT - Oct 21st, 2026
❌ @robobun, your commit e620e0b has 1 failures in 🧪 To try this PR locally: bunx bun-pr 46357That installs a local version of the PR into your bun-46357 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughCSS chunk-key generation now checks import-order equality when hashes match and probes subsequent keys for differing orders. Regression tests cover distinct CSS output for differing media conditions and layer names. ChangesCSS chunk key collision handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly summarizes the main change: comparing CSS import orders before sharing a stylesheet between entry points. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description check | |
The description clearly explains the problem, implementation, verification steps, regression tests, tradeoffs, and known limitations. It does not use the template headings exactly, but it provides the… |
Full details: Description check
-
Explanation
The description clearly explains the problem, implementation, verification steps, regression tests, tradeoffs, and known limitations. It does not use the template headings exactly, but it provides the required information in equivalent sections.
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the linear probe in css_chunk_key: it only advances past an occupied key whose stored order differs, so a non-chunking CSS entry (keyed by entry bits) can only merge with a content-keyed chunk when both the 64-bit keys and the full import orders match, which is the same 2^-64 event the base already had. I also checked whether a second entry that joins an existing CSS chunk should update that chunk's entry_bits — the found_existing skip is unchanged from the base at both call sites, so it is not something this PR introduces.
Extended reasoning...
The change touches the bundler's CSS chunk computation (computeChunks.rs, Chunk.rs, findImportedFilesInCSSOrder.rs) and adds three itBundled tests; it touches no security-sensitive surface. Two confirmed findings are posted inline, so a human look is already signalled; this note only records the probe-chain and entry_bits concerns that were examined and ruled out as unchanged from the base.
|
Both inline findings are correct, and both are older than this PR. Replies are in the threads.
No code change in this round. |
|
A follow-up for the case that this PR lists under "Not fixed" (a CSS entry point whose import order equals the stylesheet of an earlier entry point gets no output file) is in #46599. It is stacked on this branch, because it is correct only when a hit in |
|
A note from #36974, which touched the same function in its first design. #36974 no longer changes #36974 also removes the sort of CSS chunks by key. After it, the key that |
Problem
html/SeparateCSSWhenImportConditionsDifferthe second page needs@layer b;first, and it gets@media screen{@layer b;}.Bundle was not written to disk: out/s.css.compute_chunkskeys a CSS chunk byCssImportOrder::hash(src/bundler/Chunk.rs:1462). The hash leaves out the import conditions, socss_chunks.get_or_put(computeChunks.rs:183,:263) returns the chunk of another import order.Fix
css_chunk_keycompares the import order of the chunk that has the hash. If the orders differ, it takes the next free key.html/SharedCSSProductionMultipleEntriespasses.test/bundler/bundler_html.test.ts(3 new cases, each fails on main).Background
@importrules, and lists of@layernames. Each entry has the conditions of its@importchain.Downsides
Notes
Report. No user reported this. A fuzz run over random CSS import graphs with two entry points found it.
The three cases. Each fails on main and on Bun 1.4.3.
html/SeparateCSSWhenImportConditionsDiffer: the import orders differ in the media query of one layer list. Without screen media the layer order is a, b on the first page and b, a on the second page.html/SeparateCSSWhenLayerNamesDiffer: the import orders differ in the names of one layer list,x.y, zagainstx, yz.CssImportOrder::hashwrites both asx.yz.. The layeryzwins on the first page and the layerzwins on the second page.html/SeparateCSSForCSSEntryPointProduction: a page and a CSS entry point. The stylesheet of the page starts with@layer a;. The entry point needs@media print{@layer a;}, and main writes nos.css.Not changed.
CssImportOrder::hashstays as it is. A change there moves the key of each chunk, and the key sorts the CSS chunks in the output.Still open.
bun build --production page.html s.css, where the page links onlys.css, writes nos.cssand exits with 0. The two import orders are equal, so the entry point shares the chunk of the page, and the page owns that chunk. With the entry points in the other order the page links./s.css. This is older than this PR and is not a collision. #38341 fills the metafile inputs of a chunk that a CSS entry point creates.Other suites.
test/bundler/esbuild/css.test.ts,bundler_splitting.test.ts,esbuild/splitting.test.ts,bundler_html_server.test.ts,html-import-manifest.test.tsandcss/css-modules.test.tspass on a debug build.Measurements. x64 release builds,
nm -Sand a qemu instruction trace ofcss_chunk_key.css_chunk_key1,655 bytes,compute_chunks46,141 to 46,259 bytes. The build pair is from the same hunks on top of 0ffddcd, one day before this merge base. A release build of main was not repeated for this PR.css_chunk_key: 43 for one CSS entry point, 13,113 for 200 CSS entry points (66 each), 5,384 for 50 JS entry points that import one stylesheet (108 each, with the compare).Related. A change to the cascade layer order of bundled
@importrules is in preparation on top of #45675. It adds layer lists with conditions, so it makes this collision more likely. This PR does not need it.[human-review] gate passed · iteration 0 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file