Repository navigation
Conversation
WalkthroughWhen the canonicalizer rebuilds an arbitrary variant with a Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Mixed-case legacy pseudo-element selectors can still canonicalize incorrectly. This is a narrow edge case, so the PR is mergeable with bounded follow-up awareness. 🚥 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 |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| replaceObject(variant, { | ||
| kind: 'arbitrary', | ||
| selector: target.value, | ||
| selector: target.value.startsWith('::') ? `&${target.value}` : target.value, |
There was a problem hiding this comment.
The startsWith('::') check misses valid single-colon spellings such as :before, :after, :first-letter, and :first-line. For example, [&_:before]:flex still becomes **:[:before]:flex. Parsing that class wraps :before in &:is(:before). Pseudo-elements cannot appear inside :is(), so the browser ignores the rule and those styles disappear.
Preserve & for these legacy spellings too, and add child and descendant regression cases.
Knowledge Base Used: Utility and variant compilation
There was a problem hiding this comment.
I added :before, :after, :first-line and :first-letter to the same check in f465703, with tests for [&_:before] and [&_>_:before].
![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: Advanced
- Run ID:
0c394f07-befc-4ce4-aad1-82f3c1ec9537
📒 Files selected for processing (2)
packages/tailwindcss/src/canonicalize-candidates.test.tspackages/tailwindcss/src/canonicalize-candidates.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| selector: target.value, | ||
| selector: | ||
| target.value.startsWith('::') || | ||
| [':before', ':after', ':first-line', ':first-letter'].includes(target.value) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
git diff --unified=30 fa81d697fe572a10ac150d18964a093a7a874081 f465703fa74c1dba1025fd3bbad5cd059c8f2f6e -- packages/tailwindcss/src/canonicalize-candidates.ts
printf '\\n--- relevant source ---\\n'
nl -ba packages/tailwindcss/src/canonicalize-candidates.ts | sed -n '2020,2140p'
printf '\\n--- parser bindings and definitions ---\\n'
rg -n -F -- 'SelectorParser' packages/tailwindcss/srcRepository: tailwindlabs/tailwindcss
Length of output: 12300
🏁 Script executed:
printf '%s\\n' '--- selector parser declarations ---'
rg -n 'export function parse|function parse|case .:|kind: .selector.|value:' packages/tailwindcss/src/selector-parser.ts
printf '%s\\n' '--- parser implementation ---'
nl -ba packages/tailwindcss/src/selector-parser.ts | sed -n '1,240p'
printf '%s\\n' '--- canonicalization setup and target binding ---'
nl -ba packages/tailwindcss/src/canonicalize-candidates.ts | sed -n '1780,1835p'
nl -ba packages/tailwindcss/src/canonicalize-candidates.ts | sed -n '1980,2035p'Repository: tailwindlabs/tailwindcss
Length of output: 12088
🏁 Script executed:
nl -ba packages/tailwindcss/src/selector-parser.ts | sed -n '232,520p'
printf '%s\\n' '--- variant selector initialization and canonicalization callers ---'
rg -n -F -- 'modernizeArbitraryValuesVariant' packages/tailwindcss/src/canonicalize-candidates.ts
rg -n -F -- 'selector: ' packages/tailwindcss/src/candidate.ts packages/tailwindcss/src/variants.tsRepository: tailwindlabs/tailwindcss
Length of output: 9888
🏁 Script executed:
printf '%s\\n' '--- selector parser completion ---'
nl -ba packages/tailwindcss/src/selector-parser.ts | sed -n '490,680p'
printf '%s\\n' '--- arbitrary variant parsing ---'
rg -n 'kind: .arbitrary.|relative:|arbitrary variant|decode.*underscore|replaceAll.*_' packages/tailwindcss/src/candidate.ts packages/tailwindcss/src
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '240,300p'
printf '%s\\n' '--- selector filtering before target selection ---'
nl -ba packages/tailwindcss/src/canonicalize-candidates.ts | sed -n '1920,2005p'Repository: tailwindlabs/tailwindcss
Length of output: 19985
🏁 Script executed:
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '985,1065p'
nl -ba packages/tailwindcss/src/utils/decode-arbitrary-value.ts | sed -n '1,100p'
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '1170,1210p'Repository: tailwindlabs/tailwindcss
Length of output: 7692
🏁 Script executed:
rg -n 'decodeArbitraryValue\\(|variant\\.selector|parseVariant\\(' packages/tailwindcss/src/candidate.ts packages/tailwindcss/src/variants.ts
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '400,460p'
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '560,720p'
nl -ba packages/tailwindcss/src/candidate.ts | sed -n '760,825p'Repository: tailwindlabs/tailwindcss
Length of output: 11660
🏁 Script executed:
rg -n 'kind === .arbitrary.|case .arbitrary.|selector.*relative|variant\\.selector|selector: variant' packages/tailwindcss/src/variants.ts packages/tailwindcss/src/compile.ts packages/tailwindcss/src/canonicalize-candidates.ts
nl -ba packages/tailwindcss/src/variants.ts | sed -n '480,570p'
nl -ba packages/tailwindcss/src/compile.ts | sed -n '150,215p'Repository: tailwindlabs/tailwindcss
Length of output: 9321
🌐 Web query:
site:w3.org/TR/selectors pseudo-elements ASCII case-insensitive names CSS
💡 Result:
Yes. **Selectors Level 4** says pseudo-element names are ASCII case-insensitive, so `::before` and `::BEFORE` are equivalent. ([w3.org](https://www.w3.org/TR/selectors/?utm_source=openai))
Citations:
- 1: https://www.w3.org/TR/selectors/?utm_source=openai
🏁 Script executed:
nl -ba packages/tailwindcss/src/canonicalize-candidates.ts | sed -n '1755,1780p'Repository: tailwindlabs/tailwindcss
Length of output: 1116
Match legacy pseudo-element names without case sensitivity.
CSS treats legacy pseudo-element names as ASCII case-insensitive. For [&_:BEFORE]:flex, the parser preserves :BEFORE, so the exact lowercase match misses it and the fallback stores the selector without &. Normalize the name before matching and add a mixed-case regression test.
🐛 Suggested fix
- [':before', ':after', ':first-line', ':first-letter'].includes(target.value)
+ [':before', ':after', ':first-line', ':first-letter'].includes(target.value.toLowerCase())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [':before', ':after', ':first-line', ':first-letter'].includes(target.value) | |
| [':before', ':after', ':first-line', ':first-letter'].includes(target.value.toLowerCase()) |
1 participant
Summary
Fixes #20549.
[&_::before]:flexwas canonicalized to**:[::before]:flex. An arbitrary variant without&compiles to&:is(…), and pseudo-elements can't go inside:is(), so the canonical class generated an invalid selector.When the
*/**fallback moves a pseudo-element (anything starting with::, plus the legacy:before,:after,:first-lineand:first-letter) into its own arbitrary variant, it now keeps the&, so[&_::before]:flexbecomes**:[&::before]:flexand[&>::before]:flexbecomes*:[&::before]:flex. Pseudo-classes still canonicalize to**:[:hover]as before.Test plan
Added
[&_::before]:flex,[&_>_::before]:flex,[&_:before]:flexand[&_>_:before]:flexcases next to the existing:--customones incanonicalize-candidates.test.ts. They fail onmain(**:[::before]:flex) and pass with this change.