Repository navigation
Conversation
|
Updated 2:06 AM PT - Oct 29th, 2026
❌ @robobun, your commit 60b701d has some failures in 🧪 To try this PR locally: bunx bun-pr 47288That installs a local version of the PR into your bun-47288 --bun |
|
Status: ready for review. Reproduced on a release build of main (11c3671) with CI on 60b701d: the GitHub checks pass. In Buildkite build #134917, 184 of 188 jobs passed and no test failed. The other 4 jobs are the macOS x64 test jobs ( |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes.
 |
| Layer / File(s) | Summary |
|---|---|
Nesting selector parsing src/css/css_parser.rs, src/css/selectors/parser.rs, test/js/bun/css/css.test.ts |
Parser input tracks nesting selectors. Selector and pseudo-class parsing pass state by value, and selector-list parsing restores parser flags after each attempt. Tests cover nesting in pseudo-classes, forgiving selector lists, and @nest. |
CSS Modules selector serialization src/css/selectors/selector.rs, test/bundler/css/css-modules.test.ts |
Pseudo-class serialization passes the current nesting position into :local() and :global() arguments. Tests check selector output in nested rules, at-rules, selector lists, and CLI builds. |
Suggested reviewers: jarred-sumner, alii
Priority:
Merge Risk: 
4db48
-
The changes address explicit nesting detection and CSS Modules selector serialization. No concrete merge-blocking issue is established; mergeable subject to normal checks.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change: counting &per selector so nesting inside:global()and:local()uses the rule's explicit nesting selector.Description check The description is detailed and covers the problem, cause, fix, behavior changes, verification results, tests, and known limitations. It does not use the exact template headings, but it provides the r… 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 is detailed and covers the problem, cause, fix, behavior changes, verification results, tests, and known limitations. It does not use the exact template headings, but it provides the required information in equivalent sections.
-
- 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 the counter covers every & consumer the deleted AFTER_NESTING propagation did: the only two sites that consume a & delimiter (parse_compound_selector and the & arm of parse_one_simple_selector) both call note_nesting_selector, so :is(), :not(), :has(), :nth-child(of), ::slotted(), and the :global()/:local() wrappers all count through the one path. A & inside a vendor or unknown functional pseudo-class (:-webkit-foo(&)) is parsed by parse_non_ts_functional_pseudo_class as a raw token list, never as a Nesting component, so its behavior is unchanged from the base.
Extended reasoning...
The change touches the CSS selector parser (src/css/selectors/parser.rs, src/css/css_parser.rs) and selector printer (src/css/selectors/selector.rs), replacing the AFTER_NESTING bitflag with a per-ParserInput & counter and printing :global()/:local() arguments at the wrapper's position; it touches no security-sensitive surface. Three inline findings were posted, so a human look is already signalled; this note only records the sibling-site sweep that was ruled out.
| PseudoClass::Local { selector } => { | ||
| serialize_selector_impl(selector, dest, context, false, position)? | ||
| } | ||
| PseudoClass::Global { selector } => { |
There was a problem hiding this comment.
🟡 (optional) CSS-module users whose nested rule is a :global()/:local() holding a & get a flat, differently-wrong selector for a child & that does not lead; the author's test.todo pins this. fits_at (selector.rs:635) calls is_simple and has_type_selector on the parent, which see one NonTsPseudoClass component and never look inside the wrapper, so the parent is pasted as written at MidCompound. Fix: make fits_at (and so the :is() unwrap at selector.rs:981 and serialize_nesting) look through Local/Global wrappers for combinators and type selectors, so .a { :global(.dark &) { .y& {} } } prints .y_H:is(.dark .a_H) and .a { :global(span &) { .y& {} } } does not print .y_Hspan .a_H. [also at: src/css/selectors/parser.rs:662 - CSS-module users get a selector with a different meaning when a & that does not lead sits under a rule whose selector is :global(<x> <combinator> &).; src/css/selectors/selector.rs:1280 - CSS-module users whose parent rule is a :global()/:local() wrapper holding a & and a combinator now get a child & substituted inline, producing a selector that matches the wrong elements.]
Why this was flagged
Input .a { :global(.dark &) { .y& { color: red } } } in a .module.css. After the first commit the parent rule's selector is the single component :global(.dark &) with no implicit &, so when the child's & is printed at NestingPosition::MidCompound, fits_at at src/css/selectors/selector.rs:635-640 asks is_simple (selector.rs:1563) and has_type_selector (selector.rs:1529), both of which only walk the outer component list and report no combinator and no type selector. The parent is then written as-is through PseudoClass::Global at selector.rs:1273-1278, giving .y_H.dark .a_H, which matches an element with both classes y_H and dark under .a_H instead of .dark .a_H.y_H. The base printed .y_H:is(.a_H .dark .a_H) because the implicit & gave the parent a combinator; that was also wrong but at least kept the parent inside :is(). The PR's test.todo at test/bundler/css/css-modules.test.ts:909 documents the shape but no code change addresses it, and no error or warning is emitted.
Verification: The test.todo at test/bundler/css/css-modules.test.ts documents the shape. fits_at (selector.rs:635-640) for MidCompound is !has_type_selector(selector) && is_simple(selector); the single NonTsPseudoClass passes both, so the parent is printed as-is through the Global arm (selector.rs:1276-1278), yielding .y_H.dark .a_H instead of .y_H:is(.dark .a_H). On the base this same input is also wrong.
There was a problem hiding this comment.
Confirmed. The PR body lists it under "Other behaviour changes in modules", and the test.todo pins the right form. I keep the fix out of this PR, for two reasons.
- The cause is older than this PR.
is_simpleandhas_type_selectorread a wrapper as one simple selector on main too::global(.a .b) { .q& {} }prints.q_<hash>.a .bthere, and:global(div) { .x& {} }prints.x_<hash>div. On main the implied&hid this for a nested wrapper rule, because it gave the rule a combinator. - The two predicates have more readers than
fits_at.should_unwrap_isalso decides the:-webkit-any()copy of a:is()(selector.rs:166) and the target check inis_compatible(selector.rs:402). A change there needs its own tests and its own comparison of output.
No real module has the shape. In the 31 files that write & inside a wrapper, 17 wrapper rules hold 30 nested rules, and none of those has a & after a combinator or after another simple selector. The common children are right now: &:hover and .label under .a { :global(.dark &) {} } print .dark .a_<hash>:hover and .dark .a_<hash> .label_<hash>.
If a maintainer wants the look-through in this PR, I add it as a third commit.
| let css_module = dest.css_module.take(); | ||
| serialize_selector(selector, dest, context, false)?; | ||
| serialize_selector_impl(selector, dest, context, false, position)?; | ||
| dest.css_module = css_module; |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing: with bun build --no-bundle, a CSS module rule .button { :global(&.primary) {} } prints .button.primary, so the rule can match no hashed element; main printed .button_<hash> .button.primary, also dead. The Global arm at selector.rs:1277 clears dest.css_module for the whole argument, and the parent substituted for & is printed through the same dest, so the parent class loses its hash. Fix: restore css_module around the parent print inside serialize_nesting (or scope the take to the non-& components), covering :global(... &) at any position, and add the :global(&...) --no-bundle case beside the :local(&.on) CLI test, which currently avoids this shape. [also at: src/css/selectors/selector.rs:1280 - pre-existing: with bun build --no-bundle, a CSS module rule .button { :global(&.primary) {} } prints .button.primary, so the rule never matches the hashed element.]
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
A user runs bun build --no-bundle styles.module.css on .button { :global(&.primary) { color: red } }. The Global arm at src/css/selectors/selector.rs:1277 does dest.css_module.take() before serializing the argument and restores it at 1279. Inside the argument, the & reaches serialize_nesting (selector.rs:1465), which prints the parent .button via serialize_selector_impl on the same dest while css_module is None, so .button is written unhashed. The output is .button.primary while the module's export maps button to button_<hash>, so the rule targets nothing. The base branch printed .button_<hash> .button.primary for the same input, which is equally dead, so this is pre-existing in a line the diff rewrites. The new CLI test in test/bundler/css/css-modules.test.ts only exercises :local(&.on) under --no-bundle, where the Local arm at 1273 keeps css_module, so the defect is not covered by a test.
Verification: With bun build --no-bundle on .button { :global(&.primary) {} }, src/css/selectors/selector.rs:1276-1280 does dest.css_module.take() before serializing the argument, so the parent substituted for & by serialize_nesting (selector.rs:1482-1484) prints with css_module == None. The output is .button.primary; on the base the same arm produced .button_<hash> .button.primary, so both match no element. The only --no-bundle test (css-modules.test.ts:923-951) exercises :local(&.on).
There was a problem hiding this comment.
Confirmed, and the PR body lists it. Main prints .button_<hash> .button.primary and this branch prints .button.primary. Both match nothing.
I did not change it here. The fix is to replace the take() with a flag on the printer that only the names hashed at print time read (the view-transition names). #45487 removed the take() for that reason, and #47025 kept it when it took the scope bit, so I left that choice alone.
The CLI test uses :local(&.on) on purpose. It checks that the parse fix reaches --no-bundle, and it does not pin the wrong output of :global(&...). bun build --no-bundle styles.module.css prints only the stylesheet, with no map of the generated names.
…s the rule's `&`
A nested rule gets an implied `&` in front only when its selector holds no
`&`. The selector parser kept that fact in `SelectorParsingState::AFTER_NESTING`,
and each nested selector parse copied the bit back to its holder by hand.
`GenericSelector::parse`, which parses the argument of `:local()`, `:global()`,
`::cue()` and `::cue-region()`, did not. So `.button { :global(&.primary) {} }`
in a CSS module printed `.button_<hash> .button_<hash>.primary`.
`ParserInput` now counts each `&` that the selector parser reads, and
`parse_selector` adds the implied `&` only when the count did not move while
it ran. `AFTER_NESTING`, its five copies and the fold in
`parse_list_with_state` are gone. The nested selector parsers take the state
by value, because nothing flows back through it.
In a plain stylesheet, `video { ::cue(&) {} }` now prints `::cue(video)`. A `&`
that the parser reads in a selector that `:is()` or `:where()` then drops
counts too, also when it is inside a nested argument:
`.a { :is(:not(& $), .b) {} }` prints `.b`. An `@nest` selector whose only `&`
is in one of these places was an error and now builds.
…position of the wrapper
The printer started the argument of `:local()` and `:global()` as if nothing
came before it. A `&` that starts the argument then took the parent selector
as written, also when the wrapper follows a combinator or other simple
selectors: `.a .b { & :global(&.x) {} }` printed
`.a_<hash> .b_<hash> .a_<hash> .b_<hash>.x`, and `div { & .q:global(&) {} }`
printed `div .q_<hash>div`.
`serialize_pseudo_class` now gets the `NestingPosition` of the wrapper and
passes it to the argument, as `serialize_component` does for a `:is()` that it
unwraps. The two rules print `.a_<hash> .b_<hash> :is(.a_<hash> .b_<hash>).x`
and `div .q_<hash>:is(div)`.
4db48d7 to
60b701d
Compare
Follow-up to #47025: the
&part of #45487.Problem
.button { :global(&.primary) {} }builds to.button_<hash> .button_<hash>.primary, with exit 0. The&inside:global()or:local()is not seen, so the rule also gets the implied&.GenericSelector::parse(src/css/selectors/parser.rs:1812) dropsSelectorParsingState::AFTER_NESTING, which five other nested parsers copy back by hand. lightningcss 1.33 prints the same.Fix
ParserInputcounts each&the selector parser reads.parse_selectoradds the implied&only if the count did not move.AFTER_NESTINGand its copies are deleted.test/bundler/css/css-modules.test.ts(19 fail on main),test/js/bun/css/css.test.ts(6), and both directories. Self-reviewed: 12 concerns raised, 11 addressed (Notes).Background
.a { .b {} }means.a .b: a nested rule with no&gets an implied one.:global(x)and:local(x)print onlyx.::cue(&)stays wrong.Downsides
::cue(&), a&in a dropped:is()member,@nest. If unwanted, I use the hand copy..y&under a rule:global(.dark &)prints.y_<hash>.dark .a_<hash>. Main is wrong too.ParserInput: 200 to 208 bytes, one per parse.Notes
Origin and reach. A fuzzer found the
:global()bugs of #45487. #47025 landed the export half. No user reported this half. It was never right: 1.4.2 and a 1.4.3 canary print.button_<hash> .button.primary, also with exit 0. #47025 is in no release yet..module.cssfile of 305 public repositories (6,869 files), built with main and with this branch: 0 differ in CSS, JS, stderr or exit status.:global(&and:local(&in*.module.cssgives 43 files. Without forks and copies that is 31 distinct files in 12 projects: WordPress Playground, tapestry-project (11 files), near/bos-web-engine, oasisprotocol/rose-app, riccardoperra/better-comments-for-github and 7 small ones. 30 of the 31 build alone. On main all 30 print selectors that postcss-modules and esbuild do not print. Here 29 print the same selectors as both tools, and 1 differs only in how Bun lowers:not(a, b).:global :root.lightTheme :local(&). They were a build error before css: consolidate the open CSS parser fixes #47025 made the bare:globalbuild. On main they print.logo_<hash> :root.lightTheme .logo_<hash>, which can match no element.What main prints (
bun build entry.js --outdir out, default targets):styles.module.css.site-notice { :global(& .components-notice__content) {} }.site-notice_<hash> .site-notice_<hash> .components-notice__content.site-notice_<hash> .site-notice .components-notice__content.site-notice_<hash> .components-notice__content.button { :global(&.primary) {} }.button_<hash> .button_<hash>.primary.button_<hash> .button.primary.button_<hash>.primary.tip { :global(body.desktop *:not(:hover) > &) {} }.tip_<hash> body.desktop :not(:hover) > .tip_<hash>.tip_<hash> body.desktop :not(:hover) > .tipbody.desktop :not(:hover) > .tip_<hash>.nav { :global(.dark) :local(&) {} }.nav_<hash> .dark .nav_<hash>.dark .nav_<hash>The first row is from WordPress Playground.
The plain-CSS changes, and the question. The count has no copy per caller, so it also covers the other callers of
GenericSelector::parseand every nested argument. To keep main's output there needs more code, not less.video { ::cue(&) {} }video ::cue(video)::cue(video)&.p { ::cue-region(&) {} }.p ::cue-region(.p)::cue-region(.p)::cue-region.a { :is(:not(& $), .b) {} }.a .b.b&.a { @nest ::cue(&) {} }&::cue(.a)@nest.a { @nest :is(:not(& $), .b) {} }&.b@nest.p { @nest :global(.dark &) {} }&.dark .p_<hash>@nestReal stylesheets with these shapes: 0 found. A GitHub code search finds no
.cssfile with::cue(&or::cue-region(&. The:is()rows need an invalid selector, which a search cannot find. None of the 6,869 module files has one, because their output does not change.::cue(&)is commit d4d67c9 of css: resolve&inside :nth-child(... of S), :host(), ::slotted() and ::cue() #43024. css: consolidate the open CSS parser fixes #47025 took:nth-child(... of &)from that PR with a hand copy of the bit, and not that commit. This diff deletes that copy and brings::cue(&)back. css: consolidate the open CSS parser fixes #47025 also did not take a34dbf9, 6f3f701 and ad8b947 of css modules: decide local vs global at parse, fixing :global() exports and&inside :global() #45487, which are the two halves of this PR in another form.The Chrome reading of
::cue(&)holds for a parent with no combinator..a .b { ::cue(&) {} }prints::cue(.a .b), which Chrome drops. Main's.a .b ::cue(.a .b)is dropped too.A
&counts only if the parser reads it before it drops the member..a { :is($ &, .b) {} }still prints.a .b. Chrome prints no implied&there. With nesting kept, the dropped member is not printed, so.a { :is(:not(& $), .b) {} }prints.a { .b {} }.79 more shapes with an invalid member in
:is()/:where(), or with::cue(): 34 change. Chrome 153 agrees with main on the implied&in 37 of 77 and with this PR in 69 of 77. None moves away from Chrome.If these are not wanted: the other shape is a copy of the bit in the two wrapper arms (the
parse_innerof css modules: decide local vs global at parse, fixing :global() exports and&inside :global() #45487, +22 lines). It fixes the same modules, leaves plain CSS as on main, and keepsAFTER_NESTINGand the copies. Say so and I change the first commit.Other behaviour changes in modules. 0 real modules have these shapes: none of the 30 rules nested in the 17 wrapper rules of those files has a
&after a combinator or after another simple selector.A rule whose selector is a wrapper with a
&, used as a parent. On main the implied&gave it a combinator, so a nested&that does not lead got:is(). Nowis_simpleandhas_type_selectorsee one wrapper and the parent is written as it is. Onetest.todopins the right form..a { :global(.dark &) { .y& {} } }.y_<hash>:is(.a_<hash> .dark .a_<hash>).y_<hash>.dark .a_<hash>.dark .a_<hash>.y_<hash>.a { :global(span &) { .y& {} } }.y_<hash>:is(.a_<hash> span .a_<hash>).y_<hash>span .a_<hash>span .a_<hash>.y_<hash>.a { :global(& .t) { .y & {} } }.y_<hash> :is(.a_<hash> .a_<hash> .t).y_<hash> .a_<hash> .t.y_<hash> :is(.a_<hash> .t)The common children are right now:
&:hoverand.labelunder.a { :global(.dark &) {} }print.dark .a_<hash>:hoverand.dark .a_<hash> .label_<hash>. A top-level wrapper rule has the same defect on main::global(.a .b) { .q& {} }prints.q_<hash>.a .b.bun build --no-bundle: the parent behind a&inside:global(...)prints with its original name, because theGlobalprint arm takesdest.css_module..button { :global(&.primary) {} }printed.button_<hash> .button.primaryand prints.button.primary.:local(&.on)is right there:.button_<hash>.on_<hash>.Measurements. Release builds (
bun run build:release, same path) of main 11c3671 and of that commit with this diff.src/cssdid not change between 11c3671 and the base of this branch.perf_event_open, median of 3, on a bench binary ofbun_cssbuilt with the release flags.perfandvalgrindare not installed here.)serialize_pseudo_classhas no symbol of its own in either build.size_of:ParserInput200 B to 208 B.SelectorParser16 B,SelectorParsingState2 B,Component48 B,Printer496 B unchanged.size).parse_selector24,887 B to 23,952 B,serialize_component7,641 B to 7,643 B (nm -S).bun build, as a plain stylesheet and as a module: 2,067 of 2,067 CSS inputs oftest/js/bun/css, 38 of 38.cssfiles tracked in the repo and 45 of 45 css-loader test cases are identical.:global(),:local()and no wrapper), each withcolor: red. Chrome 153 checks on 399 elements whether the printed sheet styles what the nested postcss-modules output styles. Wrapper with a&: main 470 of 1,248, this PR 1,248 of 1,248. The 1,144 shapes with no&in a wrapper are byte-identical. 0 shapes change between build and error. 104 more shapes use&div, which Chrome rejects.src/+58/-74.Deleted.
SelectorParsingState::AFTER_NESTING, its five copies (parse_inner_compound_selector,parse_nth_pseudo_class,parse_is_or_where,parse_has,parse_negation) and thesaw_nestingfold inparse_list_with_state. The eleven selector parsers that took the state as&muttake it by value: the copy was the only write through it.Left out. Each of these is the same on main and here.
:global(> .x), a relative selector as the argument, is a build error (Empty selector is not allowed). Code search: 86 module files. So is:has(:global(> &)).:global { ... }block and a trailing.a :global {are build errors. Code search: 3,540 module files for:global {. css: consolidate the open CSS parser fixes #47025 pins the trailing form as an error.&in an argument that is kept as raw tokens is not counted::unknown-fn(&), and:global(&.x)in a plain stylesheet..title { :global(&::after) { inset-inline-start: 4px } }gets its:lang()guard after the pseudo-element. That rule matched nothing on main and the selector is still invalid.:host(),::slotted(),::cue()and::cue-region()print a&in their argument as the parent written out, also when the parent has a combinator. Browsers drop::cue(.a .b).&divinside a wrapper. css-nesting says to writediv&, and Chrome drops&div.Self-review. 12 concerns raised, 11 addressed. Rejected: a test that pins what
--no-bundleprints for:global(&...). That output is known to be wrong, so the test uses:local(&...)there, and the:globalcase is described above.Suites run on a debug build of this branch:
test/bundler/css/(237 pass, 1 todo),test/bundler/esbuild/css.test.ts(68 pass, 1 todo),test/js/bun/css/(3397 pass),test/internal/source-lints/(194 pass),cargo clippy -p bun_css. The first commit alone also passes its tests.