Skip to content

Commit

Permalink
Fix theme(…/15%) to only apply when used on its own (#14922)
Browse files Browse the repository at this point in the history
This PR fixes an issue where our codemod migrations can convert
`bg-[theme(colors.white/15%)]` to `bg-[var(--color-white)]/15` where the
`15%` from within the `theme(…)` is converted to a candidate modifier
(at the end).

The idea was that if the `theme(…)` is used with a modifier, then it can
only be used with colors. If a candidate uses it, it also means that a
color was used and we can use `/15` instead.

However this is not true if it is used as part of a bigger value. E.g.:
`shadow-[shadow:inset_0_0_0_1px_theme(colors.white/15%)]` would be
converted to `shadow-[inset_0_0_0_1px_var(--color-white)]/15` which is
not correct because the value isn't a color, the color is _part_ of the
value.

In this case, we make sure that the `theme(…)` is the only AST node in
the value, and if it is we can safely do the conversion. If there are
other AST nodes we keep the `theme(…)` call.

---------

Co-authored-by: Adam Wathan <adam.wathan@gmail.com>
  • Loading branch information
RobinMalfait and adamwathan authored Nov 8, 2024
1 parent 586723f commit 3fb49bb
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- _Upgrade (experimental)_: Install `@tailwindcss/postcss` next to `tailwindcss` ([#14830](https://github.com/tailwindlabs/tailwindcss/pull/14830))
- _Upgrade (experimental)_: Remove whitespace around `,` separator when print arbitrary values ([#14838](https://github.com/tailwindlabs/tailwindcss/pull/14838))
- _Upgrade (experimental)_: Fix crash during upgrade when content globs escape root of project ([#14896](https://github.com/tailwindlabs/tailwindcss/pull/14896))
- _Upgrade (experimental)_: Don't convert `theme(…/15%)` to modifier unless it is the entire arbitrary value of a utility ([#14922](https://github.com/tailwindlabs/tailwindcss/pull/14922))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ test.each([
// Arbitrary property that already contains a modifier
['[color:theme(colors.red.500/50%)]/50', '[color:theme(--color-red-500/50%)]/50'],

// Values that don't contain only `theme(…)` calls should not be converted to
// use a modifier since the color is not the whole value.
[
'shadow-[shadow:inset_0px_1px_theme(colors.white/15%)]',
'shadow-[shadow:inset_0px_1px_theme(--color-white/15%)]',
],

// 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'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,12 @@ export function createConverter(designSystem: DesignSystem, { prettyPrint = fals
// 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 && options & Convert.MigrateModifier) {
//
// If there is more than one node in the AST though, `theme(…)` must not
// be the whole value so it's not safe to use a modifier instead.
//
// E.g.: `inset 0px 1px theme(colors.red.500/50%)` is a shadow, not a color.
if (ast.length === 1 && parts.length === 2 && options & Convert.MigrateModifier) {
let [pathPart, modifierPart] = parts

// 50% -> /50
Expand Down

0 comments on commit 3fb49bb

Please sign in to comment.