Repository navigation
Conversation
A token list is a value with a grammar that the parser does not know: a
custom property, an unparsed or unknown declaration, an unknown at-rule,
the arguments of an unknown function or pseudo-class.
The parser dropped the whitespace next to a delimiter, a function and a
closing bracket, and it skipped comments. The printer then guessed the
blanks back. `--sel: .a .b` printed as `. a. b`, or as `.a.b` with
--minify, and `a/* c */b` printed as `ab`.
The parser now stores every whitespace and comment token, and the
printer writes the stored tokens and adds none. When it minifies, each
run of whitespace and comments between two tokens prints as one space,
or as `/**/` when the run holds only comments, and the run at each end
of a list does not print. A number keeps the sign it was written with,
and a `\` keeps the newline after it.
`bun pm diff` compares a print with one form for what no grammar reads
in these values: the amount of whitespace, comments, and the blank next
to a comma or next to `*` and `/` in a math function.
This is a port of lightningcss 32f1f52f ("Do not remove whitespace in
token lists") and aa2ed1e3.
|
Updated 1:55 AM PT - Oct 21st, 2026
❌ @robobun, your commit fd18d1d has 1 failures in
🧪 To try this PR locally: bunx bun-pr 45577That installs a local version of the PR into your bun-45577 --bun |
StatusReproduced on bun 1.4.3-canary.1+367d939d9 (linux x64) with the script from the report. 8 of 10 runs change the text of the value. On this branch 0 of 10 runs change it. printf '.c { --sel: .a .b; }\n' > s.css
bun build ./s.css --outdir out # --sel: . a. b
bun build ./s.css --outdir out --minify # --sel:.a.bPR: #45577 |
`TokenList::write_token` is the body of the print loop. Inline it, print a comma directly, and print a whitespace token of one blank without the scan for line breaks. `bun build` of the 15 sheets in test/js/bun/css/files, user instructions against main: +0.38% before this commit, -0.12% after. With --minify: +0.60% before, +0.24% after.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
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 2 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughCSS token-list parsing and printing now retain whitespace and comments for mode-specific serialization. ChangesCSS token-list handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the problem, implementation, tradeoffs, scope, and verification. It does not use the template headings exactly, but it provides the required change summary and test ev… |
| Title check | |
The title is concise, specific, and accurately summarizes the main change: preserving whitespace and comments in CSS token lists. |
| 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 clearly explains the problem, implementation, tradeoffs, scope, and verification. It does not use the template headings exactly, but it provides the required change summary and test evidence 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.
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.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟡
src/css/properties/custom.rs— Users running--minifyget larger output: two rules whose token-list values differ only in whitespace amount or a comment no longer merge, even though both print the same bytes. Token equality at src/css/properties/custom.rs:216 comparesWhitespace/Commentpayloads byte for byte, and the parser now stores every run verbatim instead of oneb" ". Fix: when minifying, compare token lists on the form the printer emits (any whitespace run equal to any other, comments ignored next to whitespace), so--x: a band--x: a bstill merge as on the base branch; the author's note that no merge is lost holds only for the fixtures measured.Why this was flagged
Input:
.a{--x:a b}.b{--x:a b}(or.a{--x:a /*c*/ b}.b{--x:a b}) built withbun build --minify. parse_into_impl at src/css/properties/custom.rs:718 now pushesToken::Whitespace(b" ")andToken::Commentas stored; the base replaced every run with a singleWhitespace(b" ")and dropped comments. Rule merging in the minify pass compares declarations with TokenList eql, which reaches src/css/properties/custom.rs:216-217 and compares the whitespace/comment bytes withstrings::eql, so the two values are unequal. The printer then collapses both runs to one space (src/css/properties/custom.rs:489), emitting.a{--x:a b}.b{--x:a b}where the base emitted.a,.b{--x:a b}. Nothing else canonicalises the lists before the comparison.Verification: nit — triggered when two rules carry the same token-list value whose whitespace runs differ only in length, built with
--minify. src/css/properties/custom.rs:216-217 compares whitespace withstrings::eql(a, b);parse_into_impl(custom.rs:715) stores whitespace verbatim, not oneb" "as on base. Rule merging at src/css/rules/mod.rs:1150 uses that equality, so the rules no longer merge.
|
A note on merge order with #45835, for the reviewer. On main, This PR makes the raw printer keep the sign of a number ( I read this diff and did not build the branch. With #45835 merged first, the value parses and never reaches the raw printer. |
…token-list-whitespace
… rule The diff print drops the blank next to `*` and `/` in the arguments of a math function. A `var()` or `env()` fallback in those arguments printed without that rule, so `calc(var(--x,10px*2))` and `calc(var(--x, 10px * 2))` had two forms in `bun pm diff`. A fallback behind a function with a grammar that is not known keeps its blanks: `calc(foo(var(--w, a*b)))` prints as written.
On main the parser stored each run of whitespace as one blank, so two rules whose values differ only in the amount of whitespace merged. With the text kept as written they did not merge, although `--minify` prints the same bytes for both. Two whitespace tokens are now equal, and hash the same, whatever their text. The merged rule prints the whitespace of the rule that stays.
|
About the merge that Two whitespace tokens now compare equal whatever their text, so these rules merge as on main: Two cases stay as they are:
|
…token-list-whitespace
| // Seeded round-trip test of CSS token lists. A token list is a value whose grammar the parser | ||
| // does not know, so it must print with the tokens it was written with. Replay a failure with | ||
| // BUN_CSS_TOKEN_FUZZ_SEED=<seed> (and BUN_CSS_TOKEN_FUZZ_ITERS for a soak). | ||
| import { cssInternals } from "bun:internal-for-testing"; |
There was a problem hiding this comment.
🟡 nit (optional): Maintainers get a new test file for CSS token-list behavior instead of the coverage living beside the existing CSS tests. CLAUDE.md:49 says to add tests to the existing test file for the code being changed and not to create a new file. Fix: move the seeded round-trip cases into test/js/bun/css/css.test.ts (next to the new "whitespace and comments in token lists" describe block), or state in the PR why a separate file is needed.
Why this was flagged
The PR adds test/js/bun/css/token-list-roundtrip.test.ts, a new file that exercises TokenList::parse_into and the printer in src/css/properties/custom.rs. The applicable root CLAUDE.md:49 states "Default: add your test to the existing test file for the code you're changing. Do not create a new file." and test/CLAUDE.md:152 repeats it. The existing file for this module is test/js/bun/css/css.test.ts, which this PR already extends with a describe block for the same behavior at css.test.ts (the "whitespace and comments in token lists" block). On the base branch no such file exists; after merging the module's coverage is split across two files, which the convention exists to prevent. No safeguard applies; this is a convention check, not a runtime failure.
Verification: nit. Trigger: unconditional — the PR adds a brand-new test file. git diff --stat shows test/js/bun/css/token-list-roundtrip.test.ts as a new file, while the same PR already extends the existing test/js/bun/css/css.test.ts with the same TokenList parse/print behavior. CLAUDE.md:49: "Do not create a new file." The PR description gives no reason for a separate file.
This branch has not been deployed
2 participants
Problem
bun buildre-spaces CSS values that the parser keeps as tokens: custom properties,var()values, unknown at-rules.--sel: .a .bprints as. a. b, or.a.bwith--minify.@apply a/80prints asa / 80(Bun css transpiler formats the output wrongly when using @apply utility from tailwindcss #16712).TokenList::parse_into_impl(src/css/properties/custom.rs:551) drops whitespace next to delimiters, functions and brackets, and skips comments.TokenList::to_cssguesses the blanks back.Fix
--minifyprints each run of whitespace and comments as one space, or as/**/for comments only. lightningcss prints nothing there:a/* c */bbecomesab.test/js/bun/css/css.test.ts,token-list-roundtrip.test.ts,bun-pm-diff.test.ts. Self-reviewed: 16 concerns raised, 15 addressed. Not done: the upstream report.Background
TokenListholds a value with no typed grammar as raw tokens.bun pm diffcompares re-printed CSS.PrinterOptions::canonical_token_listsgives it one form for whitespace and comments..and*only..a .band.a.bwere stored as the same tokens.Downsides
gzip -9).bun buildof the 15 fixtures uses about 2 MB more memory (peak RSS +2.3% to +2.7%). Instructions: -0.13%, +0.24% with--minify.bun pm diffreports a blank that a minifier adds after): lightningcss 1.30.2 against 1.32.0 on bulma.css reads +33 -33.Notes
Examples
--minify--sel: .a .b. a. b(.a.bminified).a .b.a .b@custom-variant dark (&:is(.dark *));(& :is(. dark * ))@apply text-[#fff] bg-[#000]text-[#fff]bg-[#000]width: attr(data-w type(<length>), 100px)type(< length> )filter: progid:DXImageTransform.Microsoft.gradient(...)DXImageTransform. Microsoft. gradient(margin: var(--a) autovar(--a)autovar(--a) auto--x: a/* c */baba/**/b.a:foo(a//**/*b).a:foo(a/*b), an open comment.a:foo(a//**/*b)--x: rgb(1 2 3 / *x)rgb(1 2 3/*x), an open commentrgb(1 2 3/ *x)--x: 2n+12n12n+1--r: 16 / 916/916 / 9.c{--sel:.a .b}.d{--sel:.a.b}Where this differs from lightningcss
--minifya comment that is the only separator prints as/**/. lightningcss prints nothing, so it printsaband an open/*.--minifythe whitespace and comments at each end of a list do not print, and a value of only whitespace and comments prints as one blank. Printed as/**/it would be an empty value, which older browsers reject.calc(1px+2px)ascalc(1px2px).\is a token only before a newline, so a newline follows it in the output. Before)or;it is an escape.\nfor CR LF, CR and FF in the source. The output does not follow the line endings of the checkout.TokenList::to_css. lightningcss moved it into each caller.PrinterOptions::canonical_token_listsis new.bun pm diffis its only user.The text of a value is kept as written without
--minify, as in lightningcss. The printer no longer adds a blank on each side of a+or-delimiter.#33683 added a check to
TokenList::to_cssthat keeps/and*apart under--minify. This PR removes the check. The parser stores the blank now, so the printer has nothing to guess.Not in this PR
z-[2147483647]prints asz-[2147480000](css: print integer tokens from their i32 instead of a 6 digit float #38482).#hashprints as a color:bg-[#FF0000]prints asbg-[red], andrgb(1,2,3)xprints as#010203x.Measurements
Release builds (
--profile=release, ThinLTO) of the merge base d940fbe and of this PR, linux x64. The workload is onebun buildwith the 15 sheets intest/js/bun/css/filesas entry points (2.9 MB), pinned to one CPU. Counters come fromperf_event_open, user space only. Each number is the median of 30 runs.--minify--minify: 132.42 -> 136.41 ms (+3.0%, noise floor 1.1%). The noise floor is the difference between two sets of runs of one binary.--minify: 147.19 -> 148.57 ms, noise floor 13.02 ms. Two copies of the base binary differ by 1.67 ms. The delta does not show.sizetext 106,411,931 -> 106,414,776 (+2,845 bytes). Data and bss do not change. Stripped file 106,555,144 -> 106,559,240 bytes.size_of::<TokenOrValue>()is 72 before and after.PrinterandPrinterOptionsget onebooleach. One host function is new (canonicalTest, inbun:internal-for-testingonly).TokenList::parse).tokens.shrink_to_fit()at the end ofTokenList::parseis not in this PR. In this session it took peak RSS to 77,984 KB (-3.0% against the merge base) and instructions to +0.46%, +0.83% with--minify. The spare capacity is on main too, so that change stands alone.gzip -9290,279 -> 290,705 (+426, +0.15%). bulma.css +7,827 raw, +323 aftergzip -9.--minifytoo. A browser does not read it in the value of a known property, so--minifycan drop it there again if the bytes matter more than one rule for every token list./**/separators added under--minify: 0.--minify: 8,820 changed lines of 114,836.a, banda,b) print differently now, so those rules no longer merge.bun pm diff: a multi-line custom property with new indentation reads formatting only. tailwindcss 4.3.3 formatted again by Prettier: 3 of 3 files formatting only. A sheet against itsbun fmtoutput: formatting only. bulma.css minified by lightningcss 1.30.2 and by 1.32.0: formatting only -> +33 -33, each line a blank after).url()rows that the bundler cannot resolve.Tests
exact_testintest/js/bun/css/util.tscompares byte for byte.cssTestignores all whitespace and stays as it is.canonicalTestinbun:internal-for-testingprints asbun pm diffdoes.test/js/bun/css/,test/cli/install/bun-pm-diff.test.ts,test/bundler/css/,test/bundler/esbuild/css.test.ts,test/bake/dev/css.test.ts,test/bundler/bundler_html.test.tsand 13 CSS files intest/regression/issue/.css-fuzz.test.tsdoes not run in CI and times out on a debug build. On release builds it passes in the same time before and after.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/bun-pm-diff.test.ts