From dc6980230d670f1f825292476de47c60ae6af13a Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 10 Oct 2024 12:31:46 +0200 Subject: [PATCH] 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). --- CHANGELOG.md | 1 + .../src/compat/apply-compat-hooks.ts | 162 +++++------------- .../tailwindcss/src/css-functions.test.ts | 42 +++++ 3 files changed, 88 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cca63f00e97..483c609af02b 100644 --- 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 index 1de9281f9a1d..ff78bd5cf60a 100644 --- 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 index 2e3e5f0aa378..cb54f3406972 100644 --- 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; + }" + `) +})