Skip to content

css: read an @property initial-value on the stylesheet's parser, and add the <string> syntax - #45806

Open
robobun wants to merge 2 commits into
mainfrom
robobun/b8157b98/css-property-initial-value
Open

robobun wants to merge 2 commits into
mainfrom
robobun/b8157b98/css-property-initial-value

Conversation

@robobun

@robobun robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • bun build fails with error: Unexpected token: url( on a valid @property rule whose initial-value has a url(): syntax: "<url>"; initial-value: url("data:,"). @sap-theming/theming-base-content has 7,192 such rules.
  • PropertyRule::parse (src/css/rules/property.rs:117) parsed the value again with a second parser. That parser had no import records, and its error positions were relative to the value.
  • A typed match of only the start was accepted: 1px 2px for <length> | <length>+ built as 1px. syntax: "<string>" was Invalid value.

Fix

  • The stylesheet parser reads the value: in place when syntax is known, else after the body from a saved position.
  • A typed value must match all of the initial-value.
  • <string> is a syntax name, as in lightningcss.
  • Verified: test/js/bun/css/css.test.ts (7 tests), test/bundler/esbuild/css.test.ts (2). Also all of test/js/bun/css, test/bundler/css. Self-reviewed: 24 concerns raised, 22 addressed (Notes).

Background

  • @property registers a custom property: a syntax (its type) and an initial-value.
  • An import record is the bundler's entry for one url(). Without the list, url() fails.
  • Weighed: lend the import records to the second parser. Positions stay relative to the value, and each value is still read twice.

Downsides

  • <color> with red blue built as red. It is now the build error that each value with no matching component has. A browser drops that rule. 0 of 1,161 real rule bodies change that way.
  • Instructions per rule: 8,404 -> 7,467 (syntax first), 8,940 (initial-value first, 2.3% of real rules). No value: 2,445 -> 2,484. .text +256 B.
Notes

Behaviour changes

  • url() in an initial-value was a build error for each syntax, * too. It is now an import record, as a url() in a declaration: 1 record and 1 asset per url(). A value that a later initial-value replaces leaves 0 records.
  • An error in an initial-value is reported at its place in the file. It was reported relative to the value: a.css:1:6 for a value on line 4.
  • A typed initial-value whose start matched a component was cut. <length> | <length>+ with 1px 2px built as 1px, and <transform-function> | <transform-list> with rotate(1turn) scale(2) built as rotate(1turn). Both are valid and now print whole. lightningcss 1.33.0 cuts them.
  • <color> with red blue built as red, and <length> with 10px 20px as 10px. Both are now Unexpected token, the error that each value with no matching component has on main. Chrome 153 drops the source rule of both, and registered the cut rule.
  • syntax: "<string>" was Invalid value. It is now a typed syntax (feat: support <string> in @property syntax parcel-bundler/lightningcss#1134).
  • Not changed: a typed value that no component matches is a build error. The last syntax and the last initial-value of a rule win.

Real style sheets (release builds of the merge base 017b243 and of this PR)

  • The sample: 1,161 distinct top-level @property bodies (42,737 rules) in 1,285 files, from 444 npm packages and 17 GitHub files.
  • Bodies: 1,121 print the same. 13 (7,212 rules) fail on main and build here. 0 build on main and fail here. 0 change. 27 fail both ways.
  • The 13: 9 with url() (7,200 rules: url("data:,") in @sap-theming/theming-base-content is 7,192 of them, the others are in 3 GitHub files), and 4 with <string> (12 rules, @material/web and packages that copy it).
  • The 27: 14 typed values that no component matches (css: keep a typed @property initial-value as written when no component matches #45807 is about them), 7 <string> rules whose value is not a string, 4 with a syntax name that no draft has (<ratio>, <line-style>), 2 typed rules with no initial-value.
  • Files, each built pretty and with --minify: 1,124 give the same bytes. 12 fail on main and build here. 0 build on main and fail here. 149 fail both ways for a cause outside this change.
  • A real install of @sap-theming/theming-base-content@11.36.5: content/Base/baseLib/sap_horizon/css_variables.css (65 @property rules) exits 1 on main and builds here.
  • test/js/bun/css/files/*.css and four files of tailwindcss 4.3.3 (none has @property): 0 of 38 builds differ.

Measured against Chrome 153 (headless, the source rule and the built rule, pretty and minified, 57 values)

  • 5 values with url() and 5 valid <string> values: build errors on main. Each built rule is registered here.
  • 3 valid values that main cut: each built rule registers the whole value.
  • 4 values that are not valid and that main cut, so that Chrome registered the built rule: build errors here.
  • 14 values that main builds typed, and 2 with light-dark(): each built rule is the same as on main.
  • The other 24 values are build errors on main and here. 14 of them are valid (css: keep a typed @property initial-value as written when no component matches #45807).

Cost (release builds of the merge base and of this PR, gdb on bun-profile, one call of NestedRuleParser::parse_block, rule 2 of 4 equal rules)

rule instructions Tokenizer::next_impl Parser::expect_exhausted mi_malloc
syntax:'<color>';inherits:false;initial-value:rgb(255 0 0) 8,404 -> 7,467 20 -> 13 4 -> 5 3 -> 2
the same with initial-value first 8,404 -> 8,940 20 -> 20 4 -> 6 3 -> 3
syntax:'<length>';inherits:false;initial-value:0px 4,950 -> 4,808 10 -> 9 3 -> 4 1 -> 1
syntax:'*';inherits:false;initial-value:rgb(from currentColor r g b) 14,528 -> 12,539 49 -> 37 5 -> 4 6 -> 5
syntax:'*';inherits:false 2,445 -> 2,484 6 -> 6 2 -> 2 0 -> 0
  • mi_realloc: 2 for the rule with rgb(from currentColor r g b), on main and here. 0 for the others.
  • In the sample syntax comes before initial-value in 22,279 of the 22,810 rules that have a value (97.7%). 19,927 of the 42,737 rules have no value.
  • One call of PropertyRule::to_css: 1,622 -> 1,623 instructions for the first rule of the table, 1,186 -> 1,186 for the last.
  • A style sheet with no @property (bulma.css): SyntaxString::parse_value calls 0 -> 0, output bytes identical.
  • Release binary (llvm-size -A, stripped): .text 81,368,037 -> 81,368,293 B (+256), .rodata 22,598,604 -> 22,598,604 B (+0).
  • File system calls of a build with no url() in @property (openat, open, readlink, read, pread64, write, close, getdents64, counted with gdb catch syscall, because strace is not installed here): 77 -> 77 for the first rule of the table, 121 -> 121 for bulma.css.

Not in this PR

Other PRs

Upstream

lightningcss 1.33.0 builds url() in an initial-value and the <string> syntax. The cases of its test_property are the first test of the @property block.

Self-review

24 concerns raised, 22 addressed. The review asked for this split: the as-written fallback and the print without targets are in #45807. Not done: a tracking issue for the descriptor arms and one for the media features. Both are named above.

Suites run (bun bd test, debug build with ASAN)

  • test/js/bun/css/css.test.ts: 1,321 pass, 0 fail. test/bundler/esbuild/css.test.ts: 58 pass, 0 fail.
  • Each file of test/js/bun/css and test/bundler/css, with the two files above and test/regression/issue/27117.test.ts, one file at a time: 2,760 pass, 4 fail. The 4 are in css-fuzz.test.ts, a timing test that does not run in CI: 1,000 Bun.build calls do not fit its 10 s limit on a debug build.
  • The 9 new tests fail on a release build of the merge base.

`PropertyRule::parse` buffered the `initial-value` and parsed it again
after the rule body with a second parser. That parser had no import
record list, so each `url()` in an initial-value stopped the build with
"Unexpected token: url(", for every syntax. Its positions were relative
to the value, so an error pointed at line 1 of the file.

The value is now parsed on the stylesheet's own parser: where it is read
when `syntax` is known, else after the body from its saved position. A
`url()` is an import record, as in a declaration. A value that a later
`initial-value` replaces leaves no record.

`SyntaxString::parse_value` returns a typed value only when a component
matches all of the value. It returned the first component that matched
the start, and the rest was dropped: `1px 2px` for
`<length> | <length>+` built as `1px`.

The `<string>` syntax name is accepted, as in lightningcss.
@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:20 PM PT - Oct 17th, 2026

❌ @robobun, your commit 65f3485 has 3 failures in Build #127840 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 45806

That installs a local version of the PR into your bun-45806 executable, so you can run:

bun-45806 --bun

@robobun

robobun commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator Author

Status: open, waiting for CI and review.

@coderabbitai

coderabbitai Bot commented Oct 17, 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: bc530e6d-1ce0-4b22-9e44-44dc5d931f95
📥 Commits

Reviewing files that changed from the base of the PR and between 0973e69 and 521613c.

📒 Files selected for processing: 4
  • src/css/rules/property.rs
  • src/css/values/syntax.rs
  • test/bundler/esbuild/css.test.ts
  • 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; 3 remain after this review.

Walkthrough

The CSS syntax parser adds <string> support and stricter full-input parsing for non-repeated components. Property rules now resolve initial values using the final syntax and preserve import state when descriptors are replaced. New tests cover parsing, serialization, validation, and URL resolution.

Changes

Registered CSS property parsing

Layer / File(s) Summary
Syntax component parsing
src/css/values/syntax.rs, test/js/bun/css/css.test.ts
The syntax parser recognizes, parses, serializes, and clones <string> values. It skips leading whitespace and requires non-repeated components to consume the remaining input. Tests cover string syntax and typed values.
Property initial-value resolution
src/css/rules/property.rs, test/js/bun/css/css.test.ts, test/bundler/esbuild/css.test.ts
Property rules retain parser state for initial values and resolve them using the final syntax. Tests cover descriptor order and replacement, validation, and URL inlining and resolution errors.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 52161

This change parses @property initial values with the stylesheet parser and adds the string syntax. No actionable merge-blocking risk was identified in the supplied context.

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.
Title check Passed The title clearly identifies both primary changes: parsing @property initial-value with the stylesheet parser and adding syntax.
Description check Passed The description explains the problem, implementation, behavior changes, limitations, and verification results. It does not use the exact template headings, but it provides the required information in …
Full details: Description check

Explanation

The description explains the problem, implementation, behavior changes, limitations, and verification results. It does not use the exact template headings, but it provides the required information in equivalent sections.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • 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.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

One more fix for SyntaxString::parse_string is ready on robobun/9afb4d1d/css-property-syntax-trim. It is one commit (f50de61) on top of 65f3485 and it can be cherry-picked into this PR.

  • syntax: "* " (a space or a tab after the star) still fails with Invalid value. The commit changes strings::trim_left to strings::trim at src/css/values/syntax.rs:69, as lightningcss (trim_matches) and the specification do.
  • It adds tests for that, for the forms of a <string> value, and for a <string> rule through bun build and Bun.build.

Without the one token, exactly the new whitespace test and the two css/PropertyRuleUniversalSyntaxTrailingSpace cases fail.

This branch has not been deployed

No deployments
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.

1 participant