Skip to content

css: keep scanning an @supports condition after a nested block - #38552

Closed
robobun wants to merge 9 commits into
mainfrom
farm/1e1431e9/css-supports-tokens-after-block
Closed

robobun wants to merge 9 commits into
mainfrom
farm/1e1431e9/css-supports-tokens-after-block

Conversation

@robobun

@robobun robobun commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • bun build fails when a token follows a function or bracket block inside an @supports condition. @supports (transform: translate(1px) rotate(1deg)) gives error: Unexpected token: rotate(. selector(:is(a) > b) and @import ... supports() fail the same way.
  • The cause is Parser::expect_no_error_token (src/css/css_parser.rs:3267). It returns after the first nested block. The caller then rejects the unread tokens.

Fix

  • Remove the early return. The loop now scans to the end of the input.
  • Correct because rust-cssparser 0.36 has the same loop without that return (src/parser.rs:1030). An error token after a block still fails.
  • Verified: the supports block in test/js/bun/css/css.test.ts (17 of its 22 tests fail without the fix). The Notes list the other suites.

Background

  • The parser keeps most of an @supports condition as a raw slice of the input and prints it back. expect_no_error_token scans to the end of that slice and rejects error tokens, for example a stray closing bracket.
  • parse_nested_block runs a closure on one block and fails with Unexpected token if the closure leaves a token unread.
  • Considered a fix in each of the three callers (src/css/rules/supports.rs). One fix here covers all three.

Downsides

  • In a scanned condition, each nested block that ends its enclosing block costs one more tokenizer call, which returns end of input and allocates nothing. Instructions not measured: the build machine has no perf counters and no valgrind.
  • Output change (pinned by tests): selector(:is(a) ) now keeps the whitespace or comment before the closing parenthesis. Conditions with no nested block already kept it.
Notes

Three example conditions. Each one exits 1 on a release build of 367d939 (1.4.3-canary.1) and builds with this change. esbuild 0.25.1 builds all three.

Condition Before
@supports (transform: translate(1px) rotate(1deg)) Unexpected token: rotate(
@supports (background: linear-gradient(red, blue) no-repeat) Unexpected token: no-repeat
@supports selector(:is(a) > b) Unexpected token: >

The same values and selectors build in an ordinary rule, and @supports (transform: translate(1px)) builds too.

Callers. expect_no_error_token has three callers, all in src/css/rules/supports.rs: parse_declaration (used by @import ... supports(declaration)), the selector() arm of parse_in_parens, and the unknown-condition fallback of parse_in_parens. An @supports (property: value) condition goes through the fallback. Each caller runs inside parse_nested_block, so an unread token fails the at-rule prelude, and a CSS parse error fails the build.

History. The Zig parser had the same early return from the day it was added (#14122), so this is not a regression from the Rust port.

Inputs that change. Probed on the same release build and on this branch with the minify test helper.

Input Before This branch
@supports (a: f() g()) and (b: [x] y) error kept as written
@supports foo(a b(c) d) error kept as written
@supports ((a) b) error kept as written
@supports selector(:is(a) ) selector(:is(a)) selector(:is(a) )
@supports selector(:is(a) /* c */) selector(:is(a)) selector(:is(a) /* c */)
@import url(x.css) supports(transform: translate(1px) ) supports(transform:translate(1px)) supports(transform:translate(1px) )
@supports selector(a ) selector(a ) selector(a )
@supports (a: f() "x) error error
@supports (a: f() url(x y)) error error
@supports (transform:translate(1px) ]) error error
@supports foo(bar) baz error error

Cost for inputs that built before. After a nested block that is the last token of its block, the loop calls next_including_whitespace_and_comments once more. That call reads the closing bracket, matches it against the stop set, and returns end of input. The nesting depth of the scan does not change, so the nesting depth limit behaves as before. The diff removes one statement and adds none.

Suites run with the debug (ASAN) build of this branch merged with main at e25d24c:

  • test/js/bun/css/css.test.ts: 1336 pass, 69 skip, 3 todo, 0 fail. With the source change reverted on the same build: 17 of the 22 supports tests fail. The five that pass both ways are the ERROR: cases, which check that an error token after a block is still rejected.
  • test/js/bun/css/: supports-condition-newline, nested-selector-list-expansion, css-loader, selector-list-error-recovery, doesnt_crash: all pass.
  • test/bundler/esbuild/css.test.ts: 56 pass, 1 todo.

test/js/bun/css/css-fuzz.test.ts does not run in CI (if (!isCI)). On an earlier merge with main at 6c1eb06, its four tests timed out under the debug build with and without this change. Its inputs contain no @supports rule.

Related. #38589 parses (property: value) conditions as declarations and merges adjacent rules with the same condition (#24770). #38559 fixes the space that the printer puts before not. Neither changes this scan.


[human-review] gate passed · iteration 0 · 2 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
[ninja] fetching https://github.com/oven-sh/ninja/releases/download/bun-ninja-5ecd8831/bun-ninja-linux-x64.zip
[ninja] extracted to /root/.bun/build-cache/ninja/bun-ninja-5ecd8831 (492ms)
ninja: Entering directory `/workspace/bun/build/debug'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/debug/rust-target/plan.json
FAILED: [code=1] rust-target/plan.json /workspace/bun/build/debug/rust-target/plan.json 
/workspace/bun/build/release/bun /workspace/bun/scripts/build/stream.ts cargo --console /workspace/bun/build/release/bun /workspace/bun/scripts/build/rust/plan.ts /workspace/bun/build/debug/rust-target/plan.input.json /workspace/bun/build/debug/rust-target/plan.json
�[1m�[91merror�[0m: failed to load manifest for workspace member `/workspace/bun/src/lint/driver`
referenced by workspace at `/workspace/bun/Cargo.toml`

Caused by:
  failed to parse manifest at `/workspace/bun/src/lint/driver/Cargo.toml`

Caused by:
  error inheri
... (truncated)

release without fix: 41 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.45ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [0.62ms]
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;
      } [1.34ms]
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.21ms]
Output .foo {
  content: "+";
}

(pass) css tests > .foo {
  content: "\2b";
} [1.20ms]
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;
      } [1.12ms]
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)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     1cc1b9c35b
  features     lto, baseline

23 deps, 155 codegen, 1183 objects in 2704ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
266 units: 192 lib, 16 proc-macro (host), 20 custom-build (host), 16 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[3/4] reconfigure
[1/1537] mkdir codegen
[2/1537] mkdir stamps
[3/1537] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/1537] install /workspace/bun
bun install v1.4.3-canary.1 (367d939d9)

Checked 22 installs across 44 packages (no changes) [2.26s]
[5/1537] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (367d939d9)

Checked 1 install across 2 packages (no changes) [70.00ms]
[6/1537] rustc unicode_ident 
[7/1537] rustc unicode_xid 
[8/1537] gen string-map defines_table
[9/1537] rustc heck 
[10
... (truncated)
diff hotspot
src/css/css_parser.rs       |  1 -
 test/js/bun/css/css.test.ts | 92 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 91 insertions(+), 2 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                         reads  edits  tests
src/css/css_parser.rs            0      0      4
test/js/bun/css/css.test.ts      1      4      4

root cause · written by the author bot

The CSS parser's expect_no_error_token returned as soon as it finished validating a nested block, so any tokens that followed that block inside an @supports or @import supports() condition were never inspected and malformed conditions were silently accepted. The fix makes the function keep scanning the remaining input after the nested block check, so trailing tokens are now examined and reported as parse errors like any other unexpected input. Tests were added covering trailing tokens, malformed conditions, minification errors, and bundled output for both @supports and `@import supp…

Parser::expect_no_error_token returned as soon as it had checked the
first function or bracket block, so any token following that block in a
@supports condition (or an @import ... supports() condition) was left
unconsumed. The enclosing parse_nested_block then failed on the leftover
tokens and the whole stylesheet failed to build with "Unexpected token".

Continue the loop after the nested block instead, matching upstream
rust-cssparser, so the condition is scanned to the end and error tokens
behind a block are still reported.
@coderabbitai

coderabbitai Bot commented Aug 14, 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: Essentials
  • Run ID: 004e09bc-111b-46db-9ff4-76e7e3283ce2
📥 Commits

Reviewing files that changed from the base of the PR and between 6c1eb06 and e7a73ac.

📒 Files selected for processing (2)
  • src/css/css_parser.rs
  • test/js/bun/css/css.test.ts
💤 Files with no reviewable changes (1)
  • src/css/css_parser.rs

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

Walkthrough

The CSS parser now continues scanning after checking a nested block. New tests cover trailing tokens in @supports and @import supports() conditions, including minification errors and bundled output.

Changes

CSS parse-error handling

Layer / File(s) Summary
Scan trailing tokens in supports conditions
src/css/css_parser.rs, test/js/bun/css/css.test.ts
expect_no_error_token continues scanning after checking a nested block. Tests cover trailing tokens, malformed conditions, and bundled output for @supports and @import supports().

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e7a73

No actionable merge-blocking issue remains; merge after normal checks.

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 and concisely describes the main parser fix: continuing to scan an @supports condition after a nested block.
Description check Passed The description provides the problem, fix, rationale, behavior changes, testing results, and limitations. It does not use the template headings exactly, but it contains the required information for wh…
Full details: Description check

Explanation

The description provides the problem, fix, rationale, behavior changes, testing results, and limitations. It does not use the template headings exactly, but it contains the required information for what the PR does and how it 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.

❤️ Share

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

@robobun

robobun commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:05 AM PT - Oct 17th, 2026

❌ @robobun, your commit 1cc1b9c has some failures in Build #127145 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 38552

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

bun-38552 --bun

@robobun

robobun commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review

  • Reproduced with bun build on a release build of 367d939 (1.4.3-canary.1). Each of these conditions fails with Unexpected token:
    • @supports (transform: translate(1px) rotate(1deg)) { a { color: red } }
    • @supports (background: linear-gradient(red, blue) no-repeat) { a { color: red } }
    • @supports selector(:is(a) > b) { a { color: red } }
  • Root cause: Parser::expect_no_error_token returns after the first nested block. This PR removes that return, which makes the loop the same as the one in rust-cssparser 0.36.
  • The branch includes main at 7f3bc6a. The source diff is still that one removed line.
  • Checked with the debug build of this branch: all three conditions build, and test/js/bun/css/css.test.ts passes (1336 pass, 0 fail). With the source change reverted, 17 of the 22 tests in its supports block fail.
  • All review notes are addressed in 8e83988 and e76ced6 (tests only): the output for whitespace and comments after a nested block is pinned, the bundler test compares the whole bundle text, and the block comments are one line each.
  • CI on the current head (build 127145): 179 jobs passed and no test is red. The check is red only because the two darwin x64 test jobs expired with no agent. The same happened in build 126653. That lane last ran css.test.ts in build 126448, with the same assertions: 1336 pass, 0 fail. Build 124555 (same source change) passed all 181 jobs.

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

LGTM — one-line removal of an incorrect early return, matching upstream rust-cssparser.

What was reviewed:

  • Confirmed expect_no_error_token has exactly three callers, all in src/css/rules/supports.rs, and each wants the whole block consumed before taking a raw slice — the early return left tokens unconsumed and parse_nested_block's exhaustion check turned them into hard errors.
  • Checked that error tokens appearing after a nested block are still rejected (the loop now reaches them; covered by the four ERROR: tests), and that junk after a complete condition is still caught by the prelude exhaustion check (@supports foo(bar) baz test).
  • Verified minify_test_with_options is the raw synchronous cssInternals.minifyTestWithOptions (not a test() wrapper), so expect(() => ...).toThrow() is the right shape for the error cases.
Extended reasoning...

Overview

This PR removes a single return Ok(()) from Parser::expect_no_error_token in src/css/css_parser.rs. The function is a loop that scans a token stream for CSS "error tokens" (bad string, bad url, stray closing bracket), recursing into nested blocks. The early return caused the scan to stop after the first function or bracket block instead of continuing to the end of the input. Its callers — all three in src/css/rules/supports.rs — run it inside parse_nested_block, which requires the closure to fully consume the block; leftover tokens after the first nested block therefore surfaced as Unexpected token and failed the whole stylesheet on inputs like @supports (transform: translate(1px) rotate(1deg)).

The fix drops the early return so the loop iterates until next_including_whitespace_and_comments returns Err (end of input). This is exactly what upstream rust-cssparser's expect_no_error_token does. A doc comment update and 19 new tests in test/js/bun/css/css.test.ts accompany the change.

Security risks

None. This is a CSS tokenizer scan that produces no side effects beyond advancing the parser position and returning Ok/Err. Removing an early return from a loop that already recurses into nested blocks does not change nesting depth (the first block was already descended into), so the existing depth limit is unaffected. No memory safety, no untrusted-size arithmetic, no allocation.

Level of scrutiny

Low-to-medium. The source change is one line in a self-contained helper with a well-defined contract ("consume until end, error on error tokens"). I grepped for all callers and confirmed the PR description's claim that only supports.rs uses it, so blast radius is limited to @supports and @import ... supports() conditions. The behavior now matches the reference implementation (rust-cssparser / lightningcss), which is the spec per the repo's "ported code" rule.

Other factors

Test coverage is thorough for a one-line fix: it hits all three raw-slice SupportsCondition shapes (parenthesized, selector(), declaration form via @import), all four block openers (function, (), [], {}), and/or combinations, a rule following the @supports block, four negative cases proving error tokens after a block are still caught, one negative case for junk after the condition, and a Bun.build integration test that exercises the bundler's @import ... supports() → @supports transform. The author verified 14/19 fail on the released binary (USE_SYSTEM_BUN=1) and all pass with the change, and ran the surrounding CSS test files. I also confirmed minifyTestWithOptions is the raw throwing internal (not a test()-wrapping helper), so the expect(() => ...).toThrow("Unexpected token") assertions are non-vacuous.

Comment thread src/css/css_parser.rs Resolved

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

LGTM — one-line fix that removes an incorrect early return in expect_no_error_token, matching upstream rust-cssparser.

What was reviewed:

  • Confirmed all three callers (supports.rs: declaration, selector(), unknown fallback) take slice_from_cloned after the call and need full consumption — the early return was the bug.
  • Checked that continuing the loop after parse_nested_block is safe: it consumes through the closing bracket and clears at_start_of, so the next iteration resumes correctly; nesting depth is unchanged so MAX_NESTING_DEPTH behaves as before.
  • Verified error-token and junk-after-condition negative tests still hit the right rejection paths.

The comment-cop bot flag on line 3302 is a false positive — the added doc sentence describes what the function does, not a workaround.

Extended reasoning...

Overview

This PR removes a single return Ok(()) from Parser::expect_no_error_token in src/css/css_parser.rs, so that after recursing into a nested function/bracket block the loop continues scanning subsequent tokens instead of returning early. It also extends the doc comment by one sentence and adds a 19-test describe("supports") block to test/js/bun/css/css.test.ts.

Correctness

expect_no_error_token is called from exactly three sites, all in src/css/rules/supports.rs, and each caller immediately takes slice_from_cloned(pos) over the consumed range to store the raw condition text. All three run inside parse_nested_block, which errors on any unconsumed remainder — that unconsumed remainder is precisely what the early return left behind, producing the reported Unexpected token: rotate. Removing the return matches upstream rust-cssparser's implementation of the same function and is exactly what every caller needs. parse_nested_block fully consumes through the matching close bracket and decrements nesting_depth before returning, so the outer loop's next next_including_whitespace_and_comments() picks up right after the block; there is no state-machine hazard from continuing.

Security risks

None. This is a CSS tokenizer scan over already-tokenized input; no allocation, syscall, or trust boundary is touched. Nesting-depth handling is unchanged (the first block was recursed into before too), so the existing MAX_NESTING_DEPTH guard is unaffected.

Level of scrutiny

Low-medium. A one-line control-flow fix in a well-scoped helper with only three callers, brought in line with the upstream reference implementation. The PR description demonstrates a clear root-cause understanding.

Other factors

Test coverage is thorough: all three SupportsCondition raw-slice shapes, all four block-opening token kinds (function, (, [, {), and/or combinators, error tokens after a block (still rejected), junk after a complete condition (still rejected via prelude exhaustion), a following-rule test (block boundary intact), and an end-to-end Bun.build including @import ... supports(). Author reports 14/19 fail with USE_SYSTEM_BUN=1 and all pass with the debug build, plus green on the surrounding CSS test files. The comment-cop bot flagged the doc-comment line, but the added text ("Nested blocks and functions are checked recursively, and parsing continues after each of them") is a one-sentence factual description of the function's contract, not a workaround justification — a bot false positive, not an unaddressed reviewer concern.

Comment thread src/css/css_parser.rs Resolved

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

LGTM — one-line removal of a spurious early return in expect_no_error_token, matching upstream rust-cssparser and the function's own doc comment.

What was reviewed:

  • Confirmed the loop still terminates (next_including_whitespace_and_comments returns Err at exhaustion → Ok(())) and error tokens after a nested block are still rejected.
  • Confirmed the only callers are the three sites in src/css/rules/supports.rs, all of which want the whole block consumed; no other consumers affected.
  • Tests cover all three raw-slice condition shapes, function/[]/{} blocks, and/or, error-token-after-block rejection, junk-after-condition rejection, and a Bun.build integration test; error assertions use a synchronous .toThrow on the internal minify helper.
Extended reasoning...

Overview

The source change is a single deleted line in Parser::expect_no_error_token (src/css/css_parser.rs:3310): the early return Ok(()) after recursing into a nested block is removed so the loop continues to the next token. The function now matches its doc comment ("Parse the input until exhaustion") and upstream rust-cssparser. The only callers are three sites in src/css/rules/supports.rs (parse_declaration, the selector() arm, and the unknown-condition fallback), all invoked inside parse_nested_block, which requires the closure to consume the entire block — so leftover tokens after a function/bracket block previously produced a spurious "Unexpected token" error on valid CSS like @supports (transform: translate(1px) rotate(1deg)). The test file gains a 19-test describe("supports", ...) block covering the fix.

Security risks

None. This is a CSS tokenizer scan over already-tokenized input; no allocation sizing, no untrusted length arithmetic, no new external I/O. Nesting depth is unchanged (the first block was already recursed into before the early return), so the existing max-nesting-depth guard behaves as before. The loop terminates because next_including_whitespace_and_comments() returns Err at end-of-input, hitting the return Ok(()) at the top of the loop.

Level of scrutiny

Low-to-moderate. The mechanism is straightforward and provably confined: grep confirms expect_no_error_token has no callers outside supports.rs and its own recursive call. The change strictly loosens acceptance of previously-rejected valid input, and the negative tests confirm that error tokens (], }) appearing after a block are still rejected, and that junk after a complete condition (@supports foo(bar) baz) is still rejected by the prelude exhaustion check.

Other factors

  • The test coverage is thorough by the repo's own standards: all three caller shapes, all three block-open token kinds (function, [], {}), and/or chains, @import ... supports(...), a rule-follows-@supports case, five negative cases, and a Bun.build end-to-end. The error tests use a synchronous () => minify_test_with_options(...) inside .toThrow("Unexpected token"), so they can actually fail (no async-arrow-to-toThrow pitfall).
  • The comment-cop bot flagged an added doc-comment sentence in an intermediate commit; the author reverted it in ffe2806 so css_parser.rs now diffs to exactly the one removed line. Those inline comments are resolved.
  • The PR body documents USE_SYSTEM_BUN=1 failing 14 of the 19 new tests and all passing on the debug build, plus green ASAN/release runs and a green CI on the fix commit (the one Windows aarch64 timeout is an unrelated known flake).

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread test/js/bun/css/css.test.ts Resolved
Comment thread test/js/bun/css/css.test.ts Resolved
Pin the whitespace and the comment that a raw condition keeps after a
nested block. Compare the whole minified bundle text in the bundler
test.

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

LGTM — the one-line deletion brings expect_no_error_token in line with the upstream cssparser loop, and the latest push addresses both points from my earlier review (the bundler test now pins the exact minified output, and the trailing-whitespace/comment behavior after a nested block is pinned by dedicated cases). The remaining inline comment is a style nit only.

Extended reasoning...

The diff removes a single early return Ok(()) in Parser::expect_no_error_token in src/css/css_parser.rs so the scan continues past the first nested block, which matches rust-cssparser's reference implementation of the same method. All three callers live in src/css/rules/supports.rs (declaration form, selector() arm, unknown fallback) and each is exercised by the new describe("supports") tests, including error tokens after a block, junk after the condition, and a Bun.build round-trip with exact output. No security-sensitive surface is touched; the parser already bounded nesting via parse_nested_block, and that depth behavior is unchanged. Neither changed file is covered by CODEOWNERS, the hunt ran dry, and the only surviving finding is a comment-length nit.

Comment thread test/js/bun/css/css.test.ts Resolved

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

LGTM — the latest pushes address everything raised earlier (exact toBe on the bundled output, pinned trailing-whitespace/comment behavior, trimmed test comments), and the one-line parser change is correct.

What was reviewed:

  • expect_no_error_token in src/css/css_parser.rs now scans to end of input, matching rust-cssparser's loop; the removed return only ever cut the scan short, nothing positioned the parser on it.
  • All three callers in src/css/rules/supports.rs run inside parse_nested_block, so the slice taken after the scan still ends at the block's closing paren and error tokens after a nested block are still rejected (covered by the ERROR: cases).
  • The new tests cover every raw-condition shape (declaration, selector(), unknown fallback, @ import ... supports()), and/or combinations, trailing rules, and a Bun.build round-trip via tempDir with using.
Extended reasoning...

The diff deletes one early return Ok(()) from Parser::expect_no_error_token in src/css/css_parser.rs and adds a describe("supports") block of minify round-trips, error cases and one Bun.build test to test/js/bun/css/css.test.ts. It touches CSS parsing of user stylesheets but no auth, crypto, or filesystem-path surface; the only behavior change beyond the fix is that trailing whitespace/comments inside selector() and @ import supports() conditions are now preserved, which the tests pin. The change matches the upstream cssparser implementation, all three callers were checked for reliance on the old stop point, prior review feedback was addressed in commits 8e83988 and e76ced6, the files are not covered by CODEOWNERS, and the bug hunt ran dry with no findings.

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

Landed in #47025 (7942008).

Keep scanning an @supports condition after a nested block. 9 real stylesheets

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