Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add theme(…) to var(…) migration
  • Loading branch information
RobinMalfait committed Oct 16, 2024
commit c903c87bfa6791db033e86d6069d630dc777620b
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
import { expect, test } from 'vitest'
import { themeToVar } from './theme-to-var'

test.each([
// Convert to `var(…)` if we can resolve the path
['[color:theme(colors.red.500)]', '[color:var(--color-red-500)]'], // Arbitrary property
['bg-[theme(colors.red.500)]', 'bg-[var(--color-red-500)]'], // Arbitrary value
['bg-[size:theme(spacing.4)]', 'bg-[size:var(--spacing-4)]'], // Arbitrary value + data type hint

// Convert to `var(…)` if we can resolve the path, but keep fallback values
['bg-[theme(colors.red.500,red)]', 'bg-[var(--color-red-500,_red)]'],

// Keep `theme(…)` if we can't resolve the path
['bg-[theme(colors.foo.1000)]', 'bg-[theme(colors.foo.1000)]'],

// Keep `theme(…)` if we can't resolve the path, but still try to convert the
// fallback value.
[
'bg-[theme(colors.foo.1000,theme(colors.red.500))]',
'bg-[theme(colors.foo.1000,var(--color-red-500))]',
],

// Use `theme(…)` (deeply nested) inside of a `calc(…)` function
['text-[calc(theme(fontSize.xs)*2)]', 'text-[calc(var(--font-size-xs)_*_2)]'],

// Multiple `theme(… / …)` calls should result in modern syntax of `theme(…)`
// - Can't convert to `var(…)` because that would lose the modifier.
// - Can't convert to a candidate modifier because there are multiple
// `theme(…)` calls.
//
// If we really want to, we can make a fancy migration that tries to move it
// to a candidate modifier _if_ all `theme(…)` calls use the same modifier.
[
'[color:theme(colors.red.500/50,theme(colors.blue.500/50))]',
'[color:theme(--color-red-500/50,_theme(--color-blue-500/50))]',
],
[
'[color:theme(colors.red.500/50,theme(colors.blue.500/50))]/50',
'[color:theme(--color-red-500/50,_theme(--color-blue-500/50))]/50',
],

// Convert the `theme(…)`, but try to move the inline modifier (e.g. `50%`),
// to a candidate modifier.
// Arbitrary property, with simple percentage modifier
['[color:theme(colors.red.500/75%)]', '[color:var(--color-red-500)]/75'],

// Arbitrary property, with numbers (0-1) without a unit
['[color:theme(colors.red.500/.12)]', '[color:var(--color-red-500)]/12'],
['[color:theme(colors.red.500/0.12)]', '[color:var(--color-red-500)]/12'],

// Arbitrary property, with more complex modifier (we only allow whole numbers
// as bare modifiers). Convert the complex numbers to arbitrary values instead.
['[color:theme(colors.red.500/12.34%)]', '[color:var(--color-red-500)]/[12.34%]'],
['[color:theme(colors.red.500/var(--opacity))]', '[color:var(--color-red-500)]/[var(--opacity)]'],
['[color:theme(colors.red.500/.12345)]', '[color:var(--color-red-500)]/[12.345]'],
['[color:theme(colors.red.500/50.25%)]', '[color:var(--color-red-500)]/[50.25%]'],

// Arbitrary value
['bg-[theme(colors.red.500/75%)]', 'bg-[var(--color-red-500)]/75'],
['bg-[theme(colors.red.500/12.34%)]', 'bg-[var(--color-red-500)]/[12.34%]'],

// Arbitrary property that already contains a modifier
['[color:theme(colors.red.500/50%)]/50', '[color:theme(--color-red-500/50%)]/50'],

// Arbitrary value, where the candidate already contains a modifier
// This should still migrate the `theme(…)` syntax to the modern syntax.
['bg-[theme(colors.red.500/50%)]/50', 'bg-[theme(--color-red-500/50%)]/50'],

// Variants, we can't use `var(…)` especially inside of `@media(…)`. We can
// still upgrade the `theme(…)` to the modern syntax.
['max-[theme(spacing.4)]:flex', 'max-[theme(--spacing-4)]:flex'],

// This test in itself doesn't make much sense. But we need to make sure
// that this doesn't end up as the modifier in the candidate itself.
['max-[theme(spacing.4/50)]:flex', 'max-[theme(--spacing-4/50)]:flex'],

// `theme(…)` calls valid in v3, but not in v4 should still be converted.
['[--foo:theme(fontWeight.semibold)]', '[--foo:theme(fontWeight.semibold)]'],
])('%s => %s', async (candidate, result) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So there's a class of theme() keypaths that are valid in v3 (and are valid in our interop layer) but don't have a keypath equivalent in v4. Things like: theme(fontWeight.semibold) or theme(cursor.progress). Let's add some tests here to see they are still migrated.

Maybe what we can do is to use the dict in default-theme.ts and resolve these to the actual values?

Copy link
Member Author

@RobinMalfait RobinMalfait Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While on call, one thing we noticed is that theme(fontWeight.semibold) is hardcoded in the font-weight utility definition to be 600 and doesn't have a corresponding CSS variable. This means that we can't migrate theme(fontWeight.semibold) to a var(…) or even a theme(--font-weight-semibold) because there is no CSS variable. We could inline it to 600 directly since it's hardcoded.

However, there is a catch... if you define --font-weight-semibold: 100 as a CSS variable inside your @theme { … }, then the value is not hardcoded anymore, but the CSS variable takes precedence.

This means that we will migrate theme(fontWeight.semibold) to theme(--font-weight-semibold) or even var(--font-weight-semibold) but only if you have this CSS variable configured.

Long story short:

  1. If you migrate to var(…) or theme(--…), and remove the CSS variable from your @theme { … } later, then the default fontWeight.semibold won't apply.
  2. If you don't migrate, but inline the value theme(fontWeight.semibold) becomes 600. Then adding the CSS variable to your @theme { … } later, then the variable won't take effect because we already hardcoded the value to 600.
  3. Keep it as-is which is the more correct thing to do I think, but then we have some theme(…) calls with dot notation, some with --… notation and some var(…) notation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the safer thing to do here is to:

  1. Migrate if we a variable was configured (that's what will happen by default)
  2. Keep the old syntax if we can't instead of inlining the value.

let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', {
base: __dirname,
})

