Do not migrate a variant literal behind a conditional - #20436

Walkthrough

The codemod now matches one-level object literals inside conditional variant expressions for brace-wrapped, quoted Vue, and object-form syntax. Tests cover invalid variant conditions, including function calls, object literals, computed members, parenthesized branches, nullish coalescing, and comparisons. Tests also confirm migration of conditional classes in JSX, component calls, and Vue :class attributes.

Merge Risk: 🔵 Low · up to 75e4e

The PR fixes conditional variant handling, but a bounded correctness risk remains for valid object-form expressions containing braces inside quoted strings, where the upgrade may still rewrite a component variant incorrectly. The change is otherwise localized and mergeable with explicit owner awareness or follow-up.

• Fix all pre-merge checks with AI

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

Confidence Score: 4/5

The PR should not merge until conditional variant values containing deeper nested object literals are protected from class migration.

The new object-group pattern handles the reported flat object literal but cannot consume deeper nested braces, allowing the same variant-value rewrite to remain reachable.

Files Needing Attention: packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts

_{Reviews (5): Last reviewed commit: "let a condition contain an object litera..." | Re-trigger Greptile}

thanks, two of the three were real and are now fixed in 893ab46a.

a call in the condition. variant={isActive(foo, bar) ? "outline" : "ghost"} was not guarded. my [^{},] ended the match at the comma inside the call. a parenthesised group is now consumed as a unit so its own commas stay inside it.

