Skip to content

css: parse math functions in text-shadow lengths - #45546

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/a27a7bfc/text-shadow-math
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/a27a7bfc/text-shadow-math

Conversation

@robobun

@robobun robobun commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Blocked on #38489 and #32286. Without them round(16px, 1em) folds to 16px and rem(-7px, 3px) to 2px.

Problem

  • text-shadow reads its lengths with LengthValue::parse (src/css/properties/text.rs:6), which has no math arm: a value with calc() stays a token list.
  • No fold or target check runs: text-shadow: 15px 0 red; text-shadow: round(1em, 5px) 0 red builds to the round() line alone, for targets without round().

Fix

  • The four lengths are Length values (a literal or a calc() tree), read with Length::parse_with_range (new, the lightningcss name).
  • Blur and spread are non-negative: math below zero becomes 0, and a negative literal leaves the declaration as written.
  • Verified: test/js/bun/css/css.test.ts, block text-shadow (25 of 40 rows fail on main). Self-reviewed: 28 concerns raised, 27 addressed.

Background

Downsides

Notes

Reproduce

printf '.d { text-shadow: 15px 0 red; text-shadow: round(1em, 5px) 0 red }\n.e { box-shadow: 15px 0 red; box-shadow: round(1em, 5px) 0 red }\n.f { text-shadow: calc(1px + 2px) 0 0 red }\n' > in.css
bun build ./in.css --outdir out && cat out/in.css
  • main: .d has the round() line only, .e has both lines, .f prints calc(1px + 2px) 0 0 red.
  • this PR: .d has both lines, .f prints 3px 0 red.

What Length::parse_with_range does

