Don't space out and/or/not inside function calls in supports-[…] variants - #20420
Conversation
……]` variants The `supports-[…]` variant spaces out the `and`, `or`, and `not` keywords to work around a Chrome bug where `@supports (a)or(b)` is invalid. The replacement was applied to the entire value, including inside function calls where these words can be part of a selector: `supports-[selector(a:not(.foo))]` generated `@supports selector(a: not (.foo))`, an unparsable selector, so the condition silently never matched. Keywords are now only spaced out at the condition level; the contents of function calls like `selector(…)` are left as-is.
62f3b1e to
56e2878
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 456c12b3-5119-474d-83dd-28ed260d9e81
📒 Files selected for processing (3)
CHANGELOG.mdpackages/tailwindcss/src/variants.test.tspackages/tailwindcss/src/variants.ts
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the current parser and recursive traversal address all three previously reported issues. Reviews (6): Last reviewed commit: "use proper walk + bail when we see `sele..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…conditions
Tokenize identifiers whole so function names like `foo-not(…)` are not
mistaken for the `not` keyword, and skip quoted strings so parens
inside values like `selector([data-foo="("])` don't unbalance the
function-depth tracking.
…ions A lone escaped paren outside a string (e.g. a class named `.foo\(` in `selector(…)`) unbalanced the function-depth tracking, leaving a following top-level `or` unspaced.
RobinMalfait
left a comment
RobinMalfait
left a comment
There was a problem hiding this comment.
Thanks!
I updated the code to make use of an existing value parser and mutating the AST, instead of using a bunch of regexes.
|
Thank you @RobinMalfait for the review and the refactor to the value parser that approach is much cleaner than the regex tokenizer, and I learned a lot comparing the two. Happy to see both fixes land |
2 participants
Summary
The
supports-[…]variant works around a Chrome bug where@supports (a)or(b)is invalid by spacing out the
and,or, andnotkeywords. However, thereplacement is applied to the entire value, including the inside of function
calls, where these words can be part of a selector.
For example,
supports-[selector(a:not(.foo))]:flexgenerates:The selector
a: not (.foo)is unparsable, so a condition that is true inevery browser silently becomes false and the utility never applies. The same
happens to class names like
.andor.orinsideselector(…).This PR only spaces out the keywords at the condition level: parens preceded by
an identifier (other than the keywords themselves) start a function call, and
everything inside is left as-is. The Chrome workaround still applies to the
condition itself, e.g.
supports-[(display:grid)or(display:flex)]stillbecomes
@supports (display: grid) or (display: flex).Test plan
selector(a:not(.foo)), class names.and/.orinside
selector(…), the Chrome(a)or(b)workaround, and a top-levelnot(…)condition.pnpm vitest run packages/tailwindcss/src/variants.test.ts— 102 passed.