css: keep the selectors on rules added for color fallbacks and logical properties by robobun · Pull Request #43240 · oven-sh/bun

robobun · 2026-09-18T06:38:27Z

Problem

Bun.build over .x:where(a), .y:is(a, b) { box-shadow: 2s lch(83% 47 20) } prints @supports (color: lab(0% 0 0)) { { box-shadow: ... } }, a style rule with no selector. Bun then fails on its own output: Invalid selector. Empty selector is not allowed.
The cause is in minify_style_arm (src/css/rules/mod.rs:774). It copies sty.selectors into the rules that a declaration adds. Two earlier steps can empty that list. The partition moves every selector out when the targets support none. A selector merge moves them into the previous rule.
The merge path also loses styling: .a{color:red} .b{color:red;border-start-start-radius:2px} builds to .a,.b{color:red} on the default targets.

Fix

Snapshot sty.selectors before the merge when the declarations staged rules. Restore them after a merge. The existing incompatible_decls snapshot does the same for declarations.
Build no added rules for a rule that the partition left without a selector. The rules for the incompatible selectors carry their own copies.
Verified: test/js/bun/css/css.test.ts, 6 new tests that all fail without the fix. Also test/js/bun/css/, test/bundler/css/, test/bundler/esbuild/css.test.ts.

Background

A declaration can need rules outside its style rule. lch() in an unparsed value stages @supports fallbacks. A logical property stages :dir() rules when a target lacks it. PropertyHandlerContext holds them for minify_style_arm.
When a target lacks some selectors of a list, each of those selectors gets its own copy of the rule.
merge_style_rules appends a rule's selectors to the previous rule when their declarations are equal.

Notes

