From bba014c7e29a682e59408d4a575a185cd927d4c0 Mon Sep 17 00:00:00 2001 From: Adam Wathan <4323180+adamwathan@users.noreply.github.com> Date: Thu, 5 Sep 2024 20:03:35 -0400 Subject: [PATCH 01/12] Add failing test --- .../tailwindcss/src/compat/config.test.ts | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts index 72278a60ba1a..0a5c6d3dc194 100644 --- a/packages/tailwindcss/src/compat/config.test.ts +++ b/packages/tailwindcss/src/compat/config.test.ts @@ -230,6 +230,44 @@ test('Variants in CSS overwrite variants from plugins', async ({ expect }) => { `) }) +describe('theme callbacks', () => { + test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values', async ({ + expect, + }) => { + let input = css` + @theme default { + --font-size-xl: 1.25rem; + --font-size-xl--line-height: 1.5rem; + } + @tailwind utilities; + @config "./config.js"; + ` + + let compiler = await compile(input, { + loadConfig: async () => ({ + theme: { + extend: { + fontSize: { + xl: ['1.25rem', { lineHeight: '1.75rem' }], + }, + lineHeight: ({ theme }) => ({ + xl: theme('fontSize.xl[1].lineHeight'), + }), + }, + }, + }), + }) + expect(compiler.build(['leading-xl'])).toMatchInlineSnapshot(` + ":root { + } + .leading-xl { + line-height: 1.75rem; + } + " + `) + }) +}) + describe('default font family compatibility', () => { test('overriding `fontFamily.sans` sets `--default-font-family`', async ({ expect }) => { let input = css` From 237c6ded5026c2e8d7a94c60f969c5507479ae7f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 14:13:22 +0200 Subject: [PATCH 02/12] WIP --- .../tailwindcss/src/compat/config.test.ts | 136 +++++++++++++++++- .../src/compat/config/deep-merge.ts | 13 +- .../src/compat/plugin-functions.ts | 83 +++++++++-- packages/tailwindcss/src/theme.ts | 4 + 4 files changed, 214 insertions(+), 22 deletions(-) diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts index 0a5c6d3dc194..a5192268fe12 100644 --- a/packages/tailwindcss/src/compat/config.test.ts +++ b/packages/tailwindcss/src/compat/config.test.ts @@ -258,13 +258,135 @@ describe('theme callbacks', () => { }), }) expect(compiler.build(['leading-xl'])).toMatchInlineSnapshot(` - ":root { - } - .leading-xl { - line-height: 1.75rem; - } - " - `) + ":root { + } + .leading-xl { + line-height: 1.75rem; + } + " + `) + }) + + test.only('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (2)', async ({ + expect, + }) => { + let input = css` + @theme default { + --color-slate-100: #000100; + --color-slate-200: #000200; + --color-slate-300: #000300; + } + @theme { + --color-slate-400: #100400; + --color-slate-500: #100500; + } + @tailwind utilities; + @config "./config.js"; + @plugin "./plugin.js"; + ` + + let compiler = await compile(input, { + loadConfig: async () => ({ + theme: { + extend: { + colors: { + slate: { + 200: '#200200', + 400: '#200400', + 600: '#200600', + }, + }, + hoverColors: ({ theme }) => theme('colors'), + }, + }, + }), + + loadPlugin: async () => { + return plugin(({ matchUtilities, theme }) => { + matchUtilities( + { + 'hover-bg': (value) => { + return { + '&:hover': { + backgroundColor: value, + }, + } + }, + }, + { + values: theme('hoverColors'), + }, + ) + }) + }, + }) + expect( + compiler.build([ + 'bg-slate-100', + 'bg-slate-200', + 'bg-slate-300', + 'bg-slate-400', + 'bg-slate-500', + 'bg-slate-600', + 'hover-bg-slate-100', + 'hover-bg-slate-200', + 'hover-bg-slate-300', + 'hover-bg-slate-400', + 'hover-bg-slate-500', + 'hover-bg-slate-600', + ]), + ).toMatchInlineSnapshot(` + ":root { + --color-slate-100: #000100; + --color-slate-300: #000300; + --color-slate-400: #100400; + --color-slate-500: #100500; + } + .bg-slate-100 { + background-color: var(--color-slate-100, #000100); + } + .bg-slate-200 { + background-color: #200200; + } + .bg-slate-300 { + background-color: var(--color-slate-300, #000300); + } + .bg-slate-400 { + background-color: var(--color-slate-400, #100400); + } + .bg-slate-500 { + background-color: var(--color-slate-500, #100500); + } + .bg-slate-600 { + background-color: #200600; + } + .hover-bg-slate-100 { + &:hover { + background-color: #000100; + } + } + .hover-bg-slate-200 { + &:hover { + background-color: #200200; + } + } + .hover-bg-slate-300 { + &:hover { + background-color: #000300; + } + } + .hover-bg-slate-400 { + &:hover { + background-color: #100400; + } + } + .hover-bg-slate-500 { + &:hover { + background-color: #100500; + } + } + " + `) }) }) diff --git a/packages/tailwindcss/src/compat/config/deep-merge.ts b/packages/tailwindcss/src/compat/config/deep-merge.ts index 49ac100b5392..3bbae51ddcef 100644 --- a/packages/tailwindcss/src/compat/config/deep-merge.ts +++ b/packages/tailwindcss/src/compat/config/deep-merge.ts @@ -10,7 +10,8 @@ export function isPlainObject(value: T): value is T & Record( target: T, sources: (Partial | null | undefined)[], - customizer: (a: any, b: any) => any, + customizer: (a: any, b: any, keypath: (keyof T)[]) => any, + parentPath: (keyof T)[] = [], ) { type Key = keyof T type Value = T[Key] @@ -21,14 +22,20 @@ export function deepMerge( } for (let k of Reflect.ownKeys(source) as Key[]) { - let merged = customizer(target[k], source[k]) + let currentParentPath = [...parentPath, k] + let merged = customizer(target[k], source[k], currentParentPath) if (merged !== undefined) { target[k] = merged } else if (!isPlainObject(target[k]) || !isPlainObject(source[k])) { target[k] = source[k] as Value } else { - target[k] = deepMerge({}, [target[k], source[k]], customizer) as Value + target[k] = deepMerge( + {}, + [target[k], source[k]], + customizer, + currentParentPath as any, + ) as Value } } } diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index 810953aab29d..b7330b74ec48 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -1,5 +1,5 @@ import type { DesignSystem } from '../design-system' -import type { Theme, ThemeKey } from '../theme' +import { ThemeOptions, type Theme, type ThemeKey } from '../theme' import { withAlpha } from '../utilities' import { DefaultMap } from '../utils/default-map' import { toKeyPath } from '../utils/to-key-path' @@ -24,16 +24,63 @@ export function createThemeFn( let resolvedValue = (() => { let keypath = toKeyPath(path) - let cssValue = readFromCss(designSystem.theme, keypath) + let [cssValue, options] = readFromCss(designSystem.theme, keypath) + + let configValue = resolveValue(get(configTheme() ?? {}, keypath) ?? null) if (typeof cssValue !== 'object') { + if (options & ThemeOptions.DEFAULT) { + return configValue ?? cssValue + } + return cssValue } - let configValue = resolveValue(get(configTheme() ?? {}, keypath) ?? null) - if (configValue !== null && typeof configValue === 'object' && !Array.isArray(configValue)) { - return deepMerge({}, [configValue, cssValue], (_, b) => b) + let configValueCopy = deepMerge({}, [configValue], (_, b) => { + return b + }) + + for (let key in cssValue) { + let configLeafValue = get(configValueCopy, key.split('-')) + if (configLeafValue && options.get(key) & ThemeOptions.DEFAULT) { + continue + } + + configValueCopy[key] = cssValue[key] + } + + console.dir({ configValueCopy }, { depth: null }) + + return configValueCopy + // console.dir({ abc }, { depth: null }) + return deepMerge({}, [configValue, cssValue], (a, b, path) => { + // console.log({ + // a, + // b, + // path, + // cssValue, + // options: cssValue === null ? options : options.get(path.join('-')), + // }) + // let fullKey = path.join('-') + // // console.log({ fullKey, options, configValue }) + // if ( + // configValue[fullKey] && + // options.get(fullKey.slice('--color-'.length) & ThemeOptions.DEFAULT) + // ) { + // return configValue[fullKey] + // } + // + // console.log({ a, b }) + // if (typeof a === 'string' && typeof b === 'string') { + // let key = path.join('-') + // console.log({ a, b, key }) + // if (options.get(key.slice('--color-'.length)) & ThemeOptions.DEFAULT) { + // return a + // } + // } + return b + }) } // Values from CSS take precedence over values from the config @@ -49,11 +96,14 @@ export function createThemeFn( } } -function readFromCss(theme: Theme, path: string[]) { +function readFromCss( + theme: Theme, + path: string[], +): [value: string | null, options: number] | [value: object, options: Map] { // `--color-red-500` should resolve to the theme variable directly, no look up // and handling of nested objects is required. if (path.length === 1 && path[0].startsWith('--')) { - return theme.get([path[0] as ThemeKey]) + return [theme.get([path[0] as ThemeKey]), theme.getOptions(path[0])] } type ThemeValue = @@ -82,10 +132,13 @@ function readFromCss(theme: Theme, path: string[]) { let map = new Map() let nested = new DefaultMap>(() => new Map()) - let ns = theme.namespace(`--${themeKey}` as any) + // TODO: Bad Robin + if (themeKey === 'colors') themeKey = 'color' + let ns = theme.namespace(`--${themeKey}` as any) if (ns.size === 0) { - return null + console.log({ themeKey }) + return [null, ThemeOptions.NONE] } for (let [key, value] of ns) { @@ -117,6 +170,12 @@ function readFromCss(theme: Theme, path: string[]) { // We have to turn the map into object-like structure for v3 compatibility let obj: Record = {} let useNestedObjects = false // paths.some((path) => nestedKeys.has(path)) + let options = new Map( + Array.from(ns.entries()).map(([key]) => { + let fullKey = key === null ? `--${themeKey}` : `--${themeKey}-${key}` + return [key, theme.getOptions(fullKey)] + }), + ) for (let [key, value] of map) { key = key ?? 'DEFAULT' @@ -139,17 +198,17 @@ function readFromCss(theme: Theme, path: string[]) { // the `DEFAULT` key from the list of possible values. If there is no // `DEFAULT` in the list, there is no match so return `null`. if (path[path.length - 1] === 'DEFAULT') { - return obj?.DEFAULT ?? null + return [obj?.DEFAULT ?? null, options.get(null) ?? 0] } // The request looked like `theme('animation.spin')` and was turned into a // lookup for `--animation-spin-*` which had only one entry which means it // should be returned directly. if ('DEFAULT' in obj && Object.keys(obj).length === 1) { - return obj.DEFAULT + return [obj.DEFAULT, options.get(null) ?? 0] } - return obj + return [obj, options] } function get(obj: any, path: string[]) { diff --git a/packages/tailwindcss/src/theme.ts b/packages/tailwindcss/src/theme.ts index 7c2f5cb07930..775c06836300 100644 --- a/packages/tailwindcss/src/theme.ts +++ b/packages/tailwindcss/src/theme.ts @@ -69,6 +69,10 @@ export class Theme { return ((this.values.get(key)?.options ?? 0) & ThemeOptions.DEFAULT) === ThemeOptions.DEFAULT } + getOptions(key: string) { + return this.values.get(key)?.options ?? 0 + } + entries() { return this.values.entries() } From 1efd99750cdb09e3a452d39e11eca651750f353f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 16:52:35 +0200 Subject: [PATCH 03/12] WIP --- .../tailwindcss/src/compat/config.test.ts | 55 ++++++++++++-- .../src/compat/flatten-color-palette.ts | 19 +++++ .../src/compat/plugin-functions.ts | 73 +++++++++---------- packages/tailwindcss/src/plugin-api.test.ts | 1 + 4 files changed, 104 insertions(+), 44 deletions(-) create mode 100644 packages/tailwindcss/src/compat/flatten-color-palette.ts diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts index a5192268fe12..4121f82b053e 100644 --- a/packages/tailwindcss/src/compat/config.test.ts +++ b/packages/tailwindcss/src/compat/config.test.ts @@ -1,6 +1,7 @@ import { describe, test } from 'vitest' import { compile } from '..' import plugin from '../plugin' +import { flattenColorPalette } from './flatten-color-palette' const css = String.raw @@ -267,7 +268,7 @@ describe('theme callbacks', () => { `) }) - test.only('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (2)', async ({ + test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (2)', async ({ expect, }) => { let input = css` @@ -296,7 +297,6 @@ describe('theme callbacks', () => { 600: '#200600', }, }, - hoverColors: ({ theme }) => theme('colors'), }, }, }), @@ -313,13 +313,12 @@ describe('theme callbacks', () => { } }, }, - { - values: theme('hoverColors'), - }, + { values: flattenColorPalette(theme('colors')) }, ) }) }, }) + expect( compiler.build([ 'bg-slate-100', @@ -385,6 +384,52 @@ describe('theme callbacks', () => { background-color: #100500; } } + .hover-bg-slate-600 { + &:hover { + background-color: #200600; + } + } + " + `) + }) + + test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (3)', async ({ + expect, + }) => { + let input = css` + @theme default { + --color-red: red; + } + @theme { + --color-blue: blue; + } + @tailwind utilities; + @config "./config.js"; + ` + + let compiler = await compile(input, { + loadConfig: async () => ({ + theme: { + extend: { + colors: { + red: 'very-red', + blue: 'very-blue', + }, + }, + }, + }), + }) + + expect(compiler.build(['bg-red', 'bg-blue'])).toMatchInlineSnapshot(` + ":root { + --color-blue: blue; + } + .bg-blue { + background-color: var(--color-blue, blue); + } + .bg-red { + background-color: very-red; + } " `) }) diff --git a/packages/tailwindcss/src/compat/flatten-color-palette.ts b/packages/tailwindcss/src/compat/flatten-color-palette.ts new file mode 100644 index 000000000000..198ae060d7c5 --- /dev/null +++ b/packages/tailwindcss/src/compat/flatten-color-palette.ts @@ -0,0 +1,19 @@ +type Colors = { + [key: string | number]: string | Colors +} + +export function flattenColorPalette(colors: Colors) { + let result: Record = {} + + for (let [root, children] of Object.entries(colors ?? {})) { + if (typeof children === 'object' && children !== null) { + for (let [parent, value] of Object.entries(flattenColorPalette(children))) { + result[`${root}${parent === 'DEFAULT' ? '' : `-${parent}`}`] = value + } + } else { + result[root] = children + } + } + + return result +} diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index b7330b74ec48..6a39c8a221b5 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -26,8 +26,28 @@ export function createThemeFn( let keypath = toKeyPath(path) let [cssValue, options] = readFromCss(designSystem.theme, keypath) + if (typeof cssValue === 'object' && cssValue !== null) { + cssValue.__CSS_VALUES__ ??= options + } + let configValue = resolveValue(get(configTheme() ?? {}, keypath) ?? null) + if ( + cssValue === null && + typeof configValue === 'object' && + configValue !== null && + Object.hasOwn(configValue, '__CSS_VALUES__') + ) { + cssValue = {} + for (let key in configValue.__CSS_VALUES__) { + cssValue[key] = configValue[key] + } + configValue = Object.fromEntries( + Object.entries(configValue).filter(([key]) => !(key in cssValue)), + ) + options = Object.assign({}, configValue.__CSS_VALUES__) + } + if (typeof cssValue !== 'object') { if (options & ThemeOptions.DEFAULT) { return configValue ?? cssValue @@ -42,45 +62,24 @@ export function createThemeFn( }) for (let key in cssValue) { + if (key === '__CSS_VALUES__') continue + let configLeafValue = get(configValueCopy, key.split('-')) - if (configLeafValue && options.get(key) & ThemeOptions.DEFAULT) { + if (configLeafValue && options[key] & ThemeOptions.DEFAULT) { continue } + // if (typeof cssValue[key] === 'object' && cssValue[key] !== null) { + // // configValueCopy[key] = { ...cssValue[key] } + // } else { configValueCopy[key] = cssValue[key] - } + // } - console.dir({ configValueCopy }, { depth: null }) + configValueCopy.__CSS_VALUES__ ??= {} + configValueCopy.__CSS_VALUES__[key] = options[key] + } return configValueCopy - // console.dir({ abc }, { depth: null }) - return deepMerge({}, [configValue, cssValue], (a, b, path) => { - // console.log({ - // a, - // b, - // path, - // cssValue, - // options: cssValue === null ? options : options.get(path.join('-')), - // }) - // let fullKey = path.join('-') - // // console.log({ fullKey, options, configValue }) - // if ( - // configValue[fullKey] && - // options.get(fullKey.slice('--color-'.length) & ThemeOptions.DEFAULT) - // ) { - // return configValue[fullKey] - // } - // - // console.log({ a, b }) - // if (typeof a === 'string' && typeof b === 'string') { - // let key = path.join('-') - // console.log({ a, b, key }) - // if (options.get(key.slice('--color-'.length)) & ThemeOptions.DEFAULT) { - // return a - // } - // } - return b - }) } // Values from CSS take precedence over values from the config @@ -132,12 +131,8 @@ function readFromCss( let map = new Map() let nested = new DefaultMap>(() => new Map()) - // TODO: Bad Robin - if (themeKey === 'colors') themeKey = 'color' - let ns = theme.namespace(`--${themeKey}` as any) if (ns.size === 0) { - console.log({ themeKey }) return [null, ThemeOptions.NONE] } @@ -170,10 +165,10 @@ function readFromCss( // We have to turn the map into object-like structure for v3 compatibility let obj: Record = {} let useNestedObjects = false // paths.some((path) => nestedKeys.has(path)) - let options = new Map( + let options = Object.fromEntries( Array.from(ns.entries()).map(([key]) => { let fullKey = key === null ? `--${themeKey}` : `--${themeKey}-${key}` - return [key, theme.getOptions(fullKey)] + return [key ?? 'DEFAULT', theme.getOptions(fullKey)] }), ) @@ -198,14 +193,14 @@ function readFromCss( // the `DEFAULT` key from the list of possible values. If there is no // `DEFAULT` in the list, there is no match so return `null`. if (path[path.length - 1] === 'DEFAULT') { - return [obj?.DEFAULT ?? null, options.get(null) ?? 0] + return [obj?.DEFAULT ?? null, options.DEFAULT ?? 0] } // The request looked like `theme('animation.spin')` and was turned into a // lookup for `--animation-spin-*` which had only one entry which means it // should be returned directly. if ('DEFAULT' in obj && Object.keys(obj).length === 1) { - return [obj.DEFAULT, options.get(null) ?? 0] + return [obj.DEFAULT, options.DEFAULT ?? 0] } return [obj, options] diff --git a/packages/tailwindcss/src/plugin-api.test.ts b/packages/tailwindcss/src/plugin-api.test.ts index e7ed5e8a681e..1063f1e90fe6 100644 --- a/packages/tailwindcss/src/plugin-api.test.ts +++ b/packages/tailwindcss/src/plugin-api.test.ts @@ -524,6 +524,7 @@ describe('theme', async () => { }) expect(fn).toHaveBeenCalledWith({ + __CSS_VALUES__: { bounce: 2, spin: 2 }, spin: 'spin 1s linear infinite', bounce: 'bounce 1s linear infinite', }) From f99b8c521aca4fa7618faeff62d102c2082b2da6 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 17:31:25 +0200 Subject: [PATCH 04/12] make TypeScript happy --- .../src/compat/plugin-functions.ts | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index 6a39c8a221b5..f3c6d8e0abea 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -26,40 +26,51 @@ export function createThemeFn( let keypath = toKeyPath(path) let [cssValue, options] = readFromCss(designSystem.theme, keypath) + // if (typeof cssValue === 'object' && cssValue !== null) { - cssValue.__CSS_VALUES__ ??= options + cssValue.__CSS_VALUES__ = options } let configValue = resolveValue(get(configTheme() ?? {}, keypath) ?? null) + // if ( cssValue === null && typeof configValue === 'object' && configValue !== null && Object.hasOwn(configValue, '__CSS_VALUES__') ) { - cssValue = {} + let localCssValue: Record = {} for (let key in configValue.__CSS_VALUES__) { - cssValue[key] = configValue[key] + localCssValue[key] = configValue[key] } configValue = Object.fromEntries( - Object.entries(configValue).filter(([key]) => !(key in cssValue)), + Object.entries(configValue).filter(([key]) => !(key in localCssValue)), ) + cssValue = localCssValue options = Object.assign({}, configValue.__CSS_VALUES__) } + // if (typeof cssValue !== 'object') { - if (options & ThemeOptions.DEFAULT) { + if (typeof options !== 'object' && options & ThemeOptions.DEFAULT) { return configValue ?? cssValue } return cssValue } - if (configValue !== null && typeof configValue === 'object' && !Array.isArray(configValue)) { - let configValueCopy = deepMerge({}, [configValue], (_, b) => { - return b - }) + // + if ( + configValue !== null && + typeof configValue === 'object' && + typeof options === 'object' && + !Array.isArray(configValue) + ) { + let configValueCopy: Record & { __CSS_VALUES__?: Record } = + deepMerge({}, [configValue], (_, b) => { + return b + }) for (let key in cssValue) { if (key === '__CSS_VALUES__') continue @@ -69,11 +80,7 @@ export function createThemeFn( continue } - // if (typeof cssValue[key] === 'object' && cssValue[key] !== null) { - // // configValueCopy[key] = { ...cssValue[key] } - // } else { configValueCopy[key] = cssValue[key] - // } configValueCopy.__CSS_VALUES__ ??= {} configValueCopy.__CSS_VALUES__[key] = options[key] @@ -98,11 +105,13 @@ export function createThemeFn( function readFromCss( theme: Theme, path: string[], -): [value: string | null, options: number] | [value: object, options: Map] { +): + | [value: string | null | Record, options: number] + | [value: Record, options: Record] { // `--color-red-500` should resolve to the theme variable directly, no look up // and handling of nested objects is required. if (path.length === 1 && path[0].startsWith('--')) { - return [theme.get([path[0] as ThemeKey]), theme.getOptions(path[0])] + return [theme.get([path[0] as ThemeKey]), theme.getOptions(path[0])] as const } type ThemeValue = @@ -193,17 +202,17 @@ function readFromCss( // the `DEFAULT` key from the list of possible values. If there is no // `DEFAULT` in the list, there is no match so return `null`. if (path[path.length - 1] === 'DEFAULT') { - return [obj?.DEFAULT ?? null, options.DEFAULT ?? 0] + return [(obj?.DEFAULT ?? null) as any, options.DEFAULT ?? 0] as const } // The request looked like `theme('animation.spin')` and was turned into a // lookup for `--animation-spin-*` which had only one entry which means it // should be returned directly. if ('DEFAULT' in obj && Object.keys(obj).length === 1) { - return [obj.DEFAULT, options.DEFAULT ?? 0] + return [obj.DEFAULT as string, options.DEFAULT ?? 0] as const } - return [obj, options] + return [obj, options] as const } function get(obj: any, path: string[]) { From 98421d237672753688963f74794677b3e1df4ca2 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 17:34:47 +0200 Subject: [PATCH 05/12] move tests around --- .../tailwindcss/src/compat/config.test.ts | 86 +++++++++---------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts index 4121f82b053e..7b34f8613ac4 100644 --- a/packages/tailwindcss/src/compat/config.test.ts +++ b/packages/tailwindcss/src/compat/config.test.ts @@ -267,10 +267,49 @@ describe('theme callbacks', () => { " `) }) +}) - test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (2)', async ({ - expect, - }) => { +describe('theme overrides order', () => { + test('user theme > js config > default theme', async ({ expect }) => { + let input = css` + @theme default { + --color-red: red; + } + @theme { + --color-blue: blue; + } + @tailwind utilities; + @config "./config.js"; + ` + + let compiler = await compile(input, { + loadConfig: async () => ({ + theme: { + extend: { + colors: { + red: 'very-red', + blue: 'very-blue', + }, + }, + }, + }), + }) + + expect(compiler.build(['bg-red', 'bg-blue'])).toMatchInlineSnapshot(` + ":root { + --color-blue: blue; + } + .bg-blue { + background-color: var(--color-blue, blue); + } + .bg-red { + background-color: very-red; + } + " + `) + }) + + test('user theme > js config > default theme (with nested object)', async ({ expect }) => { let input = css` @theme default { --color-slate-100: #000100; @@ -392,47 +431,6 @@ describe('theme callbacks', () => { " `) }) - - test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values (3)', async ({ - expect, - }) => { - let input = css` - @theme default { - --color-red: red; - } - @theme { - --color-blue: blue; - } - @tailwind utilities; - @config "./config.js"; - ` - - let compiler = await compile(input, { - loadConfig: async () => ({ - theme: { - extend: { - colors: { - red: 'very-red', - blue: 'very-blue', - }, - }, - }, - }), - }) - - expect(compiler.build(['bg-red', 'bg-blue'])).toMatchInlineSnapshot(` - ":root { - --color-blue: blue; - } - .bg-blue { - background-color: var(--color-blue, blue); - } - .bg-red { - background-color: very-red; - } - " - `) - }) }) describe('default font family compatibility', () => { From 7a90d34bfc68da37312a9754f5f681a7f3ea702c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 18:01:48 +0200 Subject: [PATCH 06/12] =?UTF-8?q?refactor=20`theme(=E2=80=A6)`,=20simplify?= =?UTF-8?q?=20implementation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/compat/plugin-functions.ts | 62 +++++++------------ 1 file changed, 24 insertions(+), 38 deletions(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index f3c6d8e0abea..32dec2180be5 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -26,32 +26,9 @@ export function createThemeFn( let keypath = toKeyPath(path) let [cssValue, options] = readFromCss(designSystem.theme, keypath) - // - if (typeof cssValue === 'object' && cssValue !== null) { - cssValue.__CSS_VALUES__ = options - } - let configValue = resolveValue(get(configTheme() ?? {}, keypath) ?? null) - // - if ( - cssValue === null && - typeof configValue === 'object' && - configValue !== null && - Object.hasOwn(configValue, '__CSS_VALUES__') - ) { - let localCssValue: Record = {} - for (let key in configValue.__CSS_VALUES__) { - localCssValue[key] = configValue[key] - } - configValue = Object.fromEntries( - Object.entries(configValue).filter(([key]) => !(key in localCssValue)), - ) - cssValue = localCssValue - options = Object.assign({}, configValue.__CSS_VALUES__) - } - - // + // Resolved to a primitive value. if (typeof cssValue !== 'object') { if (typeof options !== 'object' && options & ThemeOptions.DEFAULT) { return configValue ?? cssValue @@ -61,29 +38,35 @@ export function createThemeFn( } // - if ( - configValue !== null && - typeof configValue === 'object' && - typeof options === 'object' && - !Array.isArray(configValue) - ) { + if (configValue !== null && typeof configValue === 'object' && !Array.isArray(configValue)) { let configValueCopy: Record & { __CSS_VALUES__?: Record } = - deepMerge({}, [configValue], (_, b) => { - return b - }) + // We want to make sure that we don't mutate the original config + // value. Ideally we use `structuredClone` here, but it's not possible + // because it can contain functions. + deepMerge({}, [configValue], (_, b) => b) + + // There is no `cssValue`, which means we can back-fill it with values + // from the `configValue`. + if (cssValue === null && Object.hasOwn(configValue, '__CSS_VALUES__')) { + let localCssValue: Record = {} + for (let key in configValue.__CSS_VALUES__) { + localCssValue[key] = configValue[key] + delete configValueCopy[key] + } + cssValue = localCssValue + } for (let key in cssValue) { if (key === '__CSS_VALUES__') continue - let configLeafValue = get(configValueCopy, key.split('-')) - if (configLeafValue && options[key] & ThemeOptions.DEFAULT) { + if ( + configValue?.__CSS_VALUES__?.[key] & ThemeOptions.DEFAULT && + get(configValueCopy, key.split('-')) !== undefined + ) { continue } configValueCopy[key] = cssValue[key] - - configValueCopy.__CSS_VALUES__ ??= {} - configValueCopy.__CSS_VALUES__[key] = options[key] } return configValueCopy @@ -212,6 +195,9 @@ function readFromCss( return [obj.DEFAULT as string, options.DEFAULT ?? 0] as const } + // + obj.__CSS_VALUES__ = options + return [obj, options] as const } From 26c98ec645fae543b2c1e36c17c65b85110c3c78 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 6 Sep 2024 18:06:41 +0200 Subject: [PATCH 07/12] adjust comments --- packages/tailwindcss/src/compat/plugin-functions.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index 32dec2180be5..fcb35662ce41 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -59,6 +59,9 @@ export function createThemeFn( for (let key in cssValue) { if (key === '__CSS_VALUES__') continue + // If the value is coming from a default source (`@theme default`), + // then we keep the value from the js config (which is also a + // default source, but wins from the built in defaults). if ( configValue?.__CSS_VALUES__?.[key] & ThemeOptions.DEFAULT && get(configValueCopy, key.split('-')) !== undefined @@ -66,6 +69,7 @@ export function createThemeFn( continue } + // CSS values from `@theme` win over values from the config configValueCopy[key] = cssValue[key] } @@ -195,7 +199,8 @@ function readFromCss( return [obj.DEFAULT as string, options.DEFAULT ?? 0] as const } - // + // Attach the CSS values to the object for later use. This object could be + // mutated by the user so we want to keep the original CSS values around. obj.__CSS_VALUES__ = options return [obj, options] as const From 67daf9f18f65690f018463e2ae6a54f2f4d73dce Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 6 Sep 2024 14:55:42 -0400 Subject: [PATCH 08/12] Correctly overwrite default theme tuple values --- .../tailwindcss/src/compat/config.test.ts | 76 +++++++++++++++++-- .../src/compat/plugin-functions.ts | 61 +++++++++++---- 2 files changed, 118 insertions(+), 19 deletions(-) diff --git a/packages/tailwindcss/src/compat/config.test.ts b/packages/tailwindcss/src/compat/config.test.ts index 7b34f8613ac4..9e8849513b6a 100644 --- a/packages/tailwindcss/src/compat/config.test.ts +++ b/packages/tailwindcss/src/compat/config.test.ts @@ -232,13 +232,21 @@ test('Variants in CSS overwrite variants from plugins', async ({ expect }) => { }) describe('theme callbacks', () => { - test('line-heights defined in fontSize tuples in a config take precedence over `@theme default` CSS values', async ({ + test('tuple values from the config overwrite `@theme default` tuple-ish values from the CSS theme', async ({ expect, }) => { let input = css` @theme default { - --font-size-xl: 1.25rem; - --font-size-xl--line-height: 1.5rem; + --font-size-base: 0rem; + --font-size-base--line-height: 1rem; + --font-size-md: 0rem; + --font-size-md--line-height: 1rem; + --font-size-xl: 0rem; + --font-size-xl--line-height: 1rem; + } + @theme { + --font-size-base: 100rem; + --font-size-md--line-height: 101rem; } @tailwind utilities; @config "./config.js"; @@ -249,20 +257,76 @@ describe('theme callbacks', () => { theme: { extend: { fontSize: { - xl: ['1.25rem', { lineHeight: '1.75rem' }], + base: ['200rem', { lineHeight: '201rem' }], + md: ['200rem', { lineHeight: '201rem' }], + xl: ['200rem', { lineHeight: '201rem' }], }, + + // Direct access lineHeight: ({ theme }) => ({ + base: theme('fontSize.base[1].lineHeight'), + md: theme('fontSize.md[1].lineHeight'), xl: theme('fontSize.xl[1].lineHeight'), }), + + // Tuple access + typography: ({ theme }) => ({ + '[class~=lead-base]': { + fontSize: theme('fontSize.base')[0], + ...theme('fontSize.base')[1], + }, + '[class~=lead-md]': { + fontSize: theme('fontSize.md')[0], + ...theme('fontSize.md')[1], + }, + '[class~=lead-xl]': { + fontSize: theme('fontSize.xl')[0], + ...theme('fontSize.xl')[1], + }, + }), }, }, + + plugins: [ + plugin(function ({ addUtilities, theme }) { + addUtilities({ + '.prose': { + ...theme('typography'), + }, + }) + }), + ], }), }) - expect(compiler.build(['leading-xl'])).toMatchInlineSnapshot(` + + expect(compiler.build(['leading-base', 'leading-md', 'leading-xl', 'prose'])) + .toMatchInlineSnapshot(` ":root { + --font-size-base: 100rem; + --font-size-md--line-height: 101rem; + } + .prose { + [class~=lead-base] { + font-size: 100rem; + line-height: 201rem; + } + [class~=lead-md] { + font-size: 200rem; + line-height: 101rem; + } + [class~=lead-xl] { + font-size: 200rem; + line-height: 201rem; + } + } + .leading-base { + line-height: 201rem; + } + .leading-md { + line-height: 101rem; } .leading-xl { - line-height: 1.75rem; + line-height: 201rem; } " `) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index fcb35662ce41..d7fdace292df 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -76,6 +76,25 @@ export function createThemeFn( return configValueCopy } + // Handle the tuple case + if (Array.isArray(cssValue) && Array.isArray(options) && Array.isArray(configValue)) { + let base = cssValue[0] + let extra = cssValue[1] + + // Values from the config overwrite any default values from the CSS theme + if (options[0] & ThemeOptions.DEFAULT) { + base = configValue[0] ?? base + } + + for (let key of Object.keys(extra)) { + if (options[1][key] & ThemeOptions.DEFAULT) { + extra[key] = configValue[1][key] ?? extra[key] + } + } + + return [base, extra] + } + // Values from CSS take precedence over values from the config return cssValue ?? configValue })() @@ -125,17 +144,22 @@ function readFromCss( .join('-') let map = new Map() - let nested = new DefaultMap>(() => new Map()) + let nested = new DefaultMap>( + () => new Map(), + ) let ns = theme.namespace(`--${themeKey}` as any) if (ns.size === 0) { return [null, ThemeOptions.NONE] } + let options = new Map() + for (let [key, value] of ns) { // Non-nested values can be set directly if (!key || !key.includes('--')) { map.set(key, value) + options.set(key, theme.getOptions(!key ? `--${themeKey}` : `--${themeKey}-${key}`)) continue } @@ -148,25 +172,32 @@ function readFromCss( // Make `nestedKey` camel case: nestedKey = nestedKey.replace(/-([a-z])/g, (_, a) => a.toUpperCase()) - nested.get(mainKey === '' ? null : mainKey).set(nestedKey, value) + nested + .get(mainKey === '' ? null : mainKey) + .set(nestedKey, [value, theme.getOptions(`--${themeKey}${key}`)]) } + let baseOptions = theme.getOptions(`--${themeKey}`) for (let [key, extra] of nested) { let value = map.get(key) if (typeof value !== 'string') continue - map.set(key, [value, Object.fromEntries(extra)]) + let extraObj: Record = {} + let extraOptionsObj: Record = {} + + for (let [nestedKey, [nestedValue, nestedOptions]] of extra) { + extraObj[nestedKey] = nestedValue + extraOptionsObj[nestedKey] = nestedOptions + } + + map.set(key, [value, extraObj]) + options.set(key, [baseOptions, extraOptionsObj]) } // We have to turn the map into object-like structure for v3 compatibility let obj: Record = {} + let optionsObj: Record = {} let useNestedObjects = false // paths.some((path) => nestedKeys.has(path)) - let options = Object.fromEntries( - Array.from(ns.entries()).map(([key]) => { - let fullKey = key === null ? `--${themeKey}` : `--${themeKey}-${key}` - return [key ?? 'DEFAULT', theme.getOptions(fullKey)] - }), - ) for (let [key, value] of map) { key = key ?? 'DEFAULT' @@ -184,26 +215,30 @@ function readFromCss( set(obj, path, value) } + for (let [key, value] of options) { + set(optionsObj, [key ?? 'DEFAULT'], value) + } + // If the request looked like `theme('animation.DEFAULT')` it would have been // turned into a lookup for `--animation-*` so we should extract the value for // the `DEFAULT` key from the list of possible values. If there is no // `DEFAULT` in the list, there is no match so return `null`. if (path[path.length - 1] === 'DEFAULT') { - return [(obj?.DEFAULT ?? null) as any, options.DEFAULT ?? 0] as const + return [(obj?.DEFAULT ?? null) as any, optionsObj.DEFAULT ?? 0] as const } // The request looked like `theme('animation.spin')` and was turned into a // lookup for `--animation-spin-*` which had only one entry which means it // should be returned directly. if ('DEFAULT' in obj && Object.keys(obj).length === 1) { - return [obj.DEFAULT as string, options.DEFAULT ?? 0] as const + return [obj.DEFAULT as any, optionsObj.DEFAULT ?? 0] as const } // Attach the CSS values to the object for later use. This object could be // mutated by the user so we want to keep the original CSS values around. - obj.__CSS_VALUES__ = options + obj.__CSS_VALUES__ = optionsObj - return [obj, options] as const + return [obj, optionsObj] as const } function get(obj: any, path: string[]) { From 5f2851a5fd3f67961e3615158c1954b38a36582c Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 6 Sep 2024 14:55:52 -0400 Subject: [PATCH 09/12] Simplify code --- .../tailwindcss/src/compat/plugin-functions.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index d7fdace292df..ec4765f7a32c 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -197,22 +197,9 @@ function readFromCss( // We have to turn the map into object-like structure for v3 compatibility let obj: Record = {} let optionsObj: Record = {} - let useNestedObjects = false // paths.some((path) => nestedKeys.has(path)) for (let [key, value] of map) { - key = key ?? 'DEFAULT' - - let path: string[] = [] - let splitIndex = key.indexOf('-') - - if (useNestedObjects && splitIndex !== -1) { - path.push(key.slice(0, splitIndex)) - path.push(key.slice(splitIndex + 1)) - } else { - path.push(key) - } - - set(obj, path, value) + set(obj, [key ?? 'DEFAULT'], value) } for (let [key, value] of options) { From 14555eae28fa80a11519929518ee819ea8fbdf68 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 6 Sep 2024 15:08:13 -0400 Subject: [PATCH 10/12] Cleanup code --- packages/tailwindcss/src/compat/plugin-functions.ts | 4 ++-- packages/tailwindcss/src/theme.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index ec4765f7a32c..416da6eb3404 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -211,14 +211,14 @@ function readFromCss( // the `DEFAULT` key from the list of possible values. If there is no // `DEFAULT` in the list, there is no match so return `null`. if (path[path.length - 1] === 'DEFAULT') { - return [(obj?.DEFAULT ?? null) as any, optionsObj.DEFAULT ?? 0] as const + return [(obj?.DEFAULT ?? null) as any, optionsObj.DEFAULT ?? ThemeOptions.NONE] as const } // The request looked like `theme('animation.spin')` and was turned into a // lookup for `--animation-spin-*` which had only one entry which means it // should be returned directly. if ('DEFAULT' in obj && Object.keys(obj).length === 1) { - return [obj.DEFAULT as any, optionsObj.DEFAULT ?? 0] as const + return [obj.DEFAULT as any, optionsObj.DEFAULT ?? ThemeOptions.NONE] as const } // Attach the CSS values to the object for later use. This object could be diff --git a/packages/tailwindcss/src/theme.ts b/packages/tailwindcss/src/theme.ts index 775c06836300..b545462efb76 100644 --- a/packages/tailwindcss/src/theme.ts +++ b/packages/tailwindcss/src/theme.ts @@ -66,11 +66,11 @@ export class Theme { } hasDefault(key: string): boolean { - return ((this.values.get(key)?.options ?? 0) & ThemeOptions.DEFAULT) === ThemeOptions.DEFAULT + return (this.getOptions(key) & ThemeOptions.DEFAULT) === ThemeOptions.DEFAULT } getOptions(key: string) { - return this.values.get(key)?.options ?? 0 + return this.values.get(key)?.options ?? ThemeOptions.NONE } entries() { From 29dfe92ea5ff83e7eff7d886b344fa35e4f680fe Mon Sep 17 00:00:00 2001 From: Adam Wathan Date: Fri, 6 Sep 2024 15:48:56 -0400 Subject: [PATCH 11/12] Apply suggestions from code review --- packages/tailwindcss/src/compat/plugin-functions.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/tailwindcss/src/compat/plugin-functions.ts b/packages/tailwindcss/src/compat/plugin-functions.ts index 416da6eb3404..4523fb8dc473 100644 --- a/packages/tailwindcss/src/compat/plugin-functions.ts +++ b/packages/tailwindcss/src/compat/plugin-functions.ts @@ -60,8 +60,8 @@ export function createThemeFn( if (key === '__CSS_VALUES__') continue // If the value is coming from a default source (`@theme default`), - // then we keep the value from the js config (which is also a - // default source, but wins from the built in defaults). + // then we keep the value from the JS config (which is also a + // default source, but wins over the built-in defaults). if ( configValue?.__CSS_VALUES__?.[key] & ThemeOptions.DEFAULT && get(configValueCopy, key.split('-')) !== undefined From ffc9fedbebd31367d5aa84c90128d744d900994a Mon Sep 17 00:00:00 2001 From: Adam Wathan <4323180+adamwathan@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:52:32 -0400 Subject: [PATCH 12/12] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87256839c2be..209d999c79cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure there is always CLI feedback on save even when no new classes were found ([#14351](https://github.com/tailwindlabs/tailwindcss/pull/14351)) - Properly resolve `theme('someKey.DEFAULT')` when all `--some-key-*` keys have a suffix ([#14354](https://github.com/tailwindlabs/tailwindcss/pull/14354)) +- Make sure tuple theme values in JS configs take precedence over `@theme default` values ([#14359](https://github.com/tailwindlabs/tailwindcss/pull/14359)) ## [4.0.0-alpha.23] - 2024-09-05