diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts index 5ab859b4f..0117cfa3e 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.test.ts @@ -132,6 +132,10 @@ describe('is-safe-migration', async () => { [``, 'outline'], [``, 'outline'], [``, 'outline'], + [``, 'outline'], + [``, 'outline'], + [`Button({ variant: { a: { b: 1 } }.a ? "outline" : "ghost" })`, 'outline'], + [``, 'outline'], ])('does not replace classes in invalid positions #%#', async (example, candidate) => { expect( await migrateCandidate(designSystem, {}, candidate, { @@ -165,6 +169,13 @@ describe('is-safe-migration', async () => { 'shadow-sm', ], [`
`, 'shadow', 'shadow-sm'], + [``, 'shadow', 'shadow-sm'], + [``, 'shadow', 'shadow-sm'], + [ + ``, + 'shadow', + 'shadow-sm', + ], ])('replaces classes in valid positions #%#', async (example, candidate, expected) => { expect( await migrateCandidate(designSystem, {}, candidate, { diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts index fa7d69d57..21b24c190 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/is-safe-migration.ts @@ -4,16 +4,6 @@ import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map' import * as version from '../../utils/version' const LOGICAL_OPERATORS = ['&&', '||', '?', '===', '==', '!=', '!==', '>', '>=', '<', '<='] - -// A parenthesised group with one level of nesting, so a call in a condition keeps -// its own commas instead of ending the match early. -const PAREN_GROUP = String.raw`\((?:[^()]|\([^()]*\))*\)` -// An object literal with one level of nesting, so a condition may contain one -// without the surrounding value looking like it ended there. -const OBJECT_GROUP = String.raw`\{(?:[^{}]|\{[^{}]*\})*\}` -// A ternary or nullish operator, then whatever sits before the string literal. -const CONDITION_TAIL = String.raw`(?:\?\?|\?|:)\s*\(*\s*['"\`]$` - const CONDITIONAL_TEMPLATE_SYNTAX = [ // Skip any generic attributes like `xxx="shadow"`, // including Vue conditions like `v-if="something && shadow"` @@ -27,29 +17,8 @@ const CONDITIONAL_TEMPLATE_SYNTAX = [ // Alpine /wire:[^\s]*?$/, - // shadcn/ui variants. A conditional can sit between the prop and the literal, - // so each form below stops at whatever actually ends its own value. Getting - // that boundary wrong in either direction is costly: too narrow and a prop is - // rewritten as a class, too wide and a real class stops being migrated. - - // `variant="outline"`, `variant={"outline"}`, `variant: "outline"` + // shadcn/ui variants /variant\s*[:=]\s*\{?['"`]$/, - - // `variant={cond ? "outline" : "ghost"}` — a brace ends the value, and a - // quoted branch inside it is ordinary - new RegExp( - String.raw`variant\s*[:=]\s*\{(?:[^{}]|${PAREN_GROUP}|${OBJECT_GROUP})*?${CONDITION_TAIL}`, - ), - - // `:variant="active ? 'outline' : 'ghost'"` — the opening quote ends the value, - // so the match cannot run on into a neighbouring attribute such as `:class` - new RegExp(String.raw`variant\s*=\s*(["'])(?:(?!\1)[^{}]|${OBJECT_GROUP})*?${CONDITION_TAIL}`), - - // `{ variant: theme === "dark" ? "outline" : "ghost" }` — a comma or brace ends - // the value, quotes do not - new RegExp( - String.raw`variant\s*:\s*(?:[^{},]|${PAREN_GROUP}|${OBJECT_GROUP})*?${CONDITION_TAIL}`, - ), ] const NEXT_PLACEHOLDER_PROP = /placeholder=\{?['"`]$/ const VUE_3_EMIT = /\b\$?emit\(['"`]$/ @@ -220,6 +189,14 @@ export function isSafeMigration( } } + // Heuristic: Disallow anything inside a shadcn/ui `variant` prop, because a + // conditional can sit between the prop and the string + // + // E.g.: `` + if (isInsideVariantValue(currentLineBeforeCandidate)) { + return false + } + // Heuristic: Disallow Next.js Image `placeholder` prop if (NEXT_PLACEHOLDER_PROP.test(currentLineBeforeCandidate)) { return false @@ -233,6 +210,61 @@ export function isSafeMigration( return true } +const VARIANT_PROP = /variant\s*([:=])\s*/g +const CALLEE = /[\w$.)\]]/ + +// The candidate is the last thing on the line, so the `variant` value it sits in +// is the one that has not ended yet. Each form ends somewhere different: +// `variant={…}` at its matching brace, `variant="…"` at its own quote, and +// `{ variant: … }` at a comma or at the brace around it. +function isInsideVariantValue(line: string): boolean { + for (let match of line.matchAll(VARIANT_PROP)) { + // The prop itself is never inside a string, so a `variant:` that is only + // mentioned in some other attribute value does not count + if (isMiddleOfString(line.slice(0, match.index))) continue + + let start = match.index + match[0].length + let char = line[start] + + if (match[1] === ':') { + if (isOpenValue(line, start)) return true + } else if (char === '{') { + if (isOpenValue(line, start + 1)) return true + } else if (char === '"' || char === "'" || char === '`') { + if (!line.includes(char, start + 1)) return true + } + } + + return false +} + +function isOpenValue(line: string, start: number): boolean { + // Whether each open group is the argument list of a call, because an argument + // is passed to that call and is not the value itself + let groups: boolean[] = [] + let quote: string | null = null + + for (let i = start; i < line.length; i++) { + let char = line[i] + + if (quote !== null) { + if (char === '\\') i++ + else if (char === quote) quote = null + } else if (char === '"' || char === "'" || char === '`') { + quote = char + } else if (char === '(' || char === '[' || char === '{') { + groups.push(char === '(' && CALLEE.test(line[i - 1] ?? '')) + } else if (char === ')' || char === ']' || char === '}') { + if (groups.length === 0) return false + groups.pop() + } else if (char === ',' && groups.length === 0) { + return false + } + } + + return !groups.includes(true) +} + // Assumptions: // - All ``