Repository navigation
Conversation
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Status: fixed, stacked on #38553, awaiting CI and review. Reproduced on bun 1.4.0 ( With this branch the first prints This PR's base is #38553's branch: an earlier revision on main folded Tests: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline finding, the same mixed-variant shape was checked for round()/mod()/rem()/hypot() over a channel keyword and a percentage — those go through apply_op/try_op, not reduce_args, and were already None for mixed Number/Value before this change, so no regression there.
Extended reasoning...
The one inline nit covers the only regression surfaced: min(r, 50%) (bare keyword vs bare percentage) stops folding because the Calc<Percentage> pass now yields a Calc::Number for the keyword and reduce_args gained Number↔Number but not Number↔Value. The sibling two-arg math functions (round/mod/rem/hypot) were examined for the same class and ruled out — they route through apply_op, which already only matched (Value,Value) or (Number,Number), so their behavior on mixed forms is unchanged by this PR.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM. My earlier note about min(r, 50%) no longer folding is resolved — the response is convincing (it's the same <number> vs <percentage> type error as calc(r + 50%), browsers reject it, lightningcss leaves it unparsed byte-for-byte), and both the rejected and the percentage-typed accepted forms are now pinned in the tests. The comment-cop note is also addressed (one line at color.rs:2130).
Extended reasoning...
Overview
Three focused source changes plus ~140 lines of new tests:
src/css/values/color.rs: theCalc<Percentage>pass inRelativeComponentParser::parse_number_or_percentagenow resolves a channel keyword toCalc::Number(v)instead ofCalc::Value(Percentage { v }), sor * 50%is number × percentage and folds.src/css/values/calc.rsreduce_args: restructured to also compareCalc::NumberagainstCalc::Number(previously onlyCalc::ValuevsCalc::Value). I traced the old and new control flow — the Value/Value path, the incomparable-None path, and the non-Value-arg path are all preserved; only the Number/Number arm is new. This keepsmin(r, g)folding after the keyword became a Number and additionally foldsmin(1.5, 1.2).src/css/values/calc.rsPercentage::from_calc: non-Calc::Valuenow returns a parse error instead ofPercentage { v: NaN }, matching the adjacentAngleandTimeimpls. This turnsopacity: calc(50% + 1)from silently minifying to0into a passthrough.- Tests in
color.test.tsandcss.test.tscover the product forms, everycolor()space, alpha in all eight color functions, min/max over keywords with and without literals, the type-error rejections, and thefrom_calcfix.
Security risks
None. Pure CSS value parsing/folding; no I/O, no untrusted-length arithmetic, no allocation sizing from external data. The from_calc change moves toward fail-safe (error instead of NaN sentinel).
Level of scrutiny
Medium. The diff is small and mechanical, but it intentionally changes what folds vs. what passes through unparsed. That semantic decision was the substance of my earlier review comment; the author's response cites CSS Color 5 / CSS Values 4, shows lightningcss 1.30.2 emits identical bytes for every case in the before/after table, and pins both sides in tests. The reduce_args restructuring applies to every Calc<V>, so I checked it is behavior-preserving for the pre-existing arms.
Other factors
- My prior inline comment is marked resolved and the follow-up commit (2821e19) added the mixed-form rejections (
min(r, 50%),round(r, 30%),hypot(r, 30%),max(alpha, 30%),calc(alpha - 20%)) plus the percentage-typed forms that do fold, to both test files. - The github-actions comment-cop note is addressed — the keyword comment at color.rs:2130 is one line.
- PR description states 49/56 new
color.test.tscases and 24 newcss.test.tscases fail on the unfixed build, and the remaining new cases pin behavior that must not change (theUSE_SYSTEM_BUN=1invariant). parse_percentage(the sibling with the sameCalc::Valueshape) is intentionally left alone with a stated reason in the description; that's a separate pre-existing bug.
In relative color syntax a channel keyword is a <number>, so calc(r * 50%) is a percentage of the channel. The percentage pass of RelativeComponentParser only knew the keyword as a Percentage value, which made that a percentage times a percentage; the color failed to parse, and rgb()/hsl() with such an alpha were emitted as rgb(255 0 0/calc(alpha*50%)), which browsers reject. The pass now evaluates the expression with the keywords as the <number>s of the function's range first, and only then retries it with the keywords standing for a percentage of their range, which keeps the calc(l + 15%) form bun has always accepted working. Percentage::from_calc returned NaN instead of failing when a <number> was added to a <percentage>. The first pass relies on that being an error to fall through, and it also stops opacity: calc(50% + 1) from minifying to 0; the declaration is left as written like Angle and Time already do.
c143e7d to
1d67098
Compare
| RelativeComponentParser::parse_percentage_calc(input, this, |ctx, ident| { | ||
| let (unit_value, _) = ctx.get_ident(ident, ChannelType::NUMBER)?; | ||
| Some(Calc::Value(Box::new(Percentage { v: unit_value }))) | ||
| }) { | ||
| }) |
There was a problem hiding this comment.
🔴 This fallback wraps each keyword's stored component as Percentage{v: unit_value} and discards its range, so in lab()/lch() — where l has range 100 but a/b/c have range 1 — lab(from lab(50% 20 30) min(l, a) a b) compares 0.5 vs 20 and folds to lab(50% 20 30) instead of the spec-correct lab(20% 20 30); before this PR a was CT_NUM, didn't match ChannelType::PERCENTAGE, and the color was left unparsed for the browser to compute correctly. Separately, the description's reduce_args bullet (Number↔Number folding, min(r, .5), line-height: min(1.5, 1.2), min(NaN, .5), min(10px, 1.5)) and its tests are not in the diff — restoring that arm would fold min(l, a) correctly in the Calc<f32> pass and make this fallback unreachable for keyword-only min/max; otherwise this pass should compare range-scaled values (or reject when the arguments' ranges differ) and the description/table should be updated.
Extended reasoning...
What the bug is
The second-pass fallback of RelativeComponentParser::parse_percentage (color.rs:2202-2205) resolves each channel keyword via get_ident, takes the stored component (unit_value), discards the returned range, and wraps it as Calc::Value(Percentage{v: unit_value}). This is what lets min(r, g) fold when reduce_args cannot compare Calc::Numbers, and it is only correct when every keyword in the expression has the same range. This PR introduces per-channel ranges, and in lab()/lch() lightness has range 100 while a/b/c (and alpha) have range 1 — so min(l, a) compares 0.5 against 20 and picks the wrong argument.
Code path
For lab(from lab(50% 20 30) min(l, a) a b), the lightness position calls parse_unit_channel(i, 100). parse_lab sets from.ranges = (100, 1, 1); the origin stores components = (0.5, 20, 30, 1.0).
self.parse_number→parse_calc<f32>(color.rs:2234-2246):l→get_number→ 0.5×100 =Number(50);a→ 20×1 =Number(20).reduce_args(calc.rs:1090) only matchesif let Calc::Value(val) = &*arg, so bothNumbers survive →Calc::Function(Min)→ rejected (onlyCalc::Value/Calc::Numberaccepted at 2241-2246).parse_percentagepass 1 (color.rs:2196-2200, keyword→Calc::Number(get_number(...))): identical[Number(50), Number(20)]→Function→parse_percentage_calc(2214) only acceptsCalc::Value→ rejected.parse_percentagepass 2 (color.rs:2202-2205):l→get_ident→(0.5, 100), discards 100, wraps 0.5;a→get_ident→(20, 1), discards 1, wraps 20.reduce_argscomparesPercentage{0.5}vsPercentage{20}: pushes 0.5, thenpartial_cmp(20, 0.5) = Greater ≠ Less→ drops 20. ReturnsPercentage{0.5}→Ok(0.5).parse_unit_channelreturns 0.5 →lab(50% 20 30).
Why nothing else catches it
The Calc<f32> pass computes the correct numbers (50, 20) but cannot fold them because reduce_args has no Calc::Number arm. The first Calc<Percentage> pass has the same problem. Only the second pass folds, and it does so on the wrong axis. Every existing min/max-of-keywords test in the PR (min(r, g) in color()/rgb(), min(s, l) in hsl(), max(w, b) in hwb()) uses keywords whose ranges are equal, so stored-value comparison happens to give the same answer as range-scaled comparison.
Regression
Before this PR, LAB had types = (CT_PCT, CT_NUM, CT_NUM) and the old parse_percentage looked up keywords with ChannelType::PERCENTAGE, so a (CT_NUM) returned None from get_ident, the Calc<Percentage> pass errored, and the whole color fell back to unparsed tokens — the browser then computed min(50, 20) = 20 → lab(20% 20 30). After this PR, all channels are CT_NUM and the lookup uses ChannelType::NUMBER, so a resolves and bun emits a wrong constant — strictly worse than the pre-PR passthrough, and exactly the failure mode the PR description warns against for rgb() alpha. The same applies to lch(from ... min(l, c) c h). (min(l, alpha) was already wrong pre-PR since alpha matched PERCENTAGE; min(l, a)/min(l, b)/min(l, c) are newly broken.)
The stale reduce_args claim
The PR description's Fix section says "reduce_args: min()/max() also reduce Calc::Number arguments against each other … min(r, .5) folds too … line-height: min(1.5, 1.2) minifies to 1.2", and the Verified section lists min(r, .5), min(NaN, .5), min(10px, 1.5) as pinned tests. None of that is in the diff: the calc.rs hunk only touches Percentage::from_calc; reduce_args at ~1090 still has no Calc::Number arm; grep of test/js/bun/css/ finds none of those four cases; and the doc comment on the new parse_percentage ("min(r, g), which reduce_args only folds over values") confirms the author knows reduce_args was not changed. So min(r, .5) does not fold (both passes end with mixed [Number, Number] or [Value, Number] args), line-height: min(1.5, 1.2) stays min(1.5,1.2), and the before/after table's rows for both are wrong. The review timeline (claude[bot] at 13:59 references "the new reduce_args arms", then a fallback-pass redesign landed at 19:35) suggests the reduce_args change and its tests were dropped during the rework and the description was never updated (CLAUDE.md #11: "NEVER overstate what you got done").
Step-by-step proof
Input: lab(from lab(50% 20 30) min(l, a) a b).
Per CSS Color 5 the channel keywords are <number>s in the function's own range: l = 50, a = 20. min(50, 20) = 20. In the lightness position 20 means l = 20/100 = 0.2 stored → lab(20% 20 30). Chrome/Firefox and lightningcss (which leaves it unparsed) agree.
Bun after this PR: pass 1 → [Number(50), Number(20)], reduce_args pushes both → Function(Min) → rejected. Pass-2 Calc<Percentage> (Number keyword): same → rejected. Pass-2 fallback (Value keyword): [Value(Pct{0.5}), Value(Pct{20})]; reduce_args pushes 0.5, then compares 20 vs 0.5 → Greater, order is Less, so 20 is dropped. Reduced to [Value(0.5)] → returns 0.5 → lab(50% 20 30). Wrong.
How to fix
Two independently sufficient options:
- Restore the dropped
reduce_argschange (aCalc::Number↔Calc::Numbercomparison arm, plus the four claimed tests). Then theCalc<f32>pass foldsmin(l, a)toNumber(20),parse_calcaccepts it,parse_unit_channelreturns20/100 = 0.2→lab(20% 20 30), and the buggy fallback is never reached for keyword-onlymin/max. This also delivers what the description already claims (min(r, .5),line-height: min(1.5, 1.2)). - Or, keep
reduce_argsas-is, correct the description/table, and make this fallback compare on the range-scaled axis — e.g. wrapget_number(ident, ChannelType::NUMBER)? / <range of the position being parsed>as thePercentagevalue so all keywords land on the same scale — or simply reject (returnNone) when the arguments' ranges differ so the color falls back to unparsed tokens as before.
Either way, add lab(from lab(50% 20 30) min(l, a) a b) (and the lch analogue) to the test matrix.
| const spaces: [space: string, channels: string, printed?: string][] = [ | ||
| ["srgb", "g b"], | ||
| ["srgb-linear", "g b"], | ||
| ["display-p3", "g b"], | ||
| ["prophoto-rgb", "g b"], | ||
| ["rec2020", "g b"], | ||
| ["xyz-d50", "y z"], | ||
| ["xyz-d65", "y z", "xyz"], | ||
| ["xyz", "y z"], | ||
| ]; |
There was a problem hiding this comment.
🟡 The spaces array covers 8 of the 9 predefined color() spaces whose channel types this PR changes from CT_PCT to CT_NUM, but omits a98-rgb (the A98 struct at color.rs:1826 received the identical change). Add ["a98-rgb", "g b"] to complete the matrix per REVIEW.md "Cover the variant matrix, not just the repro".
Extended reasoning...
What the gap is
The spaces test.each array (test/js/bun/css/color.test.ts:578-587) enumerates the predefined color() spaces to verify calc(<channel> * <percentage>) works in each: srgb, srgb-linear, display-p3, prophoto-rgb, rec2020, xyz-d50, xyz-d65, xyz. It omits a98-rgb, which is the ninth predefined space the CSS Color 4 spec defines and whose types tuple this PR changes from (CT_PCT, CT_PCT, CT_PCT) to (CT_NUM, CT_NUM, CT_NUM) at src/css/values/color.rs:1826, identically to the other eight.
Why this is a finding
REVIEW.md → Tests reviewers reject → "Cover the variant matrix, not just the repro. Every sibling entry point receiving the same fix". The PR description explicitly claims coverage of "color() in every space", and A98 is one of the 9 define_colorspace! invocations receiving the CT_PCT → CT_NUM change. The test whose stated purpose is "verify this works in every color() space" should list every space it received the change in.
Why nothing else covers it
The other new test blocks in this PR use specific spaces (srgb, display-p3, xyz) as representatives; only the spaces array is the exhaustive per-space sweep. a98-rgb appears once in css.mdx as a documentation example but nowhere in the new test coverage.
Impact
Low. All predefined color() spaces route through the identical parse_predefined → parse_number_or_percentage code path; the only per-space input to that path is the types() tuple, and every tuple was changed uniformly. There is no distinct code path for a98-rgb that could diverge from the other 8. The missing entry only guards against a future accidental revert of A98's types tuple — it does not hide any correctness risk today.
Step-by-step proof of the omission
src/css/values/color.rsdiff changestypeson 9define_colorspace!invocations that participate incolor():SRGB,SRGBLinear,P3,A98,ProPhoto,Rec2020,XYZd50,XYZd65(plusLAB/LCH/OKLAB/OKLCH/HSL/HWBwhich are covered by the separate per-function test block).- Of the predefined-
color()-space subset, thespacesarray lists 8:srgb,srgb-linear,display-p3,prophoto-rgb,rec2020,xyz-d50,xyz-d65,xyz(the last two both map toXYZd65, so 7 distinct structs). a98-rgb(→A98) is the only predefinedcolor()space in the diff not present in the array.A98 { r, g, b }uses the same channel names as the other RGB-like spaces, so the entry is["a98-rgb", "g b"].
How to fix
Add one line to the spaces array:
const spaces: [space: string, channels: string, printed?: string][] = [
["srgb", "g b"],
["srgb-linear", "g b"],
["display-p3", "g b"],
["a98-rgb", "g b"],
["prophoto-rgb", "g b"],
...
|
Heads-up on the overlap with #38553 (the keyword value problem this PR's body splits out): that PR merges |
|
Overlap: #45362 replaces #38553, the base of this PR, and carries one source line of this PR ( |
### 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.
Stacked on #38553 (this PR's base branch); the diff shown here is only this change. It needs #38553 because the values it folds are the keyword numbers that PR introduces.
Problem
Bun.color("color(from red srgb calc(r * 50%) g b)", "css")isnull, and in a stylesheet the declaration is kept as an unparsed token stream (no folding, no fallbacks).calc(r * 0.5)works;calc(50% * r),min(r * 50%, 30%),calc(l * 1%)andcalc(alpha * 50%)do not, in every color function. lightningcss 1.30.2 folds all of them.rgb()/hsl()/hwb()it is worse than not folding:rgb(from red r g b / calc(alpha * 50%))is emitted asrgb(255 0 0/calc(alpha*50%)), which no browser accepts, because the path that keeps an unparseable alpha verbatim (how/ var(--alpha)is supported) takes over.RelativeComponentParser::parse_percentage(theCalc<Percentage>pass,src/css/values/color.rs) only knows a keyword as aPercentagevalue.r * 50%is then a percentage times a percentage, whichCalc::parse_productrejects (one operand has to be aCalc::Number).Percentage::from_calc(src/css/values/calc.rs) returnedPercentage { v: NaN }for anything that is not a plain value, so adding a number to a percentage silently produced NaN:opacity: calc(50% + 1)minifies toopacity:0today andcolor(srgb calc(1 + 50%) 0 0)tocolor(srgb none 0 0).Fix
parse_percentageevaluates the expression twice at most: first with each keyword as the<number>css: resolve relative color channel keywords as numbers in the function's range #38553 gives it (get_number:ris 200 inrgb(),lis 50 inlab(),.8incolor(srgb ...)), which is what CSS Color 5 says the keyword is and foldscalc(r * 50%)to a percentage of the channel; if that fails, with the keyword as a percentage of its range, which is exactly the evaluation the pass did before. The second pass keepscalc(l + 15%)(the example indocs/bundler/css.mdxuntil css: resolve relative color channel keywords as numbers in the function's range #38553 reworded it),calc(alpha - 20%),min(r, 50%)andmin(r, g)meaning what they meant; nothing that folded before stops folding or changes value. The helperparse_percentage_calcis the old body, parameterized on the keyword callback.Percentage::from_calcfails like theAngleandTimeimpls next to it instead of producing NaN. The first pass relies on this: with NaN,calc(l + 15%)would come out of the first pass as a NaN channel instead of reaching the second one. It also turnsopacity: calc(50% + 1)andcolor(srgb calc(1 + 50%) 0 0)into declarations left as written, which is what a browser does with them. Upstream's equivalent conversion isunreachable!()(it panics onopacity: calc(50% + 1)); the NaN came from the Zig port.100%is 255 inrgb(), 100 inhsl()/lab(), 1 inoklab()/color()/alpha). Every value asserted in the first three test groups is byte-identical to lightningcss 1.30.2 output, includingrgb(from rgb(200 100 50) calc(r * 50%) g b)saturating to#ff6432,calc(r * 0.1%)giving#336432, andlab(from lab(50% 20 30) calc(l * 1.2%) a b)givinglab(60% 20 30). An earlier revision of this PR sat on main and fed the stored unit values into the multiply, which gave#646432for the first of those; that is why it is stacked now.bun bd test test/js/bun/css/color.test.ts(blockrelative color calc() with percentages: 13 expression shapes, everycolor()space, channel and alpha positions in all eight functions, the forms that keep their old meaning, and the forms that are invalid either way) andbun bd test test/js/bun/css/css.test.ts(the same through the minifier, plus theopacitycases incalc edge case). With this PR'ssrc/changes removed (base branch alone), 43 of the 53 newcolor.test.tscases and 22 of the 28 newcss.test.tscases fail; the rest pin behavior that must not change. All oftest/js/bun/css/,test/bundler/css/and the css regression tests pass on the stacked branch (2545 tests), including every test css: resolve relative color channel keywords as numbers in the function's range #38553 adds.Background
rgb(from <origin> r g b / alpha),color(from <origin> srgb r g b)) lets a channel be written as a keyword naming the origin's channel, or as a math function over those keywords. CSS Color 5 defines the keywords as<number>s in the range of the function being written (rinrgb()is 0..255, incolor()0..1); css: resolve relative color channel keywords as numbers in the function's range #38553 is what makes bun resolve them that way.Calc<V>is the parsed form of a math function over values of typeV.Calc::Numberis a dimensionless number andCalc::Value(V)a typed value;*needs oneCalc::Numberoperand,+converts both sides toVthroughfrom_calc, andmin()/max()fold throughreduce_args, which only comparesCalc::Values (this is whymin(r, g)folds in the second pass and not the first).Calc<f32>(all numbers;parse_number) and then asCalc<Percentage>(parse_percentage, the function this PR changes); the consumers divide a number by the channel's range and take a percentage as the stored unit value directly.rgb()/hsl()/hwb(), whose unparsed-alpha path resolves the channels and prints the alpha tokens as they were.Before / after (bun build --minify; the last column is also what lightningcss 1.30.2 emits)