Skip to content

css: merge adjacent @supports rules with the same condition instead of dropping the later one - #33680

Closed
robobun wants to merge 2 commits into
mainfrom
farm/83686cec/css-supports-merge
Closed

robobun wants to merge 2 commits into
mainfrom
farm/83686cec/css-supports-merge

Conversation

@robobun

@robobun robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #24770

What

bun build / Bun.build on CSS silently deletes the contents of the second of two adjacent @supports rules with the same condition. This happens on the default path (no --minify) because the rule-merge pass runs unconditionally.

Reproduction

const b = await Bun.build({ entrypoints: ["e.css"] });
console.log(await b.outputs[0].text());
/* e.css */
@supports (display: grid) { .a { color: red } }
@supports (display: grid) { .b { color: blue } }
@media screen { .c { color: red } }
@media screen { .d { color: blue } }

Output before:

@supports (display: grid) {
  .a { color: red; }       /* .b { color: blue } is gone */
}
@media screen {
  .c { color: red; }
  .d { color: #00f; }      /* @media merged correctly */
}

Adjacent same-condition @supports blocks are exactly what autoprefixer, utility layers, and the bundler's own @import supports(...) handling emit, so this is silent style loss on real stylesheets. The same happens for @supports nested inside a style rule (#24770's Tailwind case).

Cause

CssRuleList::minify (src/css/rules/mod.rs) merges adjacent at-rules with identical conditions. The CssRule::Media arm appends the later rule's children into the earlier rule before discarding the shell:

last_rule.rules.v.append(&mut med.rules.v);
let _ = last_rule.minify(context, parent_is_unused)?;
break 'arm;

The CssRule::Supports arm ten lines below lost the append during the lightningcss port and just break 'arms, discarding the later rule with all its contents.

Fix

Append the later @supports rule's children into the earlier one and re-minify the merged block, matching the @media arm and upstream lightningcss.

Verification

New describe("supports") block in test/js/bun/css/css.test.ts covers: non-minified output preserves both rules merged into one block, minified output for two and three adjacent same-condition rules, a different-condition pair stays as two rules, and the nested-inside-selector case from #24770.

$ USE_SYSTEM_BUN=1 bun test test/js/bun/css/css.test.ts -t supports
 1 pass / 4 fail   # .b / .c / border-left / border-bottom dropped

$ bun bd test test/js/bun/css/css.test.ts -t supports
 5 pass / 0 fail

Full css.test.ts (1124 tests) and test/bundler/esbuild/css.test.ts (53 tests) pass.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e919062e-48c8-452d-99b1-0715abcece35

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5d816 and d0be899.

📒 Files selected for processing (2)
  • src/css/rules/mod.rs
  • test/js/bun/css/css.test.ts

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

@github-actions github-actions Bot added the claude label Jul 7, 2026
@robobun

robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:44 PM PT - Jul 7th, 2026

✅ @robobun, your commit d0be8999bf868a7ca7cf263f5e66e4452658f5df passed in Build #69963! 🎉


🧪   To try this PR locally:

bunx bun-pr 33680

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

bun-33680 --bun

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Found 1 issue this PR may fix:

  1. bun bundler cannot handle css @supports #24770 - Reports that adjacent @supports blocks with the same condition are silently dropped during CSS bundling, keeping only the first — exactly the bug this PR fixes by adding the missing append call in the CssRule::Supports merge arm.

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #24770

🤖 Generated with Claude Code

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — two-line fix mirroring the @media arm right above it, with good test coverage.

Extended reasoning...

Overview

This PR fixes a CSS bundler bug where adjacent @supports rules with the same condition had the second rule's contents silently dropped. The fix is a two-line change in src/css/rules/mod.rs that appends the later rule's children into the earlier one before discarding the shell — exactly matching the CssRule::Media arm ~10 lines above (and upstream lightningcss). Five new test cases in test/js/bun/css/css.test.ts cover non-minified output, 2- and 3-way merges, different-condition rules staying separate, and the nested-inside-selector case from #24770.

Security risks

None. This is pure CSS AST transformation with no user input validation, filesystem, network, or auth surface. The change only affects which child rules survive a merge pass.

Level of scrutiny

Low. The fix is mechanical: it copies the exact pattern from the @media arm in the same match block, adjusted only for SupportsRule::minify returning Result<(), _> instead of Result<bool, _> (hence no let _ =). The old comment ("Drop the duplicate-condition rule outright") confirms this was a porting oversight, not intentional behavior. The PR description shows the tests fail on system Bun and pass on the debug build.

Other factors

No CODEOWNERS for src/css/. No prior review comments on the PR. Full css.test.ts (1124 tests) and test/bundler/esbuild/css.test.ts (53 tests) reported passing. No bugs found by the bug-hunting system. The test cases follow the existing minify_test/cssTest conventions in the file.

@robobun

robobun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

#38589 makes @supports (property: value) conditions parse into declaration conditions, which changes when two conditions compare equal, so it needed this merge to land with it: it carries both commits from this branch (cherry-picked) plus the test expectations updated for the minified condition output. If this PR is merged first, #38589 rebases onto it with no change; otherwise #38589 supersedes it.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #38589, which carries this branch's two commits unchanged (the two-line append-and-minify in the CssRule::Supports arm of CssRuleList::minify, plus the supports test block; the diff is byte for byte the same apart from line offsets). #38589 only re-spells the expected minified conditions, since it prints them from the parsed form ((display:grid) instead of (display: grid)); the rule bodies in every assertion are the same.

Checked at #38589's head (fee67d6): the assertions from this PR pass there apart from that spelling difference, and the exact input from #24770 (five nested @supports (color: blue) blocks) keeps all five blocks, where the released binary keeps one. Main still has the drop, so #24770 stays open and is now linked from #38589 so it closes when that lands.

@robobun robobun closed this Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bun bundler cannot handle css @supports

1 participant