From 3fb49bb0ee8ac2b0c5d564696592a884d5206a1e Mon Sep 17 00:00:00 2001
From: Robin Malfait <malfait.robin@gmail.com>
Date: Fri, 8 Nov 2024 17:05:30 +0100
Subject: [PATCH] =?UTF-8?q?Fix=20`theme(=E2=80=A6/15%)`=20to=20only=20appl?=
 =?UTF-8?q?y=20when=20used=20on=20its=20own=20(#14922)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 CHANGELOG.md                                               | 1 +
 .../src/template/codemods/theme-to-var.test.ts             | 7 +++++++
 .../src/template/codemods/theme-to-var.ts                  | 7 ++++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e913c1869..e0d014184 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts
index 0e6850672..2bad8ba80 100644
--- a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts
@@ -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'],
diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts
index 78b4ee2c6..c1eb98628 100644
--- a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts
+++ b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.ts
@@ -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