Repository navigation
Conversation
…t matches A typed `initial-value` that no component of its `syntax` matched stopped the build with "Unexpected token". The typed parsers resolve a value while they parse it, so they also refuse valid values: `rgb(from currentColor r g b)`, `oklch(0 0 0 / 0.97)`, `calc(1 + 2)` for `<integer>`. `SyntaxString::parse_value` now keeps such a value as a token list, and the value is printed as written, as a declaration is. The browser validates it. An empty value for a typed syntax is still an error. The initial value is printed without the browser targets. `light-dark()` was lowered to `var()`, which is not valid in an initial value.
|
Updated 9:17 PM PT - Oct 17th, 2026
✅ @robobun, your commit 66797440c3bcebfb2d62d2dcc00d754c0516bf9e passed in 🧪 To try this PR locally: bunx bun-pr 45807That installs a local version of the PR into your bun-45807 --bun |
|
Status: open, stacked on #45806, waiting for CI and review.
|
There was a problem hiding this comment.
Beyond the inline findings, I also checked that the new fallback in SyntaxString::parse_value starts from the value's first token (is_exhausted restores the parser state before TokenList::parse runs) and that the * syntax path is byte-for-byte the same as before, including an empty initial-value under *. The uppercase named-color case (RED kept verbatim under <color>) was also examined and is consistent with the as-written intent rather than a bug.
Extended reasoning...
The change touches the CSS parser's @ property handling in src/css/values/syntax.rs (typed initial-value falls back to a raw TokenList instead of erroring) and src/css/rules/property.rs (initial-value printed with browser targets cleared), plus test updates in test/js/bun/css/css.test.ts. It touches no security-sensitive surface. The Rust diff is small, but it deliberately removes a build-time error for invalid typed values, diverges from lightningcss behavior, and the author explicitly asks a maintainer whether that is acceptable, so this is a design call a human should make rather than an approval.
Findings marked 🟡 are optional suggestions and need no follow-up push.
| // An empty value is a missing one, which is an error for a typed syntax. | ||
| if input.is_exhausted() { | ||
| return Err(input.new_error_for_next_token()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Ok(ParsedComponent::TokenList(TokenList::parse( | ||
| input, | ||
| &ParserOptions::default(None), | ||
| 0, | ||
| )?)) |
There was a problem hiding this comment.
🔴 Authors who mistype a typed initial-value (for example <length> with red) now get a silent successful build whose @ property rule every browser drops, where the base branch failed the build with "Unexpected token". The fallback at src/css/values/syntax.rs:223 keeps any typed value as a TokenList with no diagnostic once no component matched; only an empty value still errors at syntax.rs:217. Fix: keep a diagnostic for the subset the typed parsers fully cover, for example warn through the parser log when the kept TokenList has no Function, Var, Env or UnresolvedColor token, while still passing through function-bearing values like rgb(from currentColor r g b).
Why this was flagged
A style sheet contains @ property --p { syntax: "<length>"; inherits: false; initial-value: red; } and goes through bun build or the CSS transpiler, reaching SyntaxString::parse_value at src/css/values/syntax.rs:102. Every component fails, input.reset(&state) runs at syntax.rs:213, the value is not exhausted so the check at syntax.rs:217 does not fire, and syntax.rs:223 returns ParsedComponent::TokenList holding red. PropertyRule::to_css at src/css/rules/property.rs:51 prints initial-value: red and the build exits 0 with no warning. On the base branch the same input returned Err(input.new_error_for_next_token()) and the build failed with Unexpected token: red, which test/js/bun/css/css.test.ts previously asserted for <color> with 25px, <length> with var(--some-value), <string> with hi, and red; initial-value: foo. Chrome, Firefox and Safari drop an @ property rule whose initial-value does not match its syntax, so the registered property silently disappears at runtime instead of at build time.
Verification: Every component attempt fails and input.reset(&state) runs (src/css/values/syntax.rs:213); the only remaining guard is if input.is_exhausted() { return Err(...) } (lines 217-219), which does not fire for a non-empty value; control falls through to Ok(ParsedComponent::TokenList(TokenList::parse(input, ...)?)) (lines 223-227). The base branch instead returned Err(input.new_error_for_next_token()).
There was a problem hiding this comment.
This is the question that the PR body asks a maintainer, so this push adds no diagnostic. I measured the subset you propose: warn when the kept value holds no function, var() or env().
- The 57 values of the PR body: 28 are kept as written. The rule warns on 11 of the 14 that are not valid. It does not warn on
var(--x),anchor-size(width)androtate(1turn) foo(1). It warns on 1 of the 14 valid values:RED, whichCssColor::parserefuses until css: match named colors without regard to ASCII case #45092. - The real sample: 20 rule bodies are kept as written. The rule warns on 5 of the 16 that are not valid, each a list of font names under
<string>#in one file. It does not warn on the 40rgba(var(--a), var(--b))rules of@universal-material/core. - A warning of the CSS parser has no file name and no line in the output of
bun buildtoday. A nested@propertyprintswarn: invalid @ rule encountered: '@property'.
So the rule catches a plain type mismatch such as red under <length>, with one known false warning. If a maintainer wants it, it is a check of the token list after the fallback in SyntaxString::parse_value. PropertyRule::parse must then take the ParserOptions, which it does not take today.
…ter a function The rule after an `@property` rule is printed in the same sheet, so the test fails if `PropertyRule::to_css` does not put the targets back. The `<transform-list>` value that is kept as written has its blank again. The minified form drops it and the pretty form keeps it.
There was a problem hiding this comment.
The new commit covers both earlier nits (targets restored for the rule after @ property, and the rotate(1turn) foo(1) blank pinned in both pretty and minified form). Beyond the inline finding, I also checked that is_exhausted() at src/css/values/syntax.rs:217 resets the parser state (via expect_exhausted) so the TokenList::parse fallback sees the full value, and that dest.targets is restored before the ? in src/css/rules/property.rs:52 so an error while printing the initial value does not leave later rules without targets. The silent-build question for invalid typed values is still a maintainer call.
Extended reasoning...
The PR changes two CSS parser/printer paths (typed @ property initial-value fallback to a raw TokenList, and printing the initial value without browser targets) plus tests; no security-sensitive surface. The latest commit only adds tests addressing the two earlier nits. One nit was posted this run, and the behavioral trade-off the author explicitly raised (invalid typed values now build without a diagnostic, diverging from lightningcss) needs a maintainer decision, so not approving.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Still open from earlier reviews (1):
- 🔴
src/css/values/syntax.rs:227—Authors who mistype a typed initial-value (for example <length> with red) now get a silent successful build whose @ pro…
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.
…not reduced A typed `initial-value` that is not reduced is an `Unexpected token` error, because `SyntaxString::parse_value` has no unparsed form. The rows asserted that error. #45806 keeps such a value as written, so the rows failed on a merge with it. They are now todo rows that assert the value as written, next to the two rows for valid values. Without #45806 the todo test fails with the error. With #45806 it passes. The file passes in both cases. The comment pointed to #45807. That change is now part of #45806.
…not reduced A typed `initial-value` that is not reduced is an `Unexpected token` error, because `SyntaxString::parse_value` has no unparsed form. The rows asserted that error. #45806 keeps such a value as written, so the rows failed on a merge with it. They are now todo rows that assert the value as written, next to the two rows for valid values. Without #45806 the todo test fails with the error. With #45806 it passes. The file passes in both cases. The comment pointed to #45807. That change is now part of #45806.
Problem
bun buildfails witherror: Unexpected token: rgb(on a valid@propertyrule:syntax: "<color>"; initial-value: rgb(from currentColor r g b).SyntaxString::parse_value(src/css/values/syntax.rs:220) makes each refusal a build error.light-dark()in an initial-value is printed asvar(), which Chrome 153 drops.Fix
test/js/bun/css/css.test.ts(10 tests, 9 fail on css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806), all oftest/js/bun/css. Self-reviewed: 24 concerns raised, 22 addressed (Notes).Background
@propertyregisters a custom property with asyntax(its type) and aninitial-value. Chrome 153 drops a rule whose value does not match.bun buildlowers syntax for. They turnlight-dark()intovar().Downsides
foofor<color>) now builds as written. It was a build error, which lightningcss tests. A browser drops that rule. Acceptable?<integer>with1.0prints1, which Chrome registers. Main printsz-index: 1.0the same way..text+256 B.Notes
Behaviour changes
initial-valuethat no syntax component matches was a build error. It is now printed as written. That covers valid values that the typed parsers do not resolve, and values that are not valid.light-dark()in an initial-value was printed asvar(--buncss-light, ...) var(--buncss-dark, ...)for the default targets ofbun build. It is now printed aslight-dark(). For the same targets2dppxnow prints as2x. Each browser that reads@property(Chrome 85, Firefox 128, Safari 16.4) readsx,#rrggbbaa, space-separated colors and double-position color stops.src/css/compat.rshas no Safari version for thexunit. caniuse has 16.0.initial-value: red; initial-value: foowas a build error. It now printsfoo. The last descriptor wins in Chrome 153 too.initial-valueare build errors.Question for a maintainer
A typed initial-value that is not valid has no diagnostic any more. Is that acceptable?
<color>with25pxand<length>withvar(--some-value)are twoerror_testcases of itstest_property. This PR asserts the opposite for both. Its maintainer wrote on @property initial-value parsing error parcel-bundler/lightningcss#921, about a valid value that does not resolve: "We should preserve the original value as is in that case."RED,oklch(0.5 0 0),calc(1 + 2)for<integer>). Of the 14 typed values of the sample that main refuses, Chrome 153 registers 4.rgba(var(--a), var(--b))in@universal-material/core, whose 8 style sheets main cannot build. Main builds the other 39. 38 of those are a font-relative length under<length>.SyntaxString::parse_value.var()orenv(). Of the 28 values of the measured set that are kept as written, it warns on 11 of the 14 that are not valid and on 1 of the 14 valid ones (RED, until css: match named colors without regard to ASCII case #45092). In the sample it warns on 5 of the 16 bodies that are not valid, and on none of the 40rgba(var(--a), var(--b))rules. A warning of the CSS parser has no file name and no line in the output ofbun buildtoday.initial-value) reject rules that the draft registers. No engine does that yet. Both are as on main.Measured against Chrome 153 (headless, the source rule and the built rule, pretty and minified, 57 values)
rgb(from currentColor r g b),oklch(0.5 0 0),RED,contrast-color(red),<integer>calc(1 + 2),<resolution>calc(1dppx * 2),<image>paint(foo), ...): each built rule registers the same value as its source rule.fooandvar(--x)for<color>,red blue, a bare ident for<string>, ...): each built rule is dropped, as its source rule is.light-dark(): each built rule is registered. Chrome drops the built rule of css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806 and of main.<integer>with1.0prints1. Chrome drops the source rule and registers the built rule.1.0value, and 7 that differ only in spelling (blueas#00f, a bundledurl()).Values that are not valid and become valid (Chrome 153, a second set of 29 values, release builds of main, of #45806 and of this PR)
<integer>with1.0,+1.0,1e0,1e2or-0.0prints an integer. Also in a list:1 2.0,1, 2.0.1+2and1/**/2print12(<integer>,<number>),calc(1+2)printscalc(12),5/**/0%prints50%,a/**/bprintsab(<custom-ident>).#f00,#808080andrgb(255 0 0)printredorgray, which<custom-ident>and a keyword syntax accept.z-index: 1.0printsz-index:1,z-index: calc(1+2)printsz-index:calc(12),--x: a/**/bprints--x:ab.syntax: "<length>"; initial-value: 5builds as5px, which Chrome registers.src/css/properties/custom.rs). It changes each declaration and each custom property (--x: 1.0then prints1.0), and the color case stays. The second keeps the build error for a value that holds number tokens only. It covers 11 of the 20.Real style sheets (release builds of #45806 and of this PR)
@propertybodies (42,737 rules) in 1,285 files, from 444 npm packages and 17 GitHub files.light-dark(). 7 fail both ways.rgba(var(--a), var(--b))(40 rules,@universal-material/core),oklch(0 0 0 / 0.97)(@ctrl-ui/react), 3color-mix(in srgb, currentcolor 40%, transparent), and 6<string>values that are not strings (5 are lists of font names with bare idents).<ratio>,<line-style>), 3 typed rules with noinitial-value.--minify: 1,134 give the same bytes. 11 fail in css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806 and build here. 2 differ, inlight-dark()only. 0 build in css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806 and fail here. 138 fail both ways for a cause outside this change.test/js/bun/css/files/*.cssand four files of tailwindcss 4.3.3 (none has@property): 0 of 38 builds differ.Cost (release builds of #45806 and of this PR, gdb on
bun-profile, rule 2 of 4 equal rules)syntax:'<color>';inherits:false;initial-value:rgb(255 0 0)initial-valuefirstsyntax:'*';inherits:false;initial-value:rgb(from currentColor r g b)syntax:'*';inherits:falsesyntax:'<color>';inherits:false;initial-value:rgb(from currentColor r g b)(kept as written)NestedRuleParser::parse_block. Print is one call ofPropertyRule::to_css.Tokenizer::next_implcalls, 5Parser::expect_exhausted, 7mi_malloc, 2mi_realloc. The same value undersyntax:'*': 37, 4, 5 and 2.Tokenizer::next_impl,Parser::expect_exhaustedandmi_malloccounts of the other rules are the same as in css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806.@property(bulma.css):SyntaxString::parse_valuecalls 0 -> 0, output bytes identical.llvm-size -A, stripped):.text81,368,293 -> 81,368,549 B (+256),.rodata22,598,604 -> 22,598,604 B (+0).openat,open,readlink,read,pread64,write,close,getdents64, counted with gdbcatch syscall, because strace is not installed here): 77 -> 77 for the first rule of the table, 121 -> 121 forbulma.css.Weighed and set aside
url()still fails and the value is read three times.light-dark()tovar()again.Not in this PR
rotate(1turn)foo(1).@propertyinside@media,@layer,@supportsor@scopeis kept as an unknown at-rule. It still printslight-dark()asvar().calc(50% + 1)for<percentage>builds as0%) are the subject of css: fold relative colors on the scale of their function, and keep them as written when the fold is not exact #45362.Other PRs
toThrowassertions, and adds a counter toParserInputso that its own unresolved relative colors stay as written in a typed@property. After this PR that counter is not needed, and the 4 assertions change. The second of the two to merge needs a rebase.light-dark()as written for the default targets. After it, the print without targets matters for explicit targets and fordppx.oklch()) make the typed parser accept more values. Those values then print typed. No expected string of this PR holds such a value.Self-review
24 concerns raised, 22 addressed. The review asked for the split from #45806. Not done: a tracking issue for the descriptor arms and one for the media features. #45806 names both.
Suites run (
bun bd test, debug build with ASAN)test/js/bun/css/css.test.ts: 1,324 pass, 0 fail.test/bundler/esbuild/css.test.ts: 58 pass, 0 fail.test/js/bun/cssandtest/bundler/css, with the two files above andtest/regression/issue/27117.test.ts, one file at a time: 2,763 pass, 4 fail. The 4 are incss-fuzz.test.ts, a timing test that does not run in CI: 1,000Bun.buildcalls do not fit its 10 s limit on a debug build.@propertytests fail on a debug build of css: read an @property initial-value on the stylesheet's parser, keep an unmatched value as written #45806. All 10 fail on a release build of main.