Repository navigation
Conversation
The prelude and the block of an unknown at-rule went through the parser for
declaration values. That parser reads a hash as a color, so the ID selector
in `@foo { a#f00:hover {} }` printed as `ared:hover`.
Store this text as `RawTokens`: a list that holds tokens, the source text of
numbers, and `url()` imports. It has no entry for a color, a length or a
`var()`. The arguments of an unknown pseudo-class or pseudo-element use the
same type. It replaces `TokenList::parse_raw` and `TokenList::to_css_raw`.
`TokenList::parse` is now private, and `parse_value` is its one caller
outside the module.
|
Updated 7:24 AM PT - Oct 22nd, 2026
❌ @robobun, your commit b1ede6c has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46061That installs a local version of the PR into your bun-46061 --bun |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughUnknown CSS at-rules and custom selector function arguments now use context-aware raw tokens. The changes add raw-token parsing and serialization, update related exports and call sites, and add tests for token preservation, URL inlining, invalid tokens, and nesting limits. ChangesCSS Raw Token Preservation
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely describes the main change: preserving unknown at-rule preludes and blocks as raw tokens. |
| 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. |
| Description check | |
The description is complete and relevant. It explains the problem, implementation, scope, trade-offs, limitations, and verification results. Although it does not use the exact template headings, it pr… |
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, implementation, scope, trade-offs, limitations, and verification results. Although it does not use the exact template headings, it provides the required content, including how the code was verified.
- 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.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
Beyond the inline findings, I also checked that RawTokens::parse_into keeps the old parse_raw contract (same 500-deep nesting limit, same parse-error token rejection), that the number source slice is safe after a nested block or url() (the parser state is re-taken at the top of every loop iteration, so slice_from_cloned only covers the number token), and that number_len and write_gap's gap.start - 1 are bounds-safe (.get() throughout; inside guarantees gap.start > 0).
Extended reasoning...
The change replaces TokenList-based parsing of unknown at-rule preludes/blocks and unknown pseudo-function arguments with a new RawTokens type in src/css/properties/custom.rs, touching css_parser.rs, rules/unknown.rs, selectors/parser.rs and selector.rs, plus ~250 lines of new tests. It touches no auth, crypto or permission surface; the parsing is of CSS source text with bounded index arithmetic. Two inline findings were reported, so human review is already signalled; this note only records the additional paths examined.
# Conflicts: # test/bundler/esbuild/css.test.ts
Problem
bun buildreads an unknown at-rule as a declaration value.@foo { a#f00:hover {} }prints@foo{ared:hover {}}: the ID selector#f00becomes the colorred.TokenList::parseatsrc/css/css_parser.rs:365and:1881. It also prints.a .bas.a.band fails the build onvar(--value([*]), 0%)in@utility.Fix
RawTokens(src/css/properties/custom.rs): tokens, number source text andurl()imports. No entry is a color, a length or avar().TokenList::parse_rawandto_css_raw.test/js/bun/css/css.test.ts(155 new, 121 fail on main),test/bundler/esbuild/css.test.ts(4 new, all fail on main). Alsotest/js/bun/css,test/bundler/css.Background
@custom-selector,@theme, a rule from a new CSS module.TokenListholds a declaration value and folds#ff0000tored. That stays for--x: #ff0000.TokenList. It adds a branch for each hex color in each value and stops 1 of 9 rewrites.Downsides
@theme{--x:#ff0000}stays as written, as in esbuild. lightningcss folds it. A Tailwind 4 directive sample: 1,031 to 1,088 bytes.bun pm diff:#FFFto#fffin@themeis no longer "formatting only".url()in a value costs 16 more instructions (204 to 220): its printer is no longer inlined. Release.text: +256 bytes.Notes
Report. There is no GitHub issue, and no user reported the hash fold. It was found during the work on #45362. The one user report on this path is #16712 (
@apply something/80printedsomething / 80). It is closed, and main still prints that text without--minify. Related to #16712.Before and after (
--minify, main is 91e7e3b)@foo { a#f00:hover { color: red } }@foo{ared:hover { color: red }}@foo{a#f00:hover { color: red }}@foo { #FACADE, a#aabbcc { } }#facade,a#abc@custom-selector :--h a#f00, #FACADE;:--h ared,#facade@foo { .a .b > .c { } }.a.b>.c.a .b > .c@foo { a:nth-child(2n+1) { } }2n12n+1@foo { z-index: 9999999; width: 10PX }10000000,10px@foo { a/**/b { } }aba/**/b@utility x { opacity: var(--value([*]), 0%) }@theme { --color-x: #ff0000 }--color-x: red--color-x: #ff0000a:unknown(p-2.5) { }a:unknown(p-20.5)a:unknown(p-2.5)Without
--minify, main also adds spaces:.a .bprints. a. bandsomething/80printssomething / 80.What changes in an unknown at-rule
var()andenv()print as the same tokens. A number prints its source text.--minify. Main dropped it next to a delimiter, a comma and a closing bracket, and added spaces back when not minifying./**/when it is all of the text between two tokens. In every other place it does not print. Main dropped every comment, soa/**/bprintedab.var()orenv()that is not a value no longer fails the build. In a declaration it still does (a{x:var()}).@foo { }prints@foo{}(main:@foo{ }).url()is an import in the prelude and in the block, as on main.eis written as\65, becauseeafter a number can start an exponent.What changes in the arguments of an unknown pseudo-class or pseudo-element
This half is a consolidation. The two raw helpers of
TokenListhad these callers only, and the new type replaces them. Real selectors print the same bytes.:x(p-2.5)printed:x(p-20.5),1e3printed1000.0)./**/(:x(a/**/b)printed:x(ab)).a:x(.5)anda:x(0.5)are no longer merged as duplicates. Main compared the parsed numbers.url()print as on main.Other changes in the diff
TokenList::parseis private to its module.TokenList::parse_valueis the one door for a caller outside it (SyntaxString::parse_value), so a new caller has to name the value context.TokenList::parse_raw,TokenList::to_css_rawand theTokenListFnsre-exports are deleted. Two comments that named them are corrected.Not in this PR
a{--x:a#f00}prints--x:ared(css: fold relative colors on the scale of their function, and keep them as written when the fold is not exact #45362 puts a space there),element(#FACADE)printselement(#facade)..btn{@apply p-2}prints@apply p-2;with the default target. lightningcss fixed this in 729e03e2. The rows here that nest an at-rule use no target.@container scroll-state(...), CSS parser rejects@container scroll-state(...)queries ("Unexpected token: scroll-state(") #45237).url()in the prelude of an unknown at-rule is still an import. css: keep a url() in an at-rule prelude as written, and never print the resolved path of a url() #45604 changes that.bun pm diffhas no canonical print for this text. css: keep whitespace and comments in token lists #45577 adds one for token lists.URL("x.png"), with an upper-case name and a quoted string, is not an import in this text.RawTokens::parse_intotests the name asTokenListdoes on main (urlexactly), so--x: URL("x.png")and@foo { src: URL("x.png") }still agree. The unquoted formURL(x.png)is an import in both.--minify:@foo {, the line,}. Main prints the same bytes.@font-face {}has no such line.Questions for a maintainer
a/**/bprintsab). Is it accepted that Bun leaves lightningcss here? If a declaration in an unknown at-rule must keep its minification, the type stays and a splitter adds a value entry for each declaration.url()is not an import), css: keep whitespace and comments in token lists #45577 (token lists keep whitespace and comments, and the printer collapses them) and css: fold relative colors on the scale of their function, and keep them as written when the fold is not exact #45362 (a separator between tokens that would join). This PR is written to land in any order:parse_custom_at_rule_preludekeeps its wrapper, and theurl()arm ofRawTokens::parse_intoreads its flag.RawTokensalready stores every whitespace and comment token.RawTokens::to_cssthen takes its collapse rule from the printer, and theRawTokensContextargument ofto_cssgoes away.Self-review. Addressed: the join table of the first draft is gone (it missed
/before*=, which opened a comment), an escaped unit is written again, whitespace is collapsed by the printer and not by the parser,TokenList::parseis private, the stale comments and theTokenListFnsre-export are gone, tests for the error arm, the copies of a rule and the duplicate check. Not done:url()with no import. It is the change of css: keep a url() in an at-rule prelude as written, and never print the resolved path of a url() #45604.a{z-index:2147483647}prints2147480000). css: print integer tokens from their i32 instead of a 6 digit float #38482 works on that.Measured (release builds of 91e7e3b and of f0dd700 on x64 Linux. f0dd700 is this PR before main was merged into it, and the diff of
src/is the same now. Instruction counts are from qemu-user 10.0.13 with one instruction per block, for the code ofbun_css)TokenList::parse_into_impl: 5,750 bytes and 1,222 instructions in both builds, 0 differing instructions (llvm-objdump).TokenList::to_css: 7,135 -> 5,556 bytes.ext::url_to_css(1,488 bytes) is no longer inlined into it, becauseRawTokens::to_cssalso calls it. Instructions inside the two functions:a{--x:url(./a.png) 1px}204 -> 220,a{--x:1px solid #fff}197 -> 195.TokenList::parse: 429 -> 436 bytes, 42 -> 41 instructions for one call.TokenList::to_css-13,899,TokenList::parse-2,938, token equality +4,746 (Token::eqlis no longer inlined).drop_glueruns 1 to 3 times in a build (618 instructions each), in both builds. No other count changes between runs.@foo { a:is(.b) { width: calc(1px + var(--x)) } }: 12,313,241 -> 11,530,859 instructions (-6.4%).TokenList::parsecalls 4,000 -> 0. Vec growths 5,000 -> 4,000.parse_hash_colorcalls 1 -> 0..text81,665,765 -> 81,666,021 bytes (+256),.rodataequal.RawTokens::parse_intois 2,041 bytes andRawTokens::to_cssis 973 bytes.TokenList::parse_raw(1,508 bytes) is gone.size_of::<RawToken>()is 32 bytes, equal toToken(const assertion). ATokenOrValueis 72.test/js/bun/css/files, with and without--minify: 30 of 30 outputs byte-identical.@theme379 -> 407,@apply212 -> 226,@utility143 -> 143,@custom-variant185 -> 191,@custom-selector112 -> 121.valgrindandperfare not on the build machine.Not in the test suite. Two random runs on the debug build found no difference: 80,000 sequences of tokens with random separators print the expected text, and 30,000 dimensions with escaped units print as the same token in an at-rule and in a pseudo-class (the value path reads both spellings the same way).
Suites run. Debug build:
css.test.ts(1,469 pass),test/bundler/esbuild/css.test.ts(61 pass, 1 todo),test/bundler/css/(184 pass),test/bake/dev/css.test.ts(15 pass),test/bundler/bundler_html.test.ts(24 pass),test/js/bun/http/bun-serve-html.test.ts(31 pass, 2 skip). Release build of this PR: all oftest/js/bun/css/(2,672 pass, 0 fail). On the debug build,css-fuzz.test.tsandnested-selector-expansion.test.tsin that directory reach their time limits. They have no unknown at-rule. The first three suites were run again after main was merged in.