The default browser targets include Chrome 87. :is() and :where() need Chrome 88, and border-start-start-radius needs Chrome 89, so plain CSS reaches both paths with no configuration.
The partition path needs a list of two or more selectors that are all incompatible, plus a declaration that stages an @supports rule. .x:where(a), .y:where(b) is enough, in either order. Logical and dark-mode rules for the empty rule were already dropped, because those rules pass through CssRuleList::minify and the @supports rules do not.
The merge path with an @supports fallback: .a { box-shadow: 2s #ffbdbf } .b { box-shadow: 2s lch(83% 47 20) } printed .a,.b{...} plus selector-less @supports rules, so the .b fallbacks were lost. It now prints @supports (...) { .b { ... } }.
The merge path with a logical property: on the default targets the four logical corner radii, inset-inline, inset-inline-start and inset-inline-end vanished. I ran all seven on the fixed build. Each prints .a,.b{color:red} plus the same :dir() rules that .b prints when it stands alone.
A rule with one compatible and one incompatible selector, after a rule with equal declarations, gave its two selectors different declarations. .a { color: red } .b, .y:focus-visible { ... } on Safari 14 is the test for it.
Three kinds of direct output change.
1. A rule that the partition left empty no longer prints rules without a selector.
2. A rule that merged by selectors while it had staged rules gets its added rules back, on its own selectors.
3. A consequence of 2. The restored rules end the merge run, so a later rule with equal declarations no longer joins the merged rule. .a{color:red} .b{color:red;border-start-start-radius:2px} .c{color:red} printed .a,.b,.c{color:red}. It now prints .a,.b{color:red}, the .b rules, then .c{color:red}. Source order is kept and nothing is lost. An unmerged .b has always ended the run in the same way. The @supports case does not change, because the rules without a selector already ended the run.
How I checked: I compared the output of Bun 1.4.2 and of this branch block by block over 4,500 generated stylesheets. Every difference is kind 1, kind 2, or both. No block that has a selector went away. Kind 3 did not occur in them. A larger comparison that I did not run reported kind 3, and I then reproduced it by hand with the input above.
This note first listed only kinds 1 and 2.
A fourth, indirect effect, found by that larger comparison in two stylesheets and now explained. Next to a rule of kind 1 inside a nested rule list, one block changed from a bare :full-screen to :fullscreen, or from a bare ::file-upload-button to ::file-selector-button. The bare spelling is not valid CSS, so the change is a repair. .c0:fullscreen { & .c1, & .c2 { box-shadow: 2s lch(50% 40 30 / .5) } } shows it.
- The cause is a separate printer bug that this PR does not touch. StyleRule::to_css (src/css/rules/style.rs:155) sets dest.vendor_prefix to empty after a rule that has its own vendor prefix, and does not restore the value of the enclosing pass. Later nested siblings then print the parent's downleveled pseudo with no pass prefix, which gives the bare legacy name.
- The inner rule of each @supports fallback has vendor_prefix: NONE, so it triggers that reset. The rules without a selector stood in front of the first split-out rule and reset the prefix before it. They are gone now, so that rule prints correctly. The second split-out rule still prints the bare name on every build, because the valid @supports rules of the first one reset the prefix in the same way.
- The printer bug alone, with no code from this PR involved: .c0:fullscreen { & .a:any-link { color: red } & .b { color: blue } } prints .c0:full-screen .b. With the two nested rules swapped it prints .c0:fullscreen .b. Bun 1.4.2 and this branch print the same.
- I could not construct a case where this PR turns a valid spelling into a bare one. Kind 1 only removes rules that reset the prefix. Kind 2 adds @supports rules where rules without a selector, with the same vendor_prefix, already stood. Its :dir() rules carry a prefix only when the targets lack :is(), and then nested siblings do not merge. This is reasoned and checked with three inputs, not proven.
A second and a third build over the fixed output keep every added rule on its selector, and print no rule without a selector. The output still changes between passes for a separate reason that this PR does not touch: a color fallback inside a printed @supports block is generated again on each pass. Bun 1.4.2 does the same.
Reasoned, not proven: no input makes the flush inside merge_style_rules stage a rule for a rule that staged none itself. If one exists, this change prints no rule where the old code printed a selector-less one.
The 4,500 stylesheets have 1 to 9 rules, lists of 1 to 3 selectors, and some at-rule wrappers. They have no nesting, which is why they could not show the fourth effect. The baseline was the 1.4.2 release, not a build of the base commit.
A pre-push automated review of this diff did not finish. I reviewed the diff by hand before the push.
lightningcss 1.33.0, the upstream of this code, prints the same wrong output for these inputs.
bun bd test test/js/bun/css/ shows timeouts in css-fuzz.test.ts, nested-selector-expansion.test.ts and nested-vendor-prefix-duplication.test.ts. They are the same on an unfixed debug build in the same container.
css: keep incompatible selectors that fail the same way in one rule #40464, css: split target-incompatible selectors out of a rule in linear time #42243, css: keep a selector list whole when a member is vendor prefixed #42100 and css: stop emitting :-webkit-any() copies of :is() that outrank the original rule #42029 rework the selector partition in minify_style_arm. None of them changes where the added rules get their selectors.

[human-review] gate passed · iteration 0 · 3 files touched

fails on main (without fix)

ASAN without fix: 6 failed, 67 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/css/css.test.ts
bun test v1.4.3 (b52d51348)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [3.29ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.13ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [0.99ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: wh
... (truncated)

release without fix: 6 failed, 67 skipped
bun test v1.4.3-canary.1 (b52d51348)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [0.16ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [0.04ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [0.04ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: white;
        --my-font-size: 16px;
      }
      .element {
        color: var(--my-color);
        background-color: var(--my-bg);
        font-size: 
... (truncated)

passes on PR (with fix)

ASAN with fix: 67 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/css/css.test.ts
bun test v1.4.3 (b52d51348)

test/js/bun/css/css.test.ts:
Output .flexrow {
  flex-direction: row;
}

.flexcol {
  flex-direction: column;
}

.hello {
  flex-wrap: wrap;
}

.world {
  flex-wrap: nowrap;
}

(pass) css tests > .flexrow {
	flex-direction: row;
}

.flexcol {
	flex-direction: column;
}

.hello {
	flex-wrap: wrap;
}

.world {
	flex-wrap: nowrap;
} [8.48ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.94ms]
Output div {
  --foo: 1 1 1 #0101011a, 2 2 2 #02020233;
  --bar: 1 1 1 #01010116, 2 2 2 #02020233;
}

(pass) css tests > custom property cases > div {
        --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2);
        --bar: 1 1 1 #01010116, 2 2 2 #02020233;
      } [2.53ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

.element {
  color: var(--my-color);
  background-color: var(--my-bg);
  font-size: var(--my-font-size);
}

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: wh
... (truncated)

release with fix: 67 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     d8c070960f
  features     lto, baseline

23 deps, 131 codegen, 1176 objects in 935ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1250] install /workspace/bun
bun install v1.4.3-canary.1 (b52d51348)

Checked 22 installs across 61 packages (no changes) [20.00ms]
[2/1250] gen ErrorCode+*.h
[3/1250] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (b52d51348)

Checked 1 install across 2 packages (no changes) [5.00ms]
[4/1250] install /workspace/bun/src/node-fallbacks
bun install v1.4.3-canary.1 (b52d51348)

Checked 111 installs across 104 packages (no changes) [10.00ms]
[5/1250] gen bindgenv2
[6/1250] fetch tinycc
[tinycc] up to date
[7/1249] gen node-fallbacks/react-refresh.js
Bundled 1 module in 6ms

  react-refresh.js  4.81 KB  (entry point)

[8/1249] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[9/1222] fetch zlib
[zlib] up to date
[10/1222] gen .bind.ts → GeneratedBindings.cpp
[11/1222] gen bake.{client,server,error}.js
-> bake.client.js, bake.
... (truncated)

diff hotspot

src/css/context.rs          |   8 +++
 src/css/rules/mod.rs        |  33 ++++++---
 test/js/bun/css/css.test.ts | 159 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 191 insertions(+), 9 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                         reads  edits  tests
src/css/context.rs               2      1     20
src/css/rules/mod.rs             3      4     21
test/js/bun/css/css.test.ts      2      3     20

`minify_style_arm` copies `sty.selectors` into the rules that a declaration adds after its style rule: `@supports` rules for color fallbacks, `:dir()` rules for logical properties, and the dark `@media` rule. Two earlier steps can leave that list empty. - The partition moves every selector out when none is compatible with the targets. The `@supports` rules of the original rule were still printed, with no selector. - A selector merge moves the selectors into the previous rule. The `@supports` rules were printed with no selector, and the logical rules were dropped. Snapshot the selectors before the merge when the declarations staged such rules. Build no added rules for a rule that the partition left without a selector.

…path The logical test now also uses `inset-inline`, which a second property handler stages. A new case has a rule whose compatible selector merges into the previous rule while its incompatible selector gets a rule of its own. Both selectors must keep the same declarations.

robobun · 2026-09-18T06:38:42Z

Status

Reproduced on 1.4.3-canary.1+b52d51348 and on a debug build of main at fd8422c.

cat > in.css <<'CSS'
.x:where(a), .y:is(a, b) { box-shadow: 2s lch(83% 47 20) }
CSS
bun build in.css --outdir out && bun build out/in.css --outdir out2
# error: Invalid selector. Empty selector is not allowed

The merge path needs no second build to see. .a{color:red} .b{color:red;border-start-start-radius:2px} builds to .a,.b{color:red}.

Tests: bun bd test test/js/bun/css/css.test.ts -t "rules added for a declaration". All 6 fail without the change in src/css/ and pass with it.

PR: #43240

robobun · 2026-09-18T06:38:47Z

^{Updated 12:02 AM PT - Sep 18th, 2026}

✅ @robobun, your commit d8c070960f365ebd996977c74cdc3f50156ccbaa passed in Build #117586! 🎉

🧪 To try this PR locally:

bunx bun-pr 43240

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

bun-43240 --bun

coderabbitai · 2026-09-18T06:41:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: c29be632-7a78-4bae-aaf4-39f7e891ea1c

📥 Commits

Reviewing files that changed from the base of the PR and between 08a2340 and d8c0709.

📒 Files selected for processing (3)

src/css/context.rs
src/css/rules/mod.rs
test/js/bun/css/css.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The CSS minifier now detects staged handler-context rules, preserves selectors during rule merging, and avoids generating extras for selectorless styles. Tests cover selector fallbacks, logical rules, selector splitting, and repeated builds.

Changes

CSS staged-rule generation

Layer / File(s)	Summary
Staged rules and selector preservation `src/css/context.rs`, `src/css/rules/mod.rs`, `test/js/bun/css/css.test.ts`	`PropertyHandlerContext` reports staged declarations. `minify_style_arm` preserves selectors, flushes pending merges, and omits extra rules when no selectors exist. Tests cover selector fallbacks, logical properties, selector splitting, and CSS rebuilds.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to d8c07

The CSS minification fix has no identified remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: preserving selectors on generated color-fallback and logical-property rules.
Description check	✅ Passed	The description explains the problem, fix, scope, expected behavior, and verification results. It does not use the exact template headings, but it provides the required information in equivalent secti…

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

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

robobun added 2 commits September 17, 2026 23:33

robobun assigned Jarred-Sumner Sep 18, 2026

github-actions Bot added the claude label Sep 18, 2026

claude Bot reviewed Sep 18, 2026

View reviewed changes

Comment thread src/css/rules/mod.rs

This was referenced Sep 18, 2026

css: keep target-added vendor prefix passes on cloned and re-minified rules, and never print a bare :full-screen #43339

Open

css: keep the vendor prefix passes of a rule in its @supports color fallbacks #43434

Open

Navigation Menu

Repository navigation

css: keep the selectors on rules added for color fallbacks and logical properties - #43240

css: keep the selectors on rules added for color fallbacks and logical properties#43240
robobun wants to merge 2 commits into
mainfrom
robobun/d46f48f3/css-empty-selector-extra-rules

robobun commented Sep 18, 2026 •

edited

Loading

Uh oh!

robobun commented Sep 18, 2026

Uh oh!

robobun commented Sep 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 18, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

robobun commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Uh oh!

robobun commented Sep 18, 2026

Status

Uh oh!

robobun commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Sep 18, 2026 •

edited

Loading

robobun commented Sep 18, 2026 •

edited

Loading

coderabbitai Bot commented Sep 18, 2026 •

edited

Loading