a parenthesised branch. variant={cond ? ("outline") : "ghost"} was not guarded either, because the ( sat between the operator and the literal. parens are now allowed there. covered in both arms.

the guard is now

/variant\s*[:=]\s*\{?(?:(?:[^{},()]|\([^()]*\))*?(?:\?\?|\?|:)\s*)?\(*\s*['"`]$/

a bare comma or brace still ends the match, which is what keeps a neighbouring className out of reach. Button({ variant: a ? "ghost" : "x", className: b ? "shadow" : "y" }) still migrates shadow, and there is a test for it.

on the brace comment. i think that one is a misread. [^{},] is a negated class over {, } and ,, so both braces were already excluded, not just the opening one. the current class [^{},()] excludes both parens as well.

on multiline values. correct, and it is not something this change introduces. currentLineBeforeCandidate is built by walking back until \n, so every entry in CONDITIONAL_TEMPLATE_SYNTAX is line scoped, not just this one. making the guards span lines would change behaviour for all of them, so it felt like the wrong thing to fold into a targeted fix. happy to open a separate issue for it if you want that tracked.

tests are now 78 in is-safe-migration and 425 across @tailwindcss-upgrade, up from 68 and 415 on main. the three new negatives fail without the change.

both of these were right, fixed in 3730f8a5.

the vue one is the important one, and it was a regression i introduced. <div variant="ghost" :class="active ? 'shadow' : 'none'"> was being guarded, so a class that should migrate no longer did. that is worse than the bug this pr set out to fix. vue writes attributes without braces, so my [^{},()] gap crossed "ghost" and walked straight on into :class.

the rule i had wrong is when a quote may be crossed. inside a brace a quoted ternary branch is ordinary, so variant={first ? "default" : "outline"} has to cross "default". outside a brace a quote closes the attribute value, so crossing one means leaving the attribute. the guard now splits on exactly that:

\{(?:[^{}]|PAREN)*?(?:\?\?|\?|:)\s*     braced, quotes may be crossed, braces may not
(?:[^{},()'"`]|PAREN)*?(?:\?\?|\?|:)\s* bare, a quote ends it
\{                                       the plain variant={"..."} form

nested parens. variant={isActive(getState(foo, bar), x) ? "outline"} now matches too, one level of nesting.

i pulled the pattern out into named parts above the array rather than growing the inline literal. three reviewers have now had to reason about this regex and the one liner was getting hard to read, so the parts are named and commented.

two new tests, one in each direction, and both fail on the previous commit:

• <Button variant={isActive(getState(foo, bar), x) ? "outline" : "ghost"} /> must not migrate
• <div variant="ghost" :class="active ? 'shadow' : 'none'"> must still migrate

is-safe-migration is 80 tests, @tailwindcss-upgrade is 427 across 28 files, up from 68 and 415 on main. prettier clean.

thanks for the catch, the vue case is one i should have tested for myself given the whole point of the guard is not to suppress real classes.

all three were right. fixed in 99d920d2, and i changed the shape rather than patch the pattern again.

three rounds of review have now found holes in this guard, and looking at why, they were all the same underlying mistake: one regex trying to know where a value ends in three different syntaxes at once. every time i widened it for one syntax it either missed another or reached into a neighbouring attribute. so it is now one rule per form, each stopping at what actually ends that form. the array already tests each rule independently so nothing else had to change.

variant\s*[:=]\s*\{?['"`]$                                      the plain forms
variant\s*[:=]\s*\{(?:[^{}]|PAREN)*?TAIL                        jsx, a brace ends it
variant\s*=\s*(["'])(?:(?!\1)[^{}])*?TAIL                       attribute, its own quote ends it
variant\s*:\s*(?:[^{},]|PAREN)*?TAIL                            object, a comma or brace ends it

that covers the three you found:

• :variant="active ? 'ghost' : 'outline'" — the attribute rule. the backreference is what keeps it inside its own value, which is also what stops the :class over match from the last round
• variant: (value ?? "outline") — the object rule, an unbalanced opening paren is fine there now
• variant: theme === "dark" ? "outline" : "ghost" — the object rule too. quotes are ordinary inside an object value, a comma or brace is what ends it

three more negative tests, all failing on the previous commit. is-safe-migration is 83, @tailwindcss-upgrade is 430 across 28 files, against 68 and 415 on main. prettier clean.

happy to keep going if there is another hole, but if the maintainers would rather this whole heuristic move off regexes and onto a real parse of the surrounding expression, that is a bigger change than i should make unasked and i would open a separate issue for it instead.

both fixed in 75e4e2f6. an object literal is now allowed as a unit inside a condition, the same way a parenthesised group already was, so its braces do not read as the end of the value.

• :variant="isActive({ x: 1 }) ? 'ghost' : 'outline'"
• <Button variant={{ tone: "dark" }.tone === "dark" ? "outline" : "ghost"} />

both are covered by tests and both fail on the previous commit. is-safe-migration is 85, @tailwindcss-upgrade is 432 across 28 files, against 68 and 415 on main.

where i think this actually stands

this is the fifth round, and i want to be straight about the trend rather than keep quietly widening things.

@coderabbitai is right that the real answer is scanning the expression with balanced brace, paren and quote state instead of matching it. a regex can only ever handle a bounded nesting depth. { a: { b: { c: 1 } } } inside a condition would still defeat what i just pushed, and i am not going to pretend otherwise by adding a third nesting level.

what i am less sure about is whether that scanner belongs in this pull request. every entry in CONDITIONAL_TEMPLATE_SYNTAX is a line scoped regex, and the whole array shares the same limits, so replacing the approach for one entry would leave the file half in each world and would be a much larger diff than the bug being fixed here warrants.

so my suggestion, and it is only a suggestion since this is your file:

• take this as is if the four reported forms plus the eleven the reviews found are worth fixing now. it is strictly better than main and it cannot suppress a real class, which is the failure mode i care most about and which has a test in both directions
• or tell me you want the scanner and i will do it properly as a replacement for the array rather than bolted next to it

either is fine by me. i would rather ask than keep adding nesting levels until someone finds level four.

#!/bin/bash
set -euo pipefail

implementation=$(fd -a '^is-safe-migration\.ts$' packages)
tests=$(fd -a '^is-safe-migration\.test\.ts$' packages)

printf '%s\n' '=== Implementation outline ==='
ast-grep outline "$implementation" --items all --type variable,constant,function

printf '%s\n' '=== Conditional guard implementation ==='
rg -n -C 12 'OBJECT_GROUP|PAREN_GROUP|CONDITIONAL_TEMPLATE_SYNTAX|variant' "$implementation"

printf '%s\n' '=== Related tests ==='
rg -n -C 6 'isActive\(\{ x: 1 \}\)|tone: "dark"|variant=.*\?' "$tests"

printf '%s\n' '=== Commit availability and changed files ==='
git show --no-ext-diff --format=fuller --stat 75e4e2f6 --
git diff --no-ext-diff 75e4e2f6^ 75e4e2f6 -- \
  "$implementation" "$tests"

@HuzaifaChaudary, this is sufficient for this pull request.

The three syntax-specific rules use the correct value boundaries. The two new tests cover the reported object-literal cases. The existing valid-position test confirms that a neighboring Vue :class value still migrates.

A stateful scanner would remove the bounded-nesting limit. That is a separate refactor. It should replace the broader conditional-syntax mechanism in a dedicated change, not extend this targeted fix.

No additional blocking concern from my review.

Learnt from: HuzaifaChaudary
URL: https://github.com/tailwindlabs/tailwindcss/pull/20436

Timestamp: 2026-08-28T00:27:27.579Z
Learning: In `packages/tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts`, `CONDITIONAL_TEMPLATE_SYNTAX` is intentionally line-scoped. Its `variant` rules use bounded regex groups for parenthesized and object-literal expressions. A balanced-expression scanner would be a separate, broader refactor and should replace the conditional-syntax mechanism rather than be added only for one `variant` rule.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

_{You are interacting with an AI system.}

both of these are real, and both are the ceiling i described in the last comment rather than new defects. a brace inside a string and an object nested past one level are the same structural limit: a regex cannot track balanced state, so there is always a next case. i said i would not keep adding levels and i am going to hold to that.

what i did do is measure which direction the remaining gaps fail in, because that is the part that decides whether this is mergeable as it stands.

they only fail by under guarding. for both of your cases the guard does not fire, so the prop is rewritten as a class, which is exactly what main does today for every conditional form. no case gets worse than it is now.

<Button variant={{ tone: "}" }.tone === "}" ? "outline" : "ghost"} />   not guarded
<Button variant={{ a: { b: { c: 1 } } }.a ? "outline" : "ghost"} />     not guarded

nothing over guards. the failure i actually care about is a real class silently no longer migrating, since that is worse than the bug this fixes. i checked that direction explicitly, including with your new shapes:

<div variant="ghost" :class="active ? 'shadow' : 'none'">     still migrates
<Button variant="ghost" className={active ? "shadow" : "x"}/> still migrates
Button({ variant: a ? "g" : "x", className: b ? "shadow" : "y" }) still migrates
<div :class="isActive({ x: 1 }) ? 'shadow' : 'none'">          still migrates
<div className={{ a: "}" }.a ? "shadow" : "x"}>                still migrates

so the state of this pr is: fixes the four forms from #20435 plus eleven more the reviews surfaced, cannot suppress a class that migrates today, and leaves a shrinking tail of exotic shapes behaving exactly as they already do on main.

that seems like a reasonable place to stop to me, but it is your call and i am happy either way:

• merge as is and let the tail stay a known limit
• or say the word and i will replace these four rules with a small backward scanner that tracks brace, paren and quote state properly. i would keep it to the variant rules and leave the other entries in the array alone, so it stays a contained change rather than a rewrite of the heuristic

i would rather do that once, deliberately, than keep going a level at a time.