Repository navigation
Conversation
Collaborator
Author
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe change updates mixed-unit length operations. It corrects independent operand conversion and adds CSS tests for folding, unresolved expressions, at-rule errors, transforms, ChangesCSS mixed-unit length math
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: preserving CSS math functions that use different length units instead of folding them incorrectly. |
| Description check | |
The description provides detailed problem context, implementation changes, verification results, known limitations, and test coverage. It does not use the template headings exactly, but it includes th… |
| 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: Description check
-
Explanation
The description provides detailed problem context, implementation changes, verification results, known limitations, and test coverage. It does not use the template headings exactly, but it includes the required information and is substantially complete.
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.
LGTM — obvious copy/paste fix (self.to_px() → rhs.to_px()) with thorough test coverage.
What was reviewed:
- Verified the fix matches the pattern already used by
LengthValue::try_addandLengthValue::partial_cmpin the same file, and that the same-unit branch immediately above already readsrhs.value(). - Spot-checked test arithmetic (
round(96px, 10px)=100px,rem(96px, 10px)=6px,hypot(96, 10)≈96.5194,atan2(96, 0)=90deg) and theborder-spacing: 6px 6px→6pxcollapse against existing border-spacing tests. - Confirmed the non-convertible-rhs cases now correctly fall through to
Nonesinceto_px()returnsNoneforem.
Extended reasoning...
Overview
Two-line change in src/css/values/length.rs: the different-units branch of impl protocol::TryOp for LengthValue and impl protocol::TryOpTo for LengthValue previously called self.to_px() for both operands (with a comment claiming this was intentional "for behavioral compatibility"). The fix changes the second call to rhs.to_px() and deletes the misleading comments. Accompanied by 25 new minify_test cases in test/js/bun/css/css.test.ts.
Security risks
None. This is constant-folding arithmetic in the CSS minifier over f32 length values; no untrusted-input parsing, allocation, or I/O is touched.
Level of scrutiny
Low. The fix is mechanical and self-evidently correct: (1) the same-unit branch two lines above already passes rhs.value() as the second argument to f, (2) the inherent LengthValue::try_add and LengthValue::partial_cmp in the same file already use rhs.to_px()/other.to_px() in their identical different-units branches, and (3) the PR verifies output against lightningcss 1.30.2 (the reference implementation this code is ported from). The removed comments were documenting a bug, not a feature.
Other factors
Test coverage is well above the bar for a two-line fix: it exercises both changed impls (TryOp via round/rem/mod/hypot on width and border-spacing; TryOpTo via atan2), both operand orders, rounding strategies, both <length> and <length-percentage> property paths, and the newly-correct None fallthrough when rhs is a relative unit. The PR description confirms 24/25 fail with USE_SYSTEM_BUN=1 and all pass with the fix, and that the rest of the 1167-test file still passes. The out-of-scope items (3+-arg hypot, clamp #32290) have distinct root causes and are correctly deferred.
Contributor
There was a problem hiding this comment.
LGTM — obvious copy/paste fix (self.to_px() → rhs.to_px()) in the mixed-unit branch of TryOp/TryOpTo.
What was reviewed:
- The two changed lines now match the sibling
try_add/partial_cmpimpls in the same file, which already convertrhs; also matches lightningcss. - Spot-checked the test expectations arithmetically (e.g.
round(96,10)=100,hypot(96,10)≈96.5194,atan2(96,10)≈1.467rad) — all correct. - Tests cover both operand orders, both
TryOp(round/rem/mod/hypot) andTryOpTo(atan2),<length>and<length-percentage>properties, and the non-convertible (em) case that must stay unevaluated. - No CODEOWNERS on
src/css/; no memory-safety or security surface.
Extended reasoning...
Overview
Two-token fix in src/css/values/length.rs: the different-units branch of impl protocol::TryOp for LengthValue and impl protocol::TryOpTo for LengthValue was calling (self.to_px(), self.to_px()) instead of (self.to_px(), rhs.to_px()), so two-operand CSS math functions (round(), rem(), mod(), hypot(), atan2()) folded using the first operand twice when the operands had different absolute units. The two comments claiming this was intentional are removed. A 25-case test block is added to test/js/bun/css/css.test.ts.
Security risks
None. This is pure constant-folding arithmetic in the CSS minifier over already-parsed numeric values; no untrusted input handling, allocation, or FFI is touched.
Level of scrutiny
Low. The bug is self-evident from reading the line — a binary operation cannot use the same operand for both sides. The fix is mechanical and mirrors the neighboring LengthValue::try_add and LengthValue::partial_cmp, which already use rhs.to_px() in their different-units branch. The reference implementation (lightningcss) does the same, and the PR's before/after table matches lightningcss output for every case.
Other factors
- Test coverage is thorough: both
TryOpcallers (round/rem/mod/hypot) and theTryOpTocaller (atan2), both operand orders, rounding strategies, both<length>(border-spacing) and<length-percentage>(width) properties, and the previously-broken case where the second operand is a non-convertible unit (em) — which now correctly returnsNoneand leaves the function unevaluated. Author confirmed 24/25 cases fail withUSE_SYSTEM_BUN=1and all pass with the fix. - I verified several expected values by hand:
round(96,10)=100,rem(96,10)=6,mod(96,25)=21,sqrt(96²+10²)≈96.5194,sqrt(96²+16²)≈97.3242,atan2(96,0)=90°,atan2(0,-96)=180°,atan2(96,10)≈84.05°≈1.467rad. - No CODEOWNERS entry for
src/css/. No outstanding reviewer comments. The PR explicitly scopes out two unrelated bugs (3+-arg hypot, clamp) with different causes.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Aug 19, 2026
Replaces #32290. #39515 (folding `min()`, `max()` and `clamp()` over plain numbers) is stacked on this PR and adds the number case to the helper introduced here. ### Problem - The CSS minifier drops the lower bound of a `clamp()` whose center is not above its maximum. `width: clamp(10px, 5px, 20px)` prints `width:min(20px,5px)`, which is 5px instead of 10px. `clamp(10%, 5%, 20%)` prints `min(20%,5%)`, and `clamp(1em, 2px, 3px)` prints `min(3px,2px)`, dropping the `1em` bound that may be the larger one. Same on bun 1.4.0 and main. Reported in #32290. - When the center is above the maximum, the result is left as `max(10px,20px)` instead of `20px`, because the minimum is never compared. - Cause: the `clamp()` arm of `Calc::parse_with` in `src/css/values/calc.rs` compares the center with the maximum and, when the center is not above it, sets `min = None` (`calc.rs:370` on main) where it means `max = None`. The `(None, Some(max))` arm of the match below (`calc.rs:379`) then prints `min(max, center)`. lightningcss does not have this bug and also compares the center with the minimum afterwards, this came in with the port. ### Fix - The arm is restructured around the definition `clamp(min, center, max) = max(min, min(center, max))`. If the center and the maximum are comparable, the maximum is applied (the center becomes the maximum when it is above it), then the minimum: a comparable minimum folds the result to one value, an incomparable one is kept as `max(min, center)`. If the center and the maximum are not comparable, the `clamp()` is kept unchanged, as before and as upstream. - The two comparisons go through a new `Calc::partial_cmp_args`, which orders two `Calc::Value` arguments through the existing `PartialCmp` and returns `None` for anything else, so a `Calc::Number`, a sum or a nested function is never compared, exactly as before. The `Option` bookkeeping is gone, and with it the `min(max, center)` output, which only the bug produced. - Every asserted output is the one lightningcss 1.33.0 prints for the same input, including the eleven rows taken from its own calc tests. - Tests: `test/js/bun/css/css.test.ts`, new `clamp() simplification` block: lightningcss's rows (`<length>` through `border-width`, sums as arguments, unit conversion, an incomparable center, an incomparable minimum, a sum as a center or a bound), the center equal to either bound, the minimum winning over a smaller maximum with the center between the bounds and, in the two rows of the clamp() WPT, above both (these only pass when the minimum is compared with the center after the maximum has replaced it), #32290's incomparable lower bound with the center below and above the maximum, an incomparable maximum with a comparable minimum, and `%`, `deg`, `s` and a `clamp()` inside `calc()`. 18 of the 24 rows fail on the released binary, all 24 pass with the fix. - Also run: the rest of `css.test.ts` and `test/bundler/css/` (1348 pass), `cargo clippy -p bun_css` clean. ### Background - `Calc<V>` is the parsed form of a math expression for a value type `V` (`Length`, `Percentage`, `Angle`, ...). `Calc::Value` holds a `V` and two of them can be ordered at parse time when their units convert into each other (`px` and `pt`, not `px` and `em`). `Calc::Number` holds a bare number. Comparing numbers is #39515. - Unparsed fallback: when a property's value parser rejects a value, bun keeps the declaration as the tokens it was written as. `rotate: clamp(0deg, 45deg, 30deg)` took that path on main (the `max()` the bug produced is not a value an `<angle>` accepts), which is why it printed unchanged rather than wrong. `width` accepts any math function, which is why it printed the wrong `min()`. <details><summary>Notes</summary> - Related but not fixed here: `clamp(1px , 2px , 3px)` (a space before a comma) is not simplified at all, because the `clamp()` arm reads each argument with `parse_sum`, which stops at whitespace that is not followed by `+` or `-`, while `min()`/`max()` read theirs through `parse_comma_separated`. The declaration is kept as written, so the output is correct, only longer. lightningcss 1.33.0 behaves the same. It is a parsing change with its own tests and belongs in its own PR. - Eleven of the rows are lightningcss's own `test_calc` clamp rows. bun has none of lightningcss's `test_calc`, `test_math_fn` or `test_trig` tables in `css.test.ts`; porting them as one block would give the open calc.rs PRs (#32286, #38489, #38501, #38639, #38654, #39515) a shared oracle instead of a describe block each. That is a test-only change and is not part of this PR. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 18 failed, 67 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/css/css.test.ts bun test v1.4.0 (8326d1b) test/js/bun/css/css.test.ts: Output .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } (pass) css tests > .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } [3.18ms] Output .foo { content: "+"; } (pass) css tests > .foo { content: "\2b"; } [1.04ms] Output div { --foo: 1 1 1 #0101011a, 2 2 2 #02020233; --bar: 1 1 1 #01010116, 2 2 2 #02020233; } (pass) css tests > custom property cases > div { --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2); --bar: 1 1 1 #01010116, 2 2 2 #02020233; } [1.02ms] Output :root { --my-color: red; --my-bg: white; --my-font-size: 16px; } .element { color: var(--my-color); background-color: var(--my-bg); font-size: var(--my-font-size); } (pass) css tests > custom property cases > :root { --my-color: red; --my-bg: w ... (truncated) release without fix: 67 skipped bun test v1.4.0-canary.1 (9c30321) test/js/bun/css/css.test.ts: Output .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } (pass) css tests > .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } [0.11ms] Output .foo { content: "+"; } (pass) css tests > .foo { content: "\2b"; } [0.02ms] Output div { --foo: 1 1 1 #0101011a, 2 2 2 #02020233; --bar: 1 1 1 #01010116, 2 2 2 #02020233; } (pass) css tests > custom property cases > div { --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2); --bar: 1 1 1 #01010116, 2 2 2 #02020233; } [0.03ms] Output :root { --my-color: red; --my-bg: white; --my-font-size: 16px; } .element { color: var(--my-color); background-color: var(--my-bg); font-size: var(--my-font-size); } (pass) css tests > custom property cases > :root { --my-color: red; --my-bg: white; --my-font-size: 16px; } .element { color: var(--my-color); background-color: var(--my-bg); font-size: ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 67 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/css/css.test.ts bun test v1.4.0 (8326d1b) test/js/bun/css/css.test.ts: Output .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } (pass) css tests > .flexrow { flex-direction: row; } .flexcol { flex-direction: column; } .hello { flex-wrap: wrap; } .world { flex-wrap: nowrap; } [4.46ms] Output .foo { content: "+"; } (pass) css tests > .foo { content: "\2b"; } [1.13ms] Output div { --foo: 1 1 1 #0101011a, 2 2 2 #02020233; --bar: 1 1 1 #01010116, 2 2 2 #02020233; } (pass) css tests > custom property cases > div { --foo: 1 1 1 rgba(1, 1, 1, 0.1), 2 2 2 rgba(2, 2, 2, 0.2); --bar: 1 1 1 #01010116, 2 2 2 #02020233; } [1.08ms] Output :root { --my-color: red; --my-bg: white; --my-font-size: 16px; } .element { color: var(--my-color); background-color: var(--my-bg); font-size: var(--my-font-size); } (pass) css tests > custom property cases > :root { --my-color: red; --my-bg: w ... (truncated) release with fix: 67 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 643ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output) �[1m�[92m Compiling�[0m bun_clap ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/css/values/calc.rs | 56 +++++++++++++++++++-------------------------- test/js/bun/css/css.test.ts | 35 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 33 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/css/values/calc.rs 12 10 0 test/js/bun/css/css.test.ts 3 7 0 ``` </details> <!-- robobun:evidence:end -->
This was referenced Oct 14, 2026
robobun
force-pushed
the
farm/c5eda0e7/css-length-try-op-rhs
branch
from
October 18, 2026 11:43
980a04f to
5aaec10
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
Author
|
Updated 11:17 PM PT - Oct 19th, 2026
✅ @robobun, your commit 23a7c92e579f0ff5c49dafbf90d869696a92c45c passed in 🧪 To try this PR locally: bunx bun-pr 38489That installs a local version of the PR into your bun-38489 --bun |
robobun
changed the title
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.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/css/values/length.rs— pre-existing: users minifying angle math over mixed units still get a folded value a step off, the same defect this PR stops for lengths. The Angle impls at src/css/values/length.rs:781 and src/css/values/length.rs:793 cross-convert through to_degrees(), which rounds (1grad is 0.9f32), so mod(1turn, 1grad) folds to a tiny non-zero degree value instead of 0. Fix: treat mixed-unit angles like mixed-unit lengths for the step-based ops (return None from the Angle TryOp fallback, or compare in an exact unit), with a test for round/rem/mod over deg/grad/turn/rad pairs. Same pattern at 2 sites (length.rs:781, length.rs:793). [also at: src/css/values/length.rs:690 - pre-existing: users still get a wrong folded value from step functions over mixed Time or Angle units, the exact hazard this comment names for lengths.]Why this was flagged
Input
a { rotate: mod(1turn, 1grad) }through the CSS minifier (Calc::apply_op at src/css/values/calc.rs:875 calls the Angle protocol::TryOp impl). The units differ, so src/css/values/length.rs:781 evaluates f(360.0, 1grad.to_degrees()) where to_degrees() at src/css/values/angle.rs:141 yields 0.9f32 = 0.8999999762. The mod closure at src/css/values/calc.rs:419 computes ((360 % 0.9f) + 0.9f) % 0.9f, which is about 9.5e-6 rather than 0, so the output is a near-zero non-zero degree value where the spec result is 0. The base branch behaves the same (this code is not changed), but the PR's own comment at src/css/values/length.rs:688-689 names exactly this rounding-across-a-step hazard as the reason it stopped folding mixed length units, and the Angle sibling ten lines below keeps the hazard. No safeguard applies: Angle's fallback arm always returns Some.Verification: pre-existing — the Angle impls are untouched by the diff, so the base folds the same wrong value by the same route. Any angle-typed value using round()/rem()/mod() with operands in different angle units triggers it.
src/css/values/length.rs:776-781falls to_ => Angle::Deg(f(ctx, self.to_degrees(), rhs.to_degrees()))for mixed units, somod(1turn, 1grad)yieldsDeg(~9.5e-6)where the correct answer is 0.
LengthValue's TryOp and TryOpTo impls converted self to px twice when the two operands had different units. round(), rem(), mod(), hypot() and atan2() over 1in and 10px were evaluated as f(96px, 96px), and a second operand with no px value (1em) did not stop the fold. atan2() now reads both operands in px. It has no step. round(), rem(), mod() and hypot() over two different units are not folded. The px value of a unit is rounded (1pt is 1.3333334px), and round(), rem() and mod() jump where the quotient of the operands crosses a whole number, so a fold on px values can be a whole step away from the value a browser computes. The function stays as written.
robobun
force-pushed
the
farm/c5eda0e7/css-length-try-op-rhs
branch
from
October 18, 2026 12:03
5aaec10 to
19ca2e9
Compare
Contributor
There was a problem hiding this comment.
Code review found no new issues
No new issues were found in this update; 2 findings from earlier reviews are still open above.
Still open from earlier reviews (2):
- 🔴
src/css/values/length.rs:689—Users whose stylesheet has a mixed-unit round()/rem()/mod() over an inner calc() now get a value a whole step away from… - Also unresolved: 1 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
This was referenced Oct 20, 2026
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.
@Jarred-Sumner, a question after your review of #33047: Chrome 153 computes
round(down, 70mm, 1cm)as 264.567px, Firefox 140 as 226.772px. This PR keeps such a function as written. Is that the bar for math folds? Notes has two built alternatives.Problem
LengthValue::try_opreads its left operand twice when two lengths differ in unit (src/css/values/length.rs:695):round(1in, 10px)andround(1in, 1em)fold to96px,atan2(1in, 10px)to45deg.to_pxrounds (1pt is 1.3333334px), somod(4px, 1pt)becomes1.33333px.Fix
round(),rem(),mod()andhypot()over two different units stay as written.atan2()reads both operands in px. It has no step.test/js/bun/css/css.test.ts(30 rows fail on main),color.test.ts. Also rantest/js/bun/css,test/bundler/css. Self-reviewed: 43 concerns raised, 37 addressed.Background
Calc::apply_oppasses a math function over constants to the value type'stry_op.Nonekeeps the function as written.round(),rem()andmod()jump where the operand quotient crosses a whole number. A rounded operand moves the result a whole step.Downsides
round(1in, 10px).atan2(1in, 1em)is now a build error in@media,@containerand@propertypreludes. Main builds a wrong constant.calc()operand with 6 digits: 106,071 of 14,515,200 swept rows then read a step off.Notes
Reproduce
width: 96px; height: 96px; margin: 0 135.764px; rotate: 45degwidth: round(1in, 10px); height: round(1in, 1em); margin: mod(4px, 1pt) hypot(1in, 10px); rotate: 1.467radNobody reported this. The line dates from the first CSS parser commit (#14122) and is in every release since v1.1.30. Main gets 12 known-answer inputs from outside wrong: 3 from WPT
css/css-values/round-mod-rem-computed.html, 7 from the lightningcss tests, 2 from MDN (mod(1000px, 29rem)prints0). All 12 are rows in the new test block.The question, with numbers
The sweep is all 42 ordered pairs of different absolute units, whole values 1..200 against 1..12,
round()with its four strategies,rem()andmod(): 604,800 rows. Values come fromgetComputedStyleofmargin-left. Firefox is Gecko 140 in a third-party build (Mullvad Browser), WebKit is the Playwright build 27.2.round(1in, 10px)mod(1in, 25px)mod(4px, 1pt)round(down, 70mm, 1cm)round(down, 20px, 5pt)round(10px, 5pt)round(up, 4px, 1pt)round(down, 28.8px, 0.1in)hypot(1in, 10px)hypot(1in, 3px, 4px)The three shapes:
26f5343930): equal to lightningcss. 5,299 rows are a whole step from the exact value, 5,252 of them exact multiples that main prints right. It needs css: fold hypot() with three or more arguments pairwise #38501 with it, orhypot(1in, 3px, 4px)prints11px.robobun/2be5a55a/css-mixed-length-exact-fold(127 new lines inlength.rs, with thecalc.rschange of css: fold hypot() with three or more arguments pairwise #38501). The operands become whole numbers of 1/381 px, recovered from the literal's decimal. 0 of the 604,800 rows and 0 of 7,484,400 one-decimal rows are a step off. It differs from Chrome for the 9,703 rows above, as main does today.The same question exists for one unit:
mod(5cm, 1cm)folds to0on main, and Chrome computes 37.7953px.What else changes
Bun.color("hsl(atan2(round(1in, 10px), 1px) 50% 50%)", "css")isnull. Main returns#81bf40from the wrong fold.@media (min-width: calc(100px * tan(atan2(1in, 1em)))), the same in@container, and@property ... initial-value: atan2(1in, 1em)fail with "Invalid media query" or "Unexpected token". The at-rule preludes have no unparsed form to fall back to (parse_paren_block,src/css/media_query.rs). Three rows pin this.@media (min-width: round(1in, 10px))builds and keeps the function.hypot()has no step, buttry_opcannot see which function it runs, sohypot(1in, 10px)also stays as written.calc()already folded is printed with 6 digits:round(up, calc(3cm + 40mm), 1cm)becomesround(up,264.567px,1cm). For 106,071 of 14,515,200 swept rows of the formop(calc(n u1 + m u2), j u3)an exact reader of that output gets a step other than the source gives. Main prints the exact value for 98,483 of those rows and a wrong value for 9,538,072 of the 14,515,200.Self-review
43 concerns were raised against the exact fold (shape 3), which this branch carried first. This smaller shape and this text address 37. Not done here:
hypot()still folds (2 concerns). It needs a change to theTryOptrait or toCalc::apply_op.Calc::parse_math_fn.<general-enclosed>fallback for at-rule preludes (1 concern).Measurements (
bun_cssobject of main and of this PR, same rustc flags, before LTO).text-245 B,.rodata-536 B (size -A). 3,918 of 3,927 functions are byte-identical. The 9 others areapply_op,parse_math_fn,parse_hypotandparse_atan2_argsfor lengths.rem(),mod()orhypot()fold of two lengths in one unit runs 44 instructions throughCalc::apply_op, 43 on main (counted in the disassembly). The difference is one register move. No check is added.mi_malloccalls per declaration, 5 when it folds).Other pull requests
hypot()) has this branch as its base. It is not needed for this shape. Its 7 rows with two absolute units expect a px fold and wait for the answer to the question.text-shadow) is blocked on this PR and on css: fold rem() with a truncated quotient, so it keeps the sign of the dividend #32286.rem()sign) and css: fold round() by choosing between the lower and upper multiple of |B| #45249 (round()ties) change the closures this arm no longer runs for mixed units.atan2(0px, -1in)row of this PR printed3.14159radon macOS arm64 (build 126706). That row is gone. Theatan2()rows here print angles that one ulp ofatan2fdoes not change.Suites (debug build)
test/js/bun/css/css.test.ts: 1,349 pass. On main 30 of the 35 new rows fail.test/js/bun/css/color.test.ts: 1,023 pass. On main the new test fails.doesnt_crash.test.ts61 pass,test/bundler/css183 pass,test/bundler/esbuild/css.test.ts56 pass.