Canonicalization: convert variants with arbitrary values to equivalent variants - #20380
Conversation
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (2): Last reviewed commit: "update changelog" | Re-trigger Greptile |
|
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 (1)
WalkthroughVariant canonicalization now uses root-font-size-specific caches. It normalizes media and container query dimensions and indexes known functional variants. Arbitrary variants preserve existing canonical forms and migrate to equivalent breakpoint or container variants when appropriate. The upgrade codemod reads the updated cache structure and accepts multiple equivalent matches. Tests cover 🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/tailwindcss/src/canonicalize-candidates.ts (1)
1550-1557: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEquivalent-variant selection is unspecified and duplicated across both call sites. Both sites resolve a signature to a list of equivalent variant names and then take
foundVariants[0]. The list order comes from the iteration order ofdesignSystem.variants.entries()andgetCompletions(root)increatePreComputedVariantsCache. That order is an internal detail of the variant registry, so the canonical output is not pinned by any explicit rule.
packages/tailwindcss/src/canonicalize-candidates.ts#L1550-L1557: replace thefoundVariants[0]choice with a shared helper that applies an explicit preference rule, for example the shortest name with a lexicographic tie-break.packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-variants.ts#L37-L48: remove the duplicated guard-and-select block and call the same shared helper.packages/tailwindcss/src/canonicalize-candidates.test.ts (1)
964-1020: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
remoption explicit in bothtest.eachoption objects.The first block relies on the omitted-default
rem: 16path, while the second block only overridescollapseandlogicalToPhysical. Addrem: 16to the first options object andrem: undefined(or the equivalent no-conversion default) to the second so each expectation declares its own conversion precondition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f029e69d-9848-4e79-be97-140c6df18e69
📒 Files selected for processing (3)
packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-variants.tspackages/tailwindcss/src/canonicalize-candidates.test.tspackages/tailwindcss/src/canonicalize-candidates.ts
This PR improves the canonicalization for variants with arbitrary values that could be converted to static variants (based on the user
@theme).If a variant was used where we would use the 'old'
theme(…)function, then we did convert this to the more modern version:But in this case we can go a step further and convert to
max-lg:flexinstead. This is safe to do because the--breakpoint-lgcan't change at runtime because@mediaqueries don't work with CSS variables, which is why we inline it.We can extend this logic to arbitrary values in those variants as well. In fact, we already did for a
min-[64rem]:flexor[@media(width>=theme(screens.lg))]:flex, these both translated tolg:flex.That's because the
lg,md, … are static variants, whereas the others are functional variants (lg-[…]doesn't exist for example).With this PR, we will tackle functional variants, which now results as:
Fixes: #20365
Test plan