slot range negative literal math that resolves below zero (or to NaN)
x, y All typed typed
blur, spread NonNegative error, the declaration prints as written 0
  • In every slot, math that resolves to a number (calc(1 + 2), sign(-2px)) is an error, so the declaration prints as written. So is the bare sum or product that a one-argument min(), max() or hypot() leaves behind, because it prints without calc() (min(1em + 2px) as 1em + 2px, css: wrap a one-argument min()/max() sum or product in calc() #38654). Other math is typed and printed as math.
  • It tries the literal before the math function. Length::parse tries math first. The two start with different tokens, so the result is the same.
  • Length::parse and its callers do not change. box-shadow still prints 0 0 calc(1px - 6px) red as 0 0 -5px red. css: treat out-of-range and unit-less numeric values as invalid instead of merging or repairing them #42104 owns the range check for the other properties, and the two PRs touch the same lines of text.rs.
  • The spread is read only when the blur was read. A length that is not a blur is not a spread.

References

  • css-text-decor-4, text-shadow: "Negative spread values are invalid." css-values-4: a math function is range-checked after it is resolved, and NaN does not escape a top-level calculation ("it's censored into a zero value", https://drafts.csswg.org/css-values-4/#calc-ieee).
  • Chrome 153, computed text-shadow: calc(NaN * 1px) 0 red and rem(1px, 0px) 0 red are 0px 0px 0px. 0 0 calc(1px - 6px) red has a blur of 0px. calc(infinity * 1px) 0 red is 3.40282e+38px. 0 0 -5px red is dropped, and the declaration before it applies.
  • lightningcss main (ab14f90, with Add NonNegative wrapper for numeric values parcel-bundler/lightningcss#1329): TextShadow { x_offset: Length, y_offset: Length, blur: NonNegative<Length>, spread: NonNegative<Length> }. 1.33.0 does not have the range yet and prints 0 0 -5px red.
  • 36 rows, minified, against lightningcss main: 23 are equal. 5 differ in white space only (both keep the declaration as written). 3 differ on purpose: for a one-argument min(), max() or hypot() of a sum, lightningcss prints 1em + 2px 0 red, which is not valid CSS, and this PR keeps the declaration as written. The other 5 are the rows under Order.

Order

Measurements (release builds of main f2405c382d and of this PR)

  • Type sizes (cargo rustc -p bun_css --lib -- -Zprint-type-sizes): TextShadow 56 to 88 bytes, SmallList<TextShadow, 1> 64 to 96 bytes, Property 216 to 216 bytes.
  • Instructions in one parse_value::<SmallList<TextShadow, 1>> call (gdb single-step, second call of each value): 1px 1px 2px red 2,863 to 3,246. 1px 1px red 2,538 to 2,664. none 1,287 to 1,480. 1px 1px 2px 3px red 3,236 to 3,401. perf and valgrind are not on the machine.
  • Token reads per shadow, with a blur and without: main 8 and 8, this PR 9 and 8. box-shadow, which tries math first, reads 15 and 15. Failed math probes per shadow: 1 here, 4 in box-shadow.
  • The 15 stylesheets in test/js/bun/css/files (17 text-shadow declarations): LengthValue::parse 20,137 to 20,134 calls, Length::parse 3,371 to 3,371, Calc::<Length>::parse_with 3,437 to 3,454. The 17 extra calls are one failed probe per text-shadow declaration. A stylesheet with no text-shadow runs the same code as before.
  • Allocations in one parse: one block per declaration, 56 bytes before and 88 bytes now. round(1em, 5px) 0 red takes 6 blocks, the same as in box-shadow.
  • Release binary (size): .text 106,016,187 to 106,018,235 bytes. The TextShadow symbols and Length::parse_with_range go from 5,625 to 7,846 bytes (nm -S).
  • Output of real CSS: 3,375 stylesheets from 256 npm packages and the 15 in-tree ones, printed with the default targets and minified with no targets. 46 do not parse on any build. No output changes. The 757 text-shadow declarations in them have no math in a length and no negative blur or spread literal.
  • Nested invalid math in the blur slot (round(round(... foo ..., 5px), 5px)): 17, 33 and 65 calls of the calc parser at depth 16, 32 and 64.

Typed, but not valid in a browser (as on main for a value without math)

Suites (debug build)

  • test/js/bun/css/css.test.ts: 1,354 pass, 0 fail. On main 25 fail, all in this block.
  • The other files in test/js/bun/css except css-fuzz.test.ts and nested-vendor-prefix-duplication.test.ts: 1,183 pass, 0 fail.
  • test/bundler/css, test/bundler/esbuild/css.test.ts and the CSS regression tests 20546, 21907, 22317, 25785, 25794, 27117, 27458: 251 pass, 0 fail.

Self-review

  • Done on an earlier shape of this branch. It asked for these changes, and they are in: the two fold fixes are not part of this PR, the range parse is on Length with the upstream names, a negative spread from math is 0 as the spec says, a negative literal leaves the declaration as written, and the one-argument min() case is refused.
  • Rejected: wrap that bare sum in calc() inside the reader. It would be a second fix for css: wrap a one-argument min()/max() sum or product in calc() #38654 in another place. The reader refuses the value, so the output stays as on main.

Not in this PR

  • src/css/properties/transform.rs:7 has the same import. There the handler keeps the earlier declaration only when the later one is unparsed, so the same change loses the fallback for translateZ(), translate3d() and perspective(). It needs a target check in TransformHandler first.
  • Math inside custom properties and inside properties Bun does not parse. docs: correct the CSS math functions example and say where math is evaluated #45114 documents that limit.

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

fails on main (without fix)
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/css/css.test.ts
error: Source list 'bindgenV2' matched nothing
  at: file:///workspace/bun/scripts/glob-sources.ts
  hint: Patterns: src/**/*.bindv2.ts
error: script "bd" exited with code 1
__F:-1:S:0

release without fix: 49 failed, 72 skipped
bun test v1.4.3-canary.1 (367d939d9)

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.78ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [0.39ms]
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.22ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

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

(pass) css tests > custom property cases > :root {
        --my-color: red;
        --my-bg: white;
        --my-font-size: 16px;
      }
      .element {
        color: var(--my-color);
        background-color: var(--my-bg);
        font-size: 
... (truncated)
passes on PR (with fix)
ASAN with fix: 72 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/css/css.test.ts
bun test v1.4.4 (367d939d9)

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.42ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.07ms]
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.90ms]
Output :root {
  --my-color: red;
  --my-bg: white;
  --my-font-size: 16px;
}

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

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

release with fix: 72 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1603ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/241] gen ErrorCode+*.h
[2/241] gen bindgenv2
[3/241] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/239] gen NodeModuleModule.lut.h
Generating /workspace/bun/build/release/codegen/NodeModuleModule.lut.h from /workspace/bun/src/jsc/modules/NodeModuleModule.cpp
[5/239] gen .bind.ts → GeneratedBindings.cpp
[6/239] gen generated_host_exports.rs
generated_host_exports.rs: 120 exports (host=5, lazy=10, generic=105, rust=0); 249 extern-C blocks audited
[7/239] gen BunObject.lut.h
Generating /workspace/bun/build/release/codegen/BunObject.lut.h from /workspace/bun/src/jsc/bindings/BunObject.cpp
[8/239] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (15 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 field
... (truncated)
diff hotspot
src/css/properties/text.rs  |  27 ++++++--
 src/css/values/length.rs    |  30 ++++++++-
 src/css/values/number.rs    |   8 +++
 test/js/bun/css/css.test.ts | 155 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 213 insertions(+), 7 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                         reads  edits  tests
src/css/properties/text.rs       4      4     38
src/css/values/length.rs         2      3     38
src/css/values/number.rs         1      2     38
test/js/bun/css/css.test.ts      3      3     38

@robobun

robobun commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:18 AM PT - Oct 17th, 2026

⏳ @robobun, your commit 6c730ba is still building in Build #127611, but has 3 failures so far (All Failures):

@robobun

robobun commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: closed, not merged. #47025 consolidated the open CSS parser pull requests and did not take this change (see the maintainer's comment below).

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 18f59464-05bf-4458-a09a-5d0c5485aca3
📥 Commits

Reviewing files that changed from the base of the PR and between 4873399 and b2bae55.

📒 Files selected for processing: 1
  • test/js/bun/css/css.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

Length parsing now supports numeric range constraints and converts both operands during cross-unit operations. Text-shadow parsing applies nonnegative constraints to blur and spread, with added minification and browser-target tests.

Changes

CSS length math and text-shadow parsing

Layer / File(s) Summary
Length range parsing and math evaluation
src/css/values/number.rs, src/css/values/length.rs
Added NumericRange and Length::parse_with_range. Negative or NaN scalar calculation results become zero for nonnegative lengths. Cross-unit operations now convert both operands to pixels.
Text-shadow parsing and output
src/css/properties/text.rs, test/js/bun/css/css.test.ts
Offsets remain unrestricted. Blur and spread accept nonnegative lengths, and spread is parsed only after a valid blur. Tests cover minification, invalid values, fallbacks, and browser-target prefixing.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to b2bae

No merge-blocking issue is established by the supplied evidence. Normal validation remains appropriate before merging.

Pre-merge checks: Passed 4
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description clearly explains the problem, fix, scope, tradeoffs, and verification results. It does not use the template headings exactly, but it contains the required information in equivalent sec…
Title check Passed The title is concise and accurately describes the primary change: parsing CSS math functions in text-shadow lengths.
Full details: Description check

Explanation

The description clearly explains the problem, fix, scope, tradeoffs, and verification results. It does not use the template headings exactly, but it contains the required information in equivalent sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Findings marked 🟡 are optional suggestions and need no follow-up push.

Comment thread src/css/values/calc.rs Resolved
Comment thread src/css/values/length.rs Resolved
Comment thread src/css/properties/text.rs Resolved
Comment thread src/css/values/length.rs Resolved
Comment thread src/css/properties/text.rs Resolved
Comment thread src/css/values/length.rs Resolved
Comment thread src/css/properties/text.rs Resolved
@robobun
robobun force-pushed the robobun/a27a7bfc/text-shadow-math branch from 4873399 to b2bae55 Compare October 16, 2026 15:08

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still open from earlier reviews (4):

  • Unresolved: 4 minor or pre-existing.

Comment thread src/css/values/length.rs Resolved
text-shadow read its four lengths with LengthValue::parse, which takes a
dimension or a number token only. A value with calc(), round() or another
math function stayed a list of tokens: nothing was folded and no target
check ran.

The lengths are now Length values, read with Length::parse_with_range.
That is the range-aware parse lightningcss has: the offsets take any
length, the blur radius and the spread take a non-negative one. A negative
literal there is an error, and math that resolves below zero is 0. Math
that resolves to a number is not a length. A one-argument min(), max() or
hypot() of a sum is refused too, because it would print without calc().
In those cases the declaration prints as written.
@robobun
robobun force-pushed the robobun/a27a7bfc/text-shadow-math branch from b2bae55 to 6c730ba Compare October 17, 2026 15:41

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no new issues

No new issues were found in this update; 1 finding from earlier reviews is still open above.

Still open from earlier reviews (1):

  • 🔴 src/css/values/length.rs:438 — Users writing text-shadow math with mixed units get a wrong folded value after merging; base printed the declaration as…

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.

@robobun

robobun commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

#38489 changed. This PR does not carry its commit, so there is nothing to replace here, but the rows under Order are affected:

@robobun

robobun commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

#46145 fixes the bare sum of a one-argument min() or max() in calc.rs. It replaces #38654, #38639 and #38501.

After it lands, the value arrives here as Calc::Function(MathFunction::Calc(..)):

  • Length::parse_with_range can drop its refusal of Calc::Sum and Calc::Product.
  • The rows min(1em + 2px) 0 red and max(2 * min(1em, 2px)) 0 red print calc(1em + 2px) 0 red and calc(2*min(1em,2px)) 0 red.
  • The row 0 0 hypot(1em + 2px) red keeps hypot(1em + 2px), now as a typed value.

#46145 does not change length.rs. Lengths in two units stay with #38489.

@robobun

robobun commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

#32286 changed. Its fold is a - b * (a / b).trunc() now. It was a % b.

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.
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

Not taken in #47025 (7942008), which consolidated the open CSS parser pull requests. It is right or harmless, but not worth its code, the bigger output or the behavior it changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants