Fix CSS theme() function resolution issue (#14614)

We currently have three different implementations of the `resolveThemeValue()` Theme method: 1. The _core_ version which only resolves based on the exact CSS variable name. This is the v4-only version. 2. A _compat light_ version which attempts to translate a dot-notation keypath to a CSS variable name. 3. A _full compat_ version which resolves the legacy theme object which is used whenever a `@plugin` or `@config` is added. Unfortunately, there are some issues with this approach that we've encountered when testing the CSS `theme()` function while upgrading Tailwind Templates to v4 using our upgrading toolkit. 1. The _compat light_ resolver was trying to convert `theme(spacing.1)` to tuple syntax. Tuples require a nested property access, though, and instead this should be convert to `theme(--spacing-1)`. 2. We currently only load _full compat_ version if `@plugin` or `@config` directives are used. It is possible, though, that we need the full compat mapping in other cases as well. One example we encountered is `theme(fontWeight.semibold)` which maps to a dictionary in the default theme that that we do not have an equivalent for in v4 variables. To fix both issues, we decided to remove the _compat light_ version of the `theme()` function in favor of adding this behavior to an upgrade codemod. Instead, the second layer now only decides wether it can use the _core_ version or wether it needs to upgrade to the _full compat_ version. Since typos would trigger a _hard error_, we do not think this has unintended performance consequences if someone wants to use the _core_ version only (they would get an error regardless which they need to fix in order to continue).
tailwindlabs · Oct 10, 2024 · dc69802 · dc69802
1 parent df06003
commit dc69802
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 117 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Don’t crash when scanning a candidate equal to the configured prefix ([#14588](https://github.com/tailwindlabs/tailwindcss/pull/14588))
 - Ensure there's always a space before `!important` when stringifying CSS ([#14611](https://github.com/tailwindlabs/tailwindcss/pull/14611))
 - Don't set `display: none` on elements that use `hidden="until-found"` ([#14631](https://github.com/tailwindlabs/tailwindcss/pull/14631))
+- Ensure the CSS `theme()` function resolves to the right value in some compatibility situations ([#14614](https://github.com/tailwindlabs/tailwindcss/pull/14614))
 - Fix issue that could cause the CLI to crash when files are deleted while watching ([#14616](https://github.com/tailwindlabs/tailwindcss/pull/14616))
 - Ensure custom variants using the JS API have access to modifiers ([#14637](https://github.com/tailwindlabs/tailwindcss/pull/14637))
 - _Upgrade (experimental)_: Ensure CSS before a layer stays unlayered when running codemods ([#14596](https://github.com/tailwindlabs/tailwindcss/pull/14596))

diff --git a/packages/tailwindcss/src/compat/apply-compat-hooks.ts b/packages/tailwindcss/src/compat/apply-compat-hooks.ts
@@ -1,9 +1,6 @@
 import { rule, toCss, walk, WalkAction, type AstNode } from '../ast'
 import type { DesignSystem } from '../design-system'
-import type { Theme, ThemeKey } from '../theme'
-import { withAlpha } from '../utilities'
 import { segment } from '../utils/segment'
-import { toKeyPath } from '../utils/to-key-path'
 import { applyConfigToTheme } from './apply-config-to-theme'
 import { applyKeyframesToAst } from './apply-keyframes-to-ast'
 import { createCompatConfig } from './config/create-compat-config'
@@ -129,23 +126,17 @@ export async function applyCompatibilityHooks({
       return resolveThemeVariableValue(path)
     }
 
-    // Extract an eventual modifier from the path. e.g.:
-    // - "colors.red.500 / 50%" -> "50%"
-    let lastSlash = path.lastIndexOf('/')
-    let modifier: string | null = null
-    if (lastSlash !== -1) {
-      modifier = path.slice(lastSlash + 1).trim()
-      path = path.slice(0, lastSlash).trim() as ThemeKey
-    }
-
-    let themeValue = lookupThemeValue(designSystem.theme, path)
-
-    // Apply the opacity modifier if present
-    if (modifier && themeValue) {
-      return withAlpha(themeValue, modifier)
-    }
-
-    return themeValue
+    // If the theme value is not found in the simple resolver, we upgrade to the full backward
+    // compatibility support implementation of the `resolveThemeValue` function.
+    upgradeToFullPluginSupport({
+      designSystem,
+      base,
+      ast,
+      globs,
+      configs: [],
+      pluginDetails: [],
+    })
+    return designSystem.resolveThemeValue(path)
   }
 
   // If there are no plugins or configs registered, we don't need to register
@@ -176,6 +167,40 @@ export async function applyCompatibilityHooks({
     ),
   ])
 
+  upgradeToFullPluginSupport({
+    designSystem,
+    base,
+    ast,
+    globs,
+    configs,
+    pluginDetails,
+  })
+}
+
+function upgradeToFullPluginSupport({
+  designSystem,
+  base,
+  ast,
+  globs,
+  configs,
+  pluginDetails,
+}: {
+  designSystem: DesignSystem
+  base: string
+  ast: AstNode[]
+  globs: { origin?: string; pattern: string }[]
+  configs: {
+    path: string
+    base: string
+    config: UserConfig
+  }[]
+  pluginDetails: {
+    path: string
+    base: string
+    plugin: Plugin
+    options: CssPluginOptions | null
+  }[]
+}) {
   let pluginConfigs = pluginDetails.map((detail) => {
     if (!detail.options) {
       return { config: { plugins: [detail.plugin] }, base: detail.base }
@@ -291,100 +316,3 @@ export async function applyCompatibilityHooks({
     globs.push(file)
   }
 }
-
-function toThemeKey(keypath: string[]) {
-  return (
-    keypath
-      // [1] should move into the nested object tuple. To create the CSS variable
-      // name for this, we replace it with an empty string that will result in two
-      // subsequent dashes when joined.
-      .map((path) => (path === '1' ? '' : path))
-
-      // Resolve the key path to a CSS variable segment
-      .map((part) =>
-        part
-          .replaceAll('.', '_')
-          .replace(/([a-z])([A-Z])/g, (_, a, b) => `${a}-${b.toLowerCase()}`),
-      )
-
-      // Remove the `DEFAULT` key at the end of a path
-      // We're reading from CSS anyway so it'll be a string
-      .filter((part, index) => part !== 'DEFAULT' || index !== keypath.length - 1)
-      .join('-')
-  )
-}
-
-function lookupThemeValue(theme: Theme, path: string) {
-  let baseThemeKey = '--' + toThemeKey(toKeyPath(path))
-
-  let resolvedValue = theme.get([baseThemeKey as ThemeKey])
-
-  if (resolvedValue !== null) {
-    return resolvedValue
-  }
-
-  for (let [givenKey, upgradeKey] of Object.entries(themeUpgradeKeys)) {
-    if (!baseThemeKey.startsWith(givenKey)) continue
-
-    let upgradedKey = upgradeKey + baseThemeKey.slice(givenKey.length)
-    let resolvedValue = theme.get([upgradedKey as ThemeKey])
-
-    if (resolvedValue !== null) {
-      return resolvedValue
-    }
-  }
-}
-
-let themeUpgradeKeys = {
-  '--colors': '--color',
-  '--accent-color': '--color',
-  '--backdrop-blur': '--blur',
-  '--backdrop-brightness': '--brightness',
-  '--backdrop-contrast': '--contrast',
-  '--backdrop-grayscale': '--grayscale',
-  '--backdrop-hue-rotate': '--hueRotate',
-  '--backdrop-invert': '--invert',
-  '--backdrop-opacity': '--opacity',
-  '--backdrop-saturate': '--saturate',
-  '--backdrop-sepia': '--sepia',
-  '--background-color': '--color',
-  '--background-opacity': '--opacity',
-  '--border-color': '--color',
-  '--border-opacity': '--opacity',
-  '--border-radius': '--radius',
-  '--border-spacing': '--spacing',
-  '--box-shadow-color': '--color',
-  '--caret-color': '--color',
-  '--divide-color': '--borderColor',
-  '--divide-opacity': '--borderOpacity',
-  '--divide-width': '--borderWidth',
-  '--fill': '--color',
-  '--flex-basis': '--spacing',
-  '--gap': '--spacing',
-  '--gradient-color-stops': '--color',
-  '--height': '--spacing',
-  '--inset': '--spacing',
-  '--margin': '--spacing',
-  '--max-height': '--spacing',
-  '--max-width': '--spacing',
-  '--min-height': '--spacing',
-  '--min-width': '--spacing',
-  '--outline-color': '--color',
-  '--padding': '--spacing',
-  '--placeholder-color': '--color',
-  '--placeholder-opacity': '--opacity',
-  '--ring-color': '--color',
-  '--ring-offset-color': '--color',
-  '--ring-opacity': '--opacity',
-  '--scroll-margin': '--spacing',
-  '--scroll-padding': '--spacing',
-  '--space': '--spacing',
-  '--stroke': '--color',
-  '--text-color': '--color',
-  '--text-decoration-color': '--color',
-  '--text-indent': '--spacing',
-  '--text-opacity': '--opacity',
-  '--translate': '--spacing',
-  '--size': '--spacing',
-  '--width': '--spacing',
-}
diff --git a/packages/tailwindcss/src/css-functions.test.ts b/packages/tailwindcss/src/css-functions.test.ts
@@ -935,3 +935,45 @@ test('replaces CSS theme() function with values inside imported stylesheets', as
     }"
   `)
 })
+
+test('resolves paths ending with a 1', async () => {
+  expect(
+    await compileCss(
+      css`
+        @theme {
+          --spacing-1: 0.25rem;
+        }
+
+        .foo {
+          margin: theme(spacing.1);
+        }
+      `,
+      [],
+    ),
+  ).toMatchInlineSnapshot(`
+    ":root {
+      --spacing-1: .25rem;
+    }
+
+    .foo {
+      margin: .25rem;
+    }"
+  `)
+})
+
+test('upgrades to a full JS compat theme lookup if a value can not be mapped to a CSS variable', async () => {
+  expect(
+    await compileCss(
+      css`
+        .semi {
+          font-weight: theme(fontWeight.semibold);
+        }
+      `,
+      [],
+    ),
+  ).toMatchInlineSnapshot(`
+    ".semi {
+      font-weight: 600;
+    }"
+  `)
+})