expect(themeToVar(designSystem, {}, candidate)).toEqual(result)
})
237 changes: 237 additions & 0 deletions packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
import type { Config } from 'tailwindcss'
import {
parseCandidate,
type Candidate,
type CandidateModifier,
type Variant,
} from '../../../../tailwindcss/src/candidate'
import { keyPathToCssProperty } from '../../../../tailwindcss/src/compat/apply-config-to-theme'
import type { DesignSystem } from '../../../../tailwindcss/src/design-system'
import { segment } from '../../../../tailwindcss/src/utils/segment'
import { toKeyPath } from '../../../../tailwindcss/src/utils/to-key-path'
import * as ValueParser from '../../../../tailwindcss/src/value-parser'
import { printCandidate } from '../candidates'

export function themeToVar(
designSystem: DesignSystem,
_userConfig: Config,
rawCandidate: string,
): string {
function convertBasic(input: string, modernizeThemeOnly = false) {
return substituteFunctionsInValue(input, (path, fallback) => {
let parts = segment(path, '/').map((part) => part.trim())

if (
// The path contains a `/`, which means that there is a modifier such as
// `theme(colors.red.500/50%)`. We can't convert this to a CSS variable.
parts.length > 1 ||
// In some scenarios (variants), we can't migrate to `var(…)` if it ends
// up in the `@media (…)` part. To be safe, let's just modernize the
// `theme(…)`
modernizeThemeOnly
) {
let path = parts.shift()!

// Convert to modern `theme(…)` syntax using CSS variable syntax if we
// can.
let variable = `--${keyPathToCssProperty(toKeyPath(path))}` as const

if (!designSystem.theme.get([variable])) {
return null
}

let modifier = parts.length > 0 ? `/${parts.join('/')}` : ''

return fallback
? `theme(${variable}${modifier}, ${fallback})`
: `theme(${variable}${modifier})`
}

// Convert to `var(…)`
let variable = `--${keyPathToCssProperty(toKeyPath(path))}` as const
if (!designSystem.theme.get([variable])) return null

return fallback ? `var(${variable}, ${fallback})` : `var(${variable})`
})
}

function convert(input: string, containsModifier = false) {
if (containsModifier) return [convertBasic(input), null] as const

let modifiers: CandidateModifier[] = []
let result = substituteFunctionsInValue(input, (path, fallback) => {
let parts = segment(path, '/').map((part) => part.trim())

// The path contains a `/`, which means that there is a modifier such as
// `theme(colors.red.500/50%)`.
//
// Currently, we are assuming that this is only being used for colors,
// which means that we can typically convert them to a modifier on the
// candidate itself.
if (parts.length === 2) {
let [pathPart, modifierPart] = parts

// 50% -> /50
if (/^\d+%$/.test(modifierPart)) {
modifiers.push({ kind: 'named', value: modifierPart.slice(0, -1) })
}

// .12 -> /12
// .12345 -> /[12.345]
else if (/^0?\.\d+$/.test(modifierPart)) {
let value = Number(modifierPart) * 100
modifiers.push({
kind: Number.isInteger(value) ? 'named' : 'arbitrary',
value: value.toString(),
})
}

// Anything else becomes arbitrary
else {
modifiers.push({ kind: 'arbitrary', value: modifierPart })
}

// Update path to be the first part
path = pathPart
}

let variable = `--${keyPathToCssProperty(toKeyPath(path))}` as const
if (!designSystem.theme.get([variable])) return null

return fallback ? `var(${variable}, ${fallback})` : `var(${variable})`
})

// Multiple modifiers which means that there are multiple `theme(…/…)`
// values. In this case, we can't convert the modifier to a candidate
// modifier. Try to convert each `theme(…)` call to the modern syntax.
if (modifiers.length > 1) return [convertBasic(input), null] as const

return [result, modifiers[0]] as const
}

for (let candidate of parseCandidate(rawCandidate, designSystem)) {
let clone = structuredClone(candidate)
let changed = false

if (clone.kind === 'arbitrary') {
let [newValue, modifier] = convert(clone.value, clone.modifier !== null)
if (newValue !== clone.value) {
changed = true
clone.value = newValue

if (modifier !== null) {
clone.modifier = modifier
}
}
} else if (clone.kind === 'functional' && clone.value?.kind === 'arbitrary') {
let [newValue, modifier] = convert(clone.value.value, clone.modifier !== null)
if (newValue !== clone.value.value) {
changed = true
clone.value.value = newValue

if (modifier !== null) {
clone.modifier = modifier
}
}
}

// Handle variants
for (let variant of variants(clone)) {
if (variant.kind === 'arbitrary') {
let newValue = convertBasic(variant.selector, true)
if (newValue !== variant.selector) {
changed = true
variant.selector = newValue
}
} else if (variant.kind === 'functional' && variant.value?.kind === 'arbitrary') {
let newValue = convertBasic(variant.value.value, true)
if (newValue !== variant.value.value) {
changed = true
variant.value.value = newValue
}
}
}

return changed ? printCandidate(designSystem, clone) : rawCandidate
}

return rawCandidate
}

