Repository navigation
Conversation
…imation, mask, flex and flex-flow shorthands The parsers of these six shorthands accepted zero components and returned a value made of defaults. So an empty value (`background:`) and an empty list item (`transition: color 1s,`) parsed as a valid declaration that replaced the declaration before it. A browser drops such a declaration. Each parser now fails when its loop consumed no token. The value then takes the existing unparsed path and prints as written.
Collaborator
Author
|
Updated 5:20 PM PT - Oct 24th, 2026
✅ @robobun, your commit 030075e31b5618b3ba36ce4203b1dd36e735a956 passed in 🧪 To try this PR locally: bunx bun-pr 46816That installs a local version of the PR into your bun-46816 --bun |
Collaborator
Author
|
Status: not merged. A maintainer did not take this change in #47025, because no real stylesheet in that check has this input. The branch stays, so the PR can reopen if the input shows up in real CSS. How I reproduced it (main at 179664e and bun 1.4.3, Linux x64):
With this PR the three rules print as written. |
Contributor
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Require shorthand parsing progress src/css/css_parser.rs, src/css/properties/{animation,background,flex,masking,transition}.rs |
Added a parser advancement check and applied it to five shorthand parsers. They now return an error when parsing consumes no input. |
Cover empty shorthand values test/js/bun/css/css.test.ts |
Added minification tests for empty shorthand values and comma-separated items, declaration ordering, duplicate rules, keyframes, container style queries, valid single-component shorthands, and empty custom properties. |
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
03007
-
No actionable merge-blocking risk is identified; the change is ready for normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title names the six shorthands and the core change: requiring at least one component in each. It matches the changeset, which adds Parser::expect_advanced_from and checks in background, transition… Description check The description follows the template. It has a Problem/Fix/Background/Downsides structure and a detailed Notes section. Verification is covered under 'Verified' and 'Suites run with the debug build', … 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. Full details: Title check
-
Explanation
The title names the six shorthands and the core change: requiring at least one component in each. It matches the changeset, which adds Parser::expect_advanced_from and checks in background, transition, animation, mask, flex and flex-flow. It is specific and concise, and it uses the 'css:' prefix convention.
Full details: Description check
-
Explanation
The description follows the template. It has a Problem/Fix/Background/Downsides structure and a detailed Notes section. Verification is covered under 'Verified' and 'Suites run with the debug build', with test counts and the 48 new tests. The template headings 'What does this PR do?' and 'How did you verify your code works?' are not used verbatim. The Problem and Fix sections cover the first, and the Verified lines cover the second.
-
- 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}
Contributor
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 other ||-group shorthand parsers for the same empty-value gap: border (any flag in src/css/properties/border.rs), font (required size) and border-image (per-Option checks) already reject an empty value, and the new sweep test in test/js/bun/css/css.test.ts over the KNOWN table in properties_generated.rs covers the remaining typed properties, so the fix is not missing a sibling site. The position-compare check itself is sound because every component attempt is a try_parse that restores the position on failure.
Extended reasoning...
The change adds a Parser::expect_advanced_from helper in src/css/css_parser.rs and calls it after the component loop in six shorthand parsers (animation, background, flex, flex-flow, mask, transition), so an empty value or empty list item becomes an unparsed declaration instead of a value of defaults. It touches no security-sensitive surface. The inline findings are behavior-change consequences (lost trailing-comma repair, duplicate-rule interaction) that a maintainer should weigh, which is why a human look is still warranted.
|
|
||
| break; | ||
| } | ||
| parser.expect_advanced_from(start)?; |
Contributor
There was a problem hiding this comment.
🟡 (optional) Sites whose stylesheets have a trailing comma in an animation list lose the effect in built output after upgrading; on the base branch it ran. src/css/properties/transition.rs:85 and the matching checks in animation.rs:123, masking.rs:204 and background.rs:111 reject the empty list item, so .a{animation:spin 1s,} is kept as written and every browser drops the declaration, where main printed animation:1s spin,none. Fix: decide the repair explicitly: either keep the empty-item repair for list shorthands (fail only when the whole value is empty) or emit a warning for the dropped declaration so the regression is visible.
Why this was flagged
If any application stylesheet contains a trailing or doubled comma in animation, transition, mask or background (a common typo and a common template-engine artifact), the parse of the empty item now fails at src/css/properties/transition.rs:85 (and animation.rs:123, masking.rs:204, background.rs:111) and Property::parse falls back to Unparsed at src/css/properties/properties_generated.rs:3888. The output is the invalid source text, which browsers discard. On the base branch the same input was printed as animation:1s spin,none or transition:color 1s,all, which browsers accept, so the animation ran and the transition applied in deployed builds. The author's 70,034-file npm sample measures library CSS, not application CSS, so the affected population is not ruled out; the PR itself lists this as a downside a maintainer must decide, and no test or warning marks the silent loss. The dismissal took the author's framing as settled rather than sizing the user-visible regression.
Verification: Acknowledged in diff: the PR description's "Downsides" and the new test block pin it. Now src/css/properties/animation.rs:123 returns Err(invalid_declaration) for the empty item after a trailing comma, and Property::parse prints the source text. On the base branch animation:spin 1s, came out as the working 1s spin,none; now it reaches the browser as-is and is dropped.
Collaborator
Author
There was a problem hiding this comment.
This is the first Downsides bullet of the PR body. I did not change the code, for these reasons.
The repair is a side effect of the defect, and it helps only some inputs. Release builds of main and of this PR, with Chrome 153:
| source | main prints | effect of the output of main |
|---|---|---|
animation:spin 1s, |
animation:1s spin,none |
the animation runs |
background:,red |
background:0 0,red |
paints red |
background:red, |
background:0 0,0 0 |
paints nothing, the red is lost |
transition:color 1s, |
transition:color 1s,all |
no transition starts |
- If the list parsers fail only for an empty whole value,
transition:opacity 2s;transition:color 1s,still printstransition:color 1s,all. The validopacity 2sis lost. That is the list item case of the Problem section. - The longhands do not have the repair.
animation-name:spin,andtransition-property:color,print as written on main. - A warning: no value that the typed parser rejects gives a warning today (
font:,color:banana,animation-name:spin,). A warning for these four shorthands only is a special case. A warning for every known property is a new diagnostic, and css: log parser warnings with the Source and a byte position #45787 changes how the parser logs warnings.
A maintainer decides between the output of 1.4.x and the strict output. The PR body says so.
| } | ||
| break; | ||
| } | ||
| input.expect_advanced_from(start)?; |
Contributor
There was a problem hiding this comment.
🟡 (optional) Pre-existing, widened by this change: a stylesheet with .a{animation:none}.b{color:red}.a{animation:} now loses the animation:none reset that the base kept. The empty value now becomes an unparsed declaration, and StyleRule::is_duplicate (src/css/rules/style.rs:480) compares only property_id(), so the earlier .a rule is removed and the browser drops the unparsed .a{animation:}. Fix: make the duplicate-rule check not treat an unparsed declaration as equal to a parsed one of the same name, so every shorthand this PR turns into unparsed text keeps the earlier same-selector rule; the PR marks this with a cssTestTodo rather than fixing it. [also at: src/css/properties/flex.rs:163 - Minified output drops a valid flex value entirely when a later same-selector rule has an empty flex: and a different rule sits between them; main kept flex:1.; src/css/properties/masking.rs:204 - Pages that reset mask, background, transition or flex-flow in an earlier same-selector rule lose the reset in minified output when a later rule has the empty shorthand; main preserved it. src/css/properties/masking.rs:204 (and background.rs:111, transition.rs:85, flex.rs:85) makes the empty…]
Why this was flagged
Input .a{animation:none}.b{color:red}.a{animation:} through bun build or the dev server. With the diff, Animation::parse fails at src/css/properties/animation.rs:123 and the declaration is stored as an unparsed property whose property_id() is still Animation. During minify, StyleRule::is_duplicate at src/css/rules/style.rs:465-495 compares rule declarations by property_id() only, so the first .a rule is judged a duplicate of the last and removed. Output is .b{color:red}.a{animation:}; a browser drops the empty declaration, so no animation:none reaches the page. On the base branch the empty value parsed to none, so the output .b{color:red}.a{animation:none} still applied the reset. The same removal now happens for background, transition, mask, flex and flex-flow rules in that shape; the PR description and the cssTestTodo at test/js/bun/css/css.test.ts:8058 note the duplicate check but leave it unfixed.
Verification: The failed typed parse becomes Property::Unparsed carrying the original id (src/css/properties/properties_generated.rs:3888-3889). src/css/rules/style.rs:466-480 is_duplicate compares only a.property_id() != b.property_id(), so a parsed animation:none and an unparsed animation: compare equal; src/css/rules/mod.rs:850-859 then drops the earlier .a rule.
Collaborator
Author
There was a problem hiding this comment.
Confirmed. It is the second Downsides bullet of the PR body, and the cssTestTodo row holds the expected output. I did not change is_duplicate in this PR, for these reasons.
- main removes the earlier rule too.
.a{background:red}.b{color:red}.a{background:}prints.b{color:red}.a{background:0 0}on main and.b{color:red}.a{background:}with this PR. A browser computes a transparent background for both. The output of main is right only when the value that it made from nothing equals the earlier value, as withanimation:none. For.a{flex:2}.b{color:red}.a{flex:}main printsflex:1, which is also not theflex:2of the source. - The check has this defect for every unparsed value of every property, on main and with this PR.
.a{color:red}.b{top:0}.a{color:foo(1)}prints.b{top:0}.a{color:foo(1)}..a{font:12px a}.b{color:red}.a{font:}prints.b{color:red}.a{font:}. - "An unparsed declaration is not equal to a parsed one" is not the complete rule. It keeps
.a{color:red}before a later.a{color:var(--x)}. That later rule is valid and does override the first, so sheets with no defect grow. It also does not cover an earlier unparsed value:.a{background:var(--x)}.b{color:red}.a{background:}still prints.b{color:red}.a{background:}. - The rule must say which later values a browser can drop, for all properties, and it needs a measurement of the output change. That is a separate change in the rule minifier. Open PRs css: compare custom property names in
PropertyIdequality and importance in duplicate-rule detection #33684, css: fold relative colors on the scale of their function, and keep them as written when the fold is not exact #45362 and css: parse the*propertyhack as a declaration #46751 each editis_duplicate.
Jarred-Sumner
added a commit
that referenced
this pull request
Oct 27, 2026
### What does this PR do? Consolidates the open pull requests on the CSS parser (`src/css`: parsing, minifying, lowering, printing, CSS modules) into one. 179 open pull requests were in scope. Each was judged by one rule: **the output must mean to a browser what the input meant.** Every claim was run through a build of `main`, a build with the change, lightningcss 1.33 and Chromium 156 (CSSOM, computed values, painted pixels for colors and gradients), and counted against 10,116 real stylesheets: the published CSS of about 190 popular npm packages, plus stylesheets from open source repositories. 107 land here, whole or in part, most of them cut down. The source diff is net negative. Of the 10,116, 9,509 are stylesheets that someone ships. The rest are test fixtures of other tools (521), Tailwind v4 source that needs its plugin (59) and SCSS, PostCSS or template source (27). | 9,509 shipped stylesheets | `main` | this branch | |---|---|---| | fail to build | 71 | 35 | | ... of those, valid CSS that Bun rejects | | 0 | | minified output reads differently in Chromium than the input | 552 | 2 | | CSS modules with a name that is referenced but not defined, or exported but not defined | 166 | 0 | | all 10,116 | `main` | this branch | |---|---|---| | fail to build | 192 | 141 | | minified output reads differently in Chromium than the input | 593 | 15 | | build on `main`, fail here | | 0 | | right on `main`, wrong here (minified, and at the default targets) | | 0 | | size of the minified output, no targets | 162.32 MB | 162.47 MB (+0.093%) | | size of the output at the default targets | 224.1 MB | 210.2 MB (-6.2%) | A syntax error is a build error, with one exception: the IE star hack `*zoom: 1` is skipped, because tachyons and handsontable ship it. The 35 shipped stylesheets that fail are invalid CSS: 28 have a selector that Chromium drops (semantic-ui-css, swiper, @ionic/core, Adobe Spectrum, xp.css), 2 an `@import` after other rules, and one each `@supports not X and Y`, a nested `@keyframes`, a stray comma, a selector list in `::slotted()`, `@media screen\0` (govuk-frontend). The 2 that read differently fold a `color-mix()` inside a gradient (see the known gaps). The growth of the minified output is rules and declarations that `main` deleted by mistake: 87 KB of rules, 45 KB of prefixed declarations that only browser targets can rule out. #### Popular CSS that `main` breaks today - normalize.css, modern-normalize, sanitize.css: `font-family: monospace, monospace` is collapsed to `monospace`, which changes the font size from 16px to 13px (#46662). - CKEditor 5, Tabler, Carbon, VitePress, Ionic, PatternFly, Excalidraw: a rule of custom properties is deleted as a "duplicate" of a later rule with the same selector and other custom properties. 37 stylesheets lose 1,681 custom properties (#33684). - Material Components, daisyUI, DocSearch, plyr: a `var()` size loses to its own fallback (#42101). - nextra, daisyUI, CKEditor: declarations written after a nested rule are moved above it (#45462). - antd 4, purecss, govuk-frontend: browser hacks in selector lists start to apply everywhere (#42100). - foundation-sites, flatpickr, 98.css, blueprintjs, scalar, react-datepicker: the build fails on valid CSS (#46756, #38552, #38534). tachyons, handsontable: it fails on the IE star hack (#46751). - nodejs.org, Mantine: a CSS module exports classes that no rule defines, because the `@apply` or `@mixin` in them is moved out of the rule (#46759). - hint.css and 42 more: `z-index: 1000001` and `1000000` become the same number (#46715). - Adobe Spectrum: `--version: 5.0.1` prints as `5.1`. - A quarter of all stylesheets: minified output has a stray space in `align-items: center` (#41993). #### Behavior changes to look at first 1. **`light-dark()` is kept as written at the default browser targets** (#42009). The lowered form `var(--buncss-light, a) var(--buncss-dark, b)` is invalid unless a compiled `color-scheme` covers the element: 184 of the 215 real stylesheets that get it never define the variable, so every browser drops the declaration. The docs section on the polyfill is removed. 2. **`border` is not built from longhands** (#42072): `border-width:0;border-style:solid;border-color:red` stays three declarations, because `border` also resets `border-image`. 305 real stylesheets grow by 30 KB in total; `bootstrap.min.css` by 113 bytes, 5 gzipped. No real stylesheet was found where the old merge shows, so this is the one to drop if size matters more. 14 existing expectations change. 3. **A CSS module no longer exports names that only occur inside `:global(...)`** (#45487). 73 real modules lose 202 export keys, all of them names that no rule has. `import { g }` or `composes: g` of such a name becomes a build error. 4. **Container names and `@counter-style` names stay global in a CSS module** (#45247 by @mikamikasuki). `main` hashes the `@container` name but not `container-name`, so a named query never matches. postcss-modules (Vite, css-loader, Next.js) keeps them global; lightningcss and esbuild hash both. Global keeps a container that is queried from another file or from JS working. 5. **Syntax errors are build errors, also two that `main` lets through**: a media query that stops short (`@media screen and {`, `@media not {`, `@import "a.css" screen and;`) and `@media (width <) foo {`. No stylesheet of the 10,116 has either. A condition that is valid by the grammar though no browser knows it, `( ... )` or `fn( ... )`, is kept as written (#46756): `(min-width:0\0)`, `(not(hover))`. 6. **Declarations after a nested rule stay where they were written**, as a nested declarations rule (#45462). Changes the output of 199 real stylesheets; in a fuzz of 2,850 nested programs compared by computed style, `main` was wrong in 1,056 and the pull request's code, before it was trimmed here, in none. 7. **`Bun.color`**: `color()` inputs are converted instead of returned as a string (`Bun.color("color(srgb 1 0 0)", "hex")` was `"color(srgb 1 0 0)"`); `currentColor`, system colors and `light-dark()` give `null` in the converting formats instead of the keyword as a string; a relative color or `color-mix()` outside sRGB is clipped, not gamut-mapped (`hsl(from oklch(100% .399 336.3) h s l)` is `#ff5bff`, was `#fff`). 8. The `:-moz-any()` and `:-webkit-any()` copies of rules with `:is()` are gone at the default targets (#45945, #42029). The `:-webkit-any()` copy outranked the original rule: `.title{color:blue} :is(h1,h2,h3){color:red}` painted red. 5.8% of all default-target output was the `:-moz-any()` copies. 9. `Property` grows from 216 to 224 bytes: `Url` stores its text as a slice so that two `url()` values can be compared (#46895). The pull request packed a pointer and a `u32` behind `unsafe` to avoid that. #### Not in this PR, needs a decision - **Raise the default browser targets (#40369) and add a `cssTarget` option (#40368).** Today: Chrome 80, Edge 80, Firefox 78, Safari 14, Opera 67, and no public way to change them. At Vite 8's floor the output of the 9,924 stylesheets is 15.3% smaller (3.9% gzipped). About a dozen fixes here only matter at today's floor, and code for older targets is unreachable without the option. - Vendor prefixes for `user-select`, `appearance`, `hyphens` and five more (#45814): 1,116 real stylesheets lack a prefix they need at the default targets. And for `backdrop-filter` (#44648): 173. - Clip instead of gamut-map in the sRGB fallback of a plain wide-gamut color: deletes about 90 lines, changes every pinned fallback hex. If the mapping stays, #44647 by @oddharsh makes it closer to what browsers paint (colors off by 2/255 or more: 168 to 110 of 3,170 real ones). #### Changes **Rules: duplicates, merging, nesting, fallbacks** - The `:-moz-any()` copy of every rule with `:is()` or `:not(a, b)` is gone: Firefox 78 has `:is()` (#45945). 1,164 real stylesheets - Keep declarations written after a nested rule in place with a nested declarations rule (#45462). 379 real stylesheets - With nesting compiled away, an unknown at-rule inside a style rule was moved out of it: `.wrapper{@apply flex}` printed a bare `@apply flex;` and the rule was gone, so a CSS module exported a class that no rule defines. It now stays in a block under the compiled selector. 72 real modules with Tailwind's `@apply`, 25 with Mantine's `@mixin` (#46759). 316 real stylesheets - Keep !important in the rules generated for logical properties and color-scheme (#38597). 311 real stylesheets - With browser targets, remove an earlier duplicate rule only when its declaration block is equal (#46861). 171 real stylesheets - Compare custom property names in `PropertyId` equality and importance in duplicate-rule detection (#33684). 40 real stylesheets - Keep same-condition @supports rules and empty named @layer blocks (#46733). 31 real stylesheets - Keep target-added vendor prefix passes on cloned and re-minified rules, and never print a bare `:full-screen` (#43339). 12 real stylesheets - Keep the selectors on rules added for color fallbacks and logical properties (#43240). 3 real stylesheets - A later declaration of a property drops the `@supports` color copies of the earlier one, which printed after the rule and won (#45560). 2 real stylesheets - Do not merge style rules in a stylesheet that holds `revert-rule` (#46953). 1 real stylesheets - `@import ... supports(-webkit-mask: none)` kept its prefix (#38589) **Properties and shorthands** - Print align-items:center without a stray space when no safe/unsafe keyword is given (#41993). 3,317 real stylesheets - Emit the physical longhands instead of the inset shorthand for targets without it (#38583). 1,452 real stylesheets - Keep the stops of an upward gradient as written, and drop a `50%` hint only at the midpoint (#46790). 104 real stylesheets - Emit unparsed border-block-* declarations when compiling logical borders (#38582). 89 real stylesheets - Keep repeated font families and dedupe only the inserted system-ui fallbacks (#46662). 88 real stylesheets - A `width` / `height` / `min-*` / `max-*` with `var()` or `env()` was printed before the plain fallback written above it, so the fallback won (#42101). 26 real stylesheets - Keep box-orient and box-direction when flex-direction or flex-flow follows (#45672). 16 real stylesheets - Write held prefixed background declarations at every flush (#46882). 6 real stylesheets - Derive equality for Size2D and BorderRadius, and keep every prefix in the border-radius handler (#45665). 2 real stylesheets - Print `none` instead of an empty border-image, mask-border or mask-box-image value (#45143). 2 real stylesheets - Keep font/transition sub-properties on the side of the shorthand they were written on (#41990). 2 real stylesheets - CSS-wide keywords are not names: `font:12px Arial;font-family:inherit` printed `font:12px inherit`, `color-scheme:inherit` and `color-scheme:auto` printed `normal`, `transition-property:unset` was folded into `transition` (#42022). 2 real stylesheets - Only fold font longhands into `font` when the rule declared a `font` (#45459) - Keep a font family name quoted when its identifiers do not read back as the same name (#46606) - Stop rewriting valid positions into ones browsers reject (#45197) - `border` is no longer built from longhands unless the rule also sets what `border` resets (`border-image`), and an author's `border-image` reset is no longer dropped. `transition` is likewise only written where the rule wrote one, because it resets `transition-behavior` (#42072) - `border; border-image: <gradient>; border-image-slice: 1` lost the gradient in every default build (#42018) - A prefixed `linear-gradient` keeps its direction, written or generated: the prefixed syntax names the starting side and measures angles differently (#46697) **At-rules** - Print @font-face format() keywords as strings (#37979). 439 real stylesheets - Print @supports `not` conditions without a leading space (#38559). 88 real stylesheets - Reset after the number token of a unicode-range (#45835). 33 real stylesheets - Keep `<general-enclosed>` media and container conditions as written (#46756). 24 real stylesheets - A strict range such as `(width < 768px)` lowered for old targets still matched at exactly 768px. The step is now `.02px`. The `not all and` form for other units is not taken (#46806). 18 real stylesheets - Keep scanning an @supports condition after a nested block (#38552). 9 real stylesheets - Every `@counter-style name` printed as `@counter-stylename`, which browsers drop (also #46692). In a CSS module the name now stays global, like the `list-style` that refers to it (#33679). 9 real stylesheets - Keep a keyframe whose block has an invalid item (#46288). 2 real stylesheets - Read an @Property initial-value on the stylesheet's parser, keep an unmatched value as written (#45806). 2 real stylesheets - `@media {}` is `all`. A media query that stops after `and`, `or` or `not` (`@media screen and {`) is a build error; `main` printed `not all` or dropped the condition, silently. The `@layer` and type selector parts are not taken (#46869). 2 real stylesheets - Accept a name-only `@container` and a `style()` feature with no value (#46689). 1 real stylesheets - Parse `<timeline-range-name> <percentage>` keyframe selectors (#45652) - Allow `or` inside parentheses after a media type (#45088) - Print the `=` comparison in media and container queries as `=` (#45071) - Parse the @font-palette-values font-family descriptor as a list of family names (#45484) **Selectors** - Never wrap a selector list in :is() (#46787). 414 real stylesheets - Stop emitting :-webkit-any() copies of :is() that outrank the original rule (#42029). 373 real stylesheets - With browser targets, a selector list with a vendor-prefixed member was split, which revives hacks that rely on other engines dropping the whole list: `.s *::-ms-backdrop,.s{flex:auto}` applied everywhere. Rewritten; the list is printed as written (#42100). 42 real stylesheets - Print the vendor prefix on downleveled :any-link, :read-only, :read-write, :placeholder-shown and :autofill (#42131). 12 real stylesheets - Fix nesting lowering of a `&` that does not lead its selector (#43047). 8 real stylesheets - `@scope`: `&` and implicit selectors compile to `:where(:scope)`, relative selectors and bare declarations parse (#43165). 7 real stylesheets - Only reject bare :global / :local pseudo-classes in CSS modules (#38534). 5 real stylesheets - Stop truncating `@scope to (...)` and stop replacing `&` in the scope-end selector (#43146). 2 real stylesheets - `[xl|href$=".pdf"]` lost its namespace prefix (#45159). 2 real stylesheets - Match An+B keywords exactly and take only `+` as the leading sign (#42953). 1 real stylesheets - Parse ::details-content and ::picker() like ::part() (#45504). 1 real stylesheets - Parse :window-inactive as an ordinary pseudo-class, and accept it after ::selection (#45815) - Print :nth-col(1) and :nth-last-col(1) in functional form (#41575) - `li::before::marker` failed the build (#45810) - `&` inside `:nth-child(... of S)` is resolved when nesting is compiled away. The second selector serializer (326 lines) is deleted (#43024) **Colors and `Bun.color`** - Keep light-dark() as written under the default browser targets (#42009). 227 real stylesheets - Number and percentage channels parse in `lab()`, `lch()`, `oklab()`, `oklch()`, `hsl()` and `hwb()`; the hue of `lch` / `oklch` is no longer scaled by alpha (#45084). 130 real stylesheets - `Bun.color` returns `null` for `currentColor`, system colors and `light-dark()` in the converting formats, instead of the CSS keyword as a string (#33047). 130 real stylesheets - Relative colors: channel keywords are on the scale of their function (`rgb(from #369 calc(r + 10) g b)`), the origin's alpha is kept, a relative color kept as written gets no `@supports` copies, and a result outside sRGB is not folded (#45362). 31 real stylesheets - Keep rotate: 0deg out of none, and keep the origin alpha in relative colors (#38486). 13 real stylesheets - Compute the D50 reference white in f32 so greys convert to lab(l 0 0) like lightningcss (#38535). 7 real stylesheets - Return a missing hue for a grey from conversions into lch, oklch, hsl and hwb (#45201). 5 real stylesheets - Fix color(a98-rgb), srgb-linear relative colors and color-mix(in xyz-d50) (#38487). 4 real stylesheets - Match named colors without regard to ASCII case (#45092). 3 real stylesheets - Fold min(), max() and clamp() over plain numbers (#39515) - `calc(l - .08)` in a percentage channel of a relative color became NaN and printed `#000` (also #45970) (#45971) - Bun.color: convert color() inputs instead of returning them as a CSS string (#38488) - A `color-mix(in srgb, ...)` result outside sRGB prints `color(srgb ...)` instead of a gamut-mapped hex (#39269) **`calc()` and numbers** - Rounding to 6 significant digits ate into the integer part: `z-index:2147483647` became `2147480000`, `max-width:9999999px` became `10000000px`, `--x:1234567.5` became `1234570`. It now only cuts a fraction short: a value kept as tokens prints its source text, a typed value prints every integer digit and at least one digit of the fraction (#46715). 43 real stylesheets - A number without a unit is no longer repaired into `px` (`height:1.5715` is dropped by a standards-mode browser, so it prints as written), `calc(-1 * 0)` is not a length, and on a property of the range [0,∞] (sizes, border radii, padding, border and outline widths, gap, `flex-basis`, `font-size`) a `calc()` that folds below zero prints `0` instead of an invalid negative literal, and a negative literal no longer replaces the valid declaration before it (`width:10px;width:-5px` printed `width:-5px`) (#42104). 24 real stylesheets - Print numbers from the f32's shortest digits, not its f64 expansion (#38565). 17 real stylesheets - Print negative numbers below 1 with a single minus sign (#38516) - Keep math functions over two different length units as written (#38489) - Fold rem() with a truncated quotient, so it keeps the sign of the dividend (#32286) - A one-argument `min()` / `max()` around a sum printed without any function (`width:100% - 20px`) (#46145) - `round()` breaks ties upward and handles a negative step as browsers do (#45249) **CSS modules** - A CSS module exports its `@keyframes` names (`styles.spin`). Hashing is unchanged (#46720). 145 real stylesheets - A name inside `:global(...)` prints plain but was exported as `name_<hash>`, a class no rule has (also #38573) (#45487). 90 real stylesheets - Keep container names global in CSS modules (#45247). 6 real stylesheets - Tag the keyframes name when an `animation` value falls back to raw tokens (#46034). 6 real stylesheets - In a plain stylesheet `composes: a from "./b.css"` was treated as an import: the file was inlined and an absolute path printed (#45961). 4 real stylesheets - Drop a rejected composes instead of failing the file at print time (#38538). 1 real stylesheets - Keep every rule's custom properties for the composes conflict check (#38523) - Keep composes working after a nested rule in the same block (#38520) - `IdentOrRef::hash` read 2 of 16 bytes, so in release builds every class of a module hashed alike (#30784) **Tokens, `url()`, diagnostics** - The block skip stack no longer allocates on every block exit (#39533) - Report the diagnostic's line without its line breaks in position.lineText (#38342). 192 real stylesheets - Compare `url()` values by their text (#46895). 26 real stylesheets - Values kept as tokens keep the blanks they were written with instead of having them re-invented at print time: `attr(x type(<number>))` printed `type(< number> )`, which browsers drop. Minified output still drops the blanks that cannot matter (#45577). 19 real stylesheets - The IE star hack (`*zoom:1`) failed the whole build. The declaration is skipped, as every browser does. It is the one syntax error that builds: tachyons, handsontable and jasmine ship it (#46751). 8 real stylesheets - The path of a copied asset spliced into `url()` is always quoted, so a file name with a space or a parenthesis cannot end the `url()` (#41557). 7 real stylesheets - Move the skip loop of parse_until_before out of the generic body (#39520) - Remove dead code from bun_css: the unused `impl Default for Property` (#45618) - Default the at-rule and qualified-rule parser traits, drop the url_to_css shim (#31999) **Speed** Four inputs that took quadratic or exponential time, or produced output out of proportion, are now linear, with no new limit and no extra pass (#31639, #42243, #38531, #43421). **Tests only** - `css-fuzz.test.ts` never ran in CI and timed out on debug builds; it is a deterministic table now (#38500) - `nth-anplusb-ident.test.ts` counts heaps in a child process (#46331) - `color.test.ts` checks the ansi-16 sweep with one `expect()` (#42824) - Not CSS, but it failed this PR's CI and fails on `main` too: `ws-proxy.test.ts` on Windows, "a message sent during a TLS renegotiation arrives after it". The helper server now renegotiates from the callback of the frame's write (#46170) **No pull request covered these** - `--v: 5.0.1` printed as `5.1`, `--w: 1.0.0` as `10`, `--x: 1+2` as `12`, `2n+1` as `2n1`: a number token that touches another keeps its source text. - `color-scheme: auto` printed as `normal`, and `color-scheme: dark light` as `light dark`. - A bare `:global` or `:local` in a CSS module sets the scope of the names after it, up to the end of the selector, as in css-loader. It was a build error. - The scrollbar states (`a:horizontal`, `::-webkit-scrollbar{&:horizontal{}}`) are valid in a plain compound, as in WebKit and Blink. - Without browser targets, `-webkit-box-align`, `-ms-flex-align` and the like were dropped next to their standard form (flatpickr, foundation-sites, materialize-css: 28 stylesheets), and `-webkit-fill-available` before `-moz-available` (beercss). - `(-o-min-device-pixel-ratio: 2/1)` printed as `2`. - `sin()`, `cos()` and `tan()` are exact at multiples of 90deg: `calc(100px * cos(90deg))` printed `-.00000437114px`. - A `display-p3` color at targets that have P3 but not the other `color()` spaces (Safari 10.1 to 14) got itself as a fallback. - `width: rem(-18px, 5px)` folds to a negative literal once `rem()` is right; it prints `0`. - Consecutive nested declarations rules, which arise when same-query `@media` blocks in one rule merge, are folded into one and count against the existing selector expansion limit as the `&{}` rules they replace did. #### Known and not fixed here - Four longhands are still folded into a `transition` the author did not write where the targets add a prefixed copy of a property (`mask-image` at Chrome 80): only the shorthand gives the copy its own times. - A later `background`, `box-shadow` or `border-*` declaration still loses to the `@supports` copy of an earlier `var()` value of the same property with a wide-gamut color. Same on `main`; #45560 is taken for the properties of the fallback handler and for unparsed values only. - A merged rule prints the unprefixed name in the passes of the rule it absorbed: `.d::-webkit-input-placeholder{}.d::-moz-placeholder{}` prints `::placeholder` for the second (#42106, not taken: it breaks the merge of prefix-equivalent rules). - The `border`, `outline` and `font` shorthands, `line-height`, `scroll-padding`, `background-size` and a few more still print a `calc()` that folds below zero as a negative literal. No real stylesheet has one. - `all` is not a property Bun knows, so `a{margin-top:1px;all:unset}` prints `all` first. Same on `main`. - A selector kept as raw tokens loses the blank before `*` when minified: `--sel: .dark *` prints `.dark*`. Same on `main`. - `width:calc(0% + 12px)` folds to `12px`, which lays out differently in a table cell. Same on `main`. - The `:dir()` / `:lang()` rules of a lowered logical property print before the nested rules of their rule, so a logical declaration written after a nested rule of equal specificity still loses to it. Same on `main`. - A fraction is still rounded to 6 significant digits (`12345.678` prints `12345.7`, `33.3333333333%` prints `33.3333%`). Keeping every digit would add up to 282 KB over the corpus. - `@keyframes "quoted"` is hashed in a module but not exported. - In a module, a keyframes name inside a `var()` fallback is not hashed: `animation: var(--enter, fade 1s)`. Same on `main`. - A `color-mix()` or relative color folded to a hex inside a gradient changes the interpolation from Oklab to sRGB (VS Code, clawhub). Printing `color(srgb ..)` instead needs a fallback for `mask-image`, `list-style-image` and custom properties at the default targets, which have none. - A comma list in `@container` still fails the build. - After a bare `:global`, names that are hashed at print time (`::view-transition-old(foo)`) are still hashed; inside `:global(...)` they are not. #### Pull requests in scope that do not land here - Duplicates of a pull request that lands here: #36961 (of #37979), #38478 (of #46662), #38480 (of #46733), #38482 (of #46715), #38491 (of #38500), #38508 (of #45201), #38513 (of #45362), #38573 (of #45487), #38640 (of #38500), #45525 (of #45247), #45589 (of #46756), #45970 (of #45971) - Already fixed on `main`: #38674 - Right, but for input nobody writes (0 of 10,116 stylesheets, and not new syntax): #31916, #38526, #38592, #39349, #41139, #41801, #42042, #42484, #43078, #45604, #45908, #46086, #46221, #46224, #46273, #46694, #46707, #46737, #46755, #46816, #46870, #46934 - Right or harmless, but not worth their code, their bigger output or the behavior they change: #30905, #31924, #33332, #34426, #36225, #38006, #38512, #38547, #38551, #38561, #38572, #38576, #38607, #38623, #38667, #38692, #42106, #42912, #42918, #43407, #43434, #44700, #45442, #45546, #45747, #45759, #45825, #45946, #46061, #46127, #46206 - Need a decision (see above), stay open: #40368, #40369, #44647, #44648, #45814 - Stays open: #45787 Out of scope: CSS linking and chunking in the bundler, the dev server, docs-only pull requests, and repository-wide sweeps that touch `src/css` in passing. Thanks to @mikamikasuki (#45247), @alii (#39520, #31999) and @oddharsh, whose measurements on #40369 and #41990 were used. Fixes #16727. Fixes #24770. Fixes #30772. Fixes #44704. Fixes #45089. Fixes #45237. Fixes #46872. ### How did you verify your code works? - The 35 CSS test files pass on a debug build, also with `FORCE_COLOR=1`. 600+ of the tests in this branch fail on a build of `main`. An earlier head of this branch passed all 179 CI jobs that ran. - The reproduction of each issue listed above was run on `main` (wrong) and on this branch (right). - Differential run over the 10,116 stylesheets, `main` against this branch, minified without targets and through `Bun.build` at the default targets: no stylesheet that builds on `main` fails; Chromium 156 compares the output with the input rule by rule (CSSOM, then computed values on a sized element in a standards-mode document, colors as painted pixels in sRGB and P3). The comparison has 195 control pairs that must read equal or different. For stylesheets that read differently on both builds, mostly because of lowering, the sets of differences were compared; the three new ones are two minifier fixtures that are wrong on both and one comparison artifact. - CSS modules are checked for consistency instead, since hashed names defeat the comparison: every reference points at a name the output defines, every export exists. - `Bun.color`: 59 inputs in 13 formats on both builds; every changed cell was read.
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
.a{background:red;background:}prints.a{background:0 0}. A browser drops that declaration.flex: ;asflex: 1. No user reported it (css: print the space after@counter-style, and hash the counter style name inlist-stylein CSS modules #46692 found it).Okwith defaults when no component matched, seeBackground::parse(src/css/properties/background.rs:39).Fix
background,transition,animation,mask,flexandflex-flowfail when their loop consumed no token (Parser::expect_advanced_from). The declaration stays unparsed and prints as written, likefont:.||group, which needs an operand. Chrome 153 drops these declarations.test/js/bun/css/css.test.ts(48 new tests, 38 fail on main). Alsotest/js/bun/css/,test/bundler/css/.Background
src/cssports, has the same defect.parse_value: all typed declarations pay, and it misses the empty list item.Downsides
animation:spin 1s,printed the working1s spin,noneand now prints as written. A maintainer must decide..arules lets the duplicate-rule check remove the first..a{animation:none}.b{color:red}.a{animation:}loses a reset that main kept by accident..text+1,024 B. Valid values of these shorthands: -15 to +12 instructions each. 0 of 70,034 npm stylesheets change.Notes
Repro.
bun build e.css --outdir=o --minify, main and 1.4.3:.a{background:red;background:}.a{background:0 0}.b{animation:spin 1s;animation:}.b{animation:1s spin;animation:none}.b{animation:1s spin;animation:}.c{transition:color 1s;transition:}.c{transition:all}.d{flex-flow:column;flex-flow:}.d{flex-flow:row}.e{mask:url(#m);mask:}.e{mask:url(#m);mask:none}.f{flex:2;flex:}.f{flex:1}.a{background:red;background:!important}.a{background:red;background:0 0!important}.a{transition:opacity 2s;transition:color 1s,}.a{transition:color 1s,all}.a{transition:color 1s;transition:;transition-duration:2s}.a{transition:all 2s}@container style(background:){.a{color:red}}style(background:0 0).g{font:12px a;font:},.h{border:1px solid red;border:}A value of blanks or of a comment behaves like the empty value.
--x:is not affected: an empty custom property is valid, and custom properties do not go through these parsers.Dev server.
Bun.serve({ development: true, routes: { "/": html } })with.card{flex:2;background:white}and a saved.card.on{flex: ;background: ;}. main serves.card.on { background: none; flex: 1; }, which the browser applies. This PR serves.card.on { flex: ; background: ; }, which the browser ignores.Browser check. Chrome 153 headless, the parsed rule text and the computed longhands. It drops the second declaration of each row above and keeps the first.
Scope. A sweep of all 366 names of the typed property table (with vendor prefixes), as
.x{NAME: }and.x{NAME:,}: on main 18 names parse the empty value and 12 of them parse the lone comma. With this PR none does. The testno known property parses an empty value or a lone commaruns this sweep from the table inproperties_generated.rs. An unparsed value keeps the blank and a typed value does not print it, so the test also fails for a parser whose empty value prints as nothing. It does not check that an earlier declaration survives: the 18-name tests do that.Reach. Both release builds, in one process each, over 71,304
.cssfiles of 8,167 npm packages (1.0 GB). 70,034 parse. 0 print differently, minified or not. One published file outside that sample changes:inventive@0.9.7,dist/css/components/datepickerold.css, hasbackground: ;twice. main printsbackground: none, this PR prints it as written. I found no report in the trackers of Bun or of Lightning CSS.The repair that goes away. main printed these invalid lists in a form that a browser accepts. They now print as written, with exit code 0 and no message, as every value that the typed parser rejects does:
.a{animation:spin 1s,}animation:1s spin,none, the animation runs.a{mask:url(#m),}mask:url(#m),none.a{background:,red}background:0 0,red, paints red.a{background:red,}background:0 0,0 0, paints nothing.a{transition:color 1s,}transition:color 1s,all, no transition startsStill wrong, in other layers.
StyleRule::is_duplicate,src/css/rules/style.rs:465) removes an earlier rule that has the same selector and the same property names. It does not read values..a{background:red}.b{color:red}.a{background:}prints.b{color:red}.a{background:}(main:.b{color:red}.a{background:0 0}). The same applies to two adjacent rules that cannot merge for the default targets, for example with:focus-visible. main does this forfont:,border:and every unparsed value. Adjacent rules that merge, and rules with an at-rule between them, keep both declarations. A todo test row marks the case..a{animation:none}.b{color:red}.a{animation:}. main prints.b{color:red}.a{animation:none}, this PR prints.b{color:red}.a{animation:}, and the reset is lost.color,text-shadowand the four logicalborder-*-radiusnames. The 12 size properties print the two declarations in the wrong order. Open PR css: keep an unparsed width/height/color after the declarations written before it #42101 changes those handlers.Why the position and not the
Options.GenericBorder::parsekeeps ananyflag andBorderImagetests eachOption. The six parsers have 31 loop arms. The position compare adds two lines per parser and has no list that a new arm must join. It is exact here because every component attempt is atry_parse, which restores the position when it fails.Measurements. Release builds of a8b5e92 (the merge base when I measured) and of the same commit with this change, linux x64.
bloaty,perfandvalgrindare not installed here. Sizes come fromllvm-nm -Sandllvm-size -A, instruction counts fromperf_event_open(instructions:u) aroundbun build --no-bundle <sheet>, medians of 5 interleaved runs on 100,000-rule sheets. A release build of the final commit has the same six symbol sizes..text82,257,737 B to 82,258,761 B (+1,024 B). Of 2,720bun_csssymbols, 6 change size (+1,233 B): theparse_value::<T>instantiations forMask(+355),Background(+282),Flex(+183),Transition(+175),FlexFlow(+125) andAnimation(+113). The new code is the failure path: it drops the component locals and builds the error.Property::parse,Parser::expect_exhausted,parse_comma_separated_internaland the other 130parse_valuesymbols is the same in both builds, with addresses normalized.transition:color 1s+11.0, a three-itemtransition+10.0,animation:spin 1s+12.0,flex:1+7.0,flex-flow:column+7.5,background:red-11.0, a seven-componentbackground-15.0,mask:linear-gradient(red,blue)-4.0. A rule of these sheets costs 10,000 to 30,000 instructions.color+655,margin+168, a custom property -63,will-change+410, all below the range of the runs (494 to 162,910).tailwind.css-11 andbulma.css-371, also below the range.background:var(--x)-274,transition:var(--x)-159,flex:initial-145 instructions per value.background:red;background:+2,088 instructions and +1 allocation per rule (two declarations are kept and printed).transition:opacity 2s;transition:color 1s,+462 instructions and -9 allocations per rule.bulma.css75,002 in both builds, 10,000 rules oftransition:color 1sor ofbackground:red61,280 in both.test/js/bun/css/files, with and without--minify: 0 of 30 files differ.Other designs.
parse_valueand in the comma-separated list parser. It covers every typed property, also later ones. It leaves the six parsers returning a value built from no token, and 130 of 136 value types pay for a check they cannot fail.flexandflex-floware not lists.Not in this PR.
flex: 2 3 4,flex: 1 2 calc(0)andtransition: -1s -2s(open PR css: treat out-of-range and unit-less numeric values as invalid instead of merging or repairing them #42104),transition: initial 1s(open PR css: keep css-wide keywords and percentage stretch out of the font and transition shorthands #42022), andbackground: red, green(the note inBackground::parse).composes:with an empty value is a parse error. It does not go through these parsers.list-style, have the same loop. Open PR css: print the space after@counter-style, and hash the counter style name inlist-stylein CSS modules #46692 portslist-styleand writes its own check for the empty value. It can callParser::expect_advanced_from.Self-review, the three steps not done.
.x{NAME:var(--x);NAME:}in the sweep, with a skip list for the six names above. They test the handlers of 348 names that this PR does not change, and css: keep an unparsed width/height/color after the declarations written before it #42101 adds that sweep.Suites run with the debug build.
test/js/bun/css/css.test.ts(1362 pass, 69 skip, 4 todo, 0 fail),test/bundler/css/andtest/bundler/esbuild/css.test.ts(241 pass, 1 todo, 0 fail),test/js/bun/css/doesnt_crash.test.ts(61 pass). Intest/js/bun/css/, the 4 tests ofcss-fuzz.test.tstime out in my container with the debug build. The same 4 time out on main.