function substituteFunctionsInValue(
value: string,
handle: (value: string, fallback?: string) => string | null,
) {
let ast = ValueParser.parse(value)
ValueParser.walk(ast, (node, { replaceWith }) => {
if (node.kind === 'function' && node.value === 'theme') {
if (node.nodes.length < 1) return

let pathNode = node.nodes[0]
if (pathNode.kind !== 'word') return

let path = pathNode.value

// For the theme function arguments, we require all separators to contain
// comma (`,`), spaces alone should be merged into the previous word to
// avoid splitting in this case:
//
// theme(--color-red-500 / 75%) theme(--color-red-500 / 75%, foo, bar)
//
// We only need to do this for the first node, as the fallback values are
// passed through as-is.
let skipUntilIndex = 1
for (let i = skipUntilIndex; i < node.nodes.length; i++) {
if (node.nodes[i].value.includes(',')) {
break
}
path += ValueParser.toCss([node.nodes[i]])
skipUntilIndex = i + 1
}

path = eventuallyUnquote(path)
let fallbackValues = node.nodes.slice(skipUntilIndex + 1)

let replacement =
fallbackValues.length > 0 ? handle(path, ValueParser.toCss(fallbackValues)) : handle(path)
if (replacement === null) return

replaceWith(ValueParser.parse(replacement))
}
})

return ValueParser.toCss(ast)
}

function eventuallyUnquote(value: string) {
if (value[0] !== "'" && value[0] !== '"') return value

let unquoted = ''
let quoteChar = value[0]
for (let i = 1; i < value.length - 1; i++) {
let currentChar = value[i]
let nextChar = value[i + 1]

if (currentChar === '\\' && (nextChar === quoteChar || nextChar === '\\')) {
unquoted += nextChar
i++
} else {
unquoted += currentChar
}
}

return unquoted
}

function* variants(candidate: Candidate) {
function* inner(variant: Variant): Iterable<Variant> {
yield variant
if (variant.kind === 'compound') {
yield* inner(variant.variant)
}
}

for (let variant of candidate.variants) {
yield* inner(variant)
}
}
2 changes: 2 additions & 0 deletions packages/@tailwindcss-upgrade/src/template/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { bgGradient } from './codemods/bg-gradient'
import { important } from './codemods/important'
import { prefix } from './codemods/prefix'
import { simpleLegacyClasses } from './codemods/simple-legacy-classes'
import { themeToVar } from './codemods/theme-to-var'
import { variantOrder } from './codemods/variant-order'
import { spliceChangesIntoString, type StringChange } from './splice-changes-into-string'

Expand All @@ -25,6 +26,7 @@ export const DEFAULT_MIGRATIONS: Migration[] = [
bgGradient,
simpleLegacyClasses,
arbitraryValueToBareValue,
themeToVar,
variantOrder,
]

Expand Down