From ebaff18a9f089ed2b592e6a00147a6e4e2545d74 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Tue, 17 Sep 2024 17:00:12 +0200 Subject: [PATCH] Fix stacking variant order when variants inside a group are treated as equal (#14431) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes an issue with the order of CSS when using stacked variants when two variants have the same order (as defined by the custom comperator function). ## The problem Take, for example, our breakpoint variants. Those are split into `max-*` variants and a group containing all `min-*` variants as well as the unprefixed static ones (e.g. `lg`, `sm`). We currently define a custom sort order for all breakpoints variants that will compare their order based on the resolved value provided. So if you define `--breakpoint-sm: 100px` and `--breakpoint-lg: 200px`, we first check if both breakpoints have the same unit and then we rank based on the numerical value, making `sm` appear before `lg`. But since the `min-*` variant and the `sm` variant share the same group, this also means that `min-sm` and `sm` as well as `min-lg` and `lg` will always have the same order (which makes sense—they also have the exact same CSS they generate!) The issue now arises when you use these together with variant stacking. So, say you want to stack the two variants `max-lg:min-sm`. We always want stacked variants to appear _after_ their non-stacked individual parts (since they are more specific). To do this right now, we generate a bitfield based on the variant order. If you have four variants like this: | Order | Variant | | ------------- | ------------- | | 0 | `max-lg` | | 1 | `max-sm` | | 2 | `min-sm` | | 3 | `min-lg` | We will assign one bit for each used variant starting from the lowest bit, so for the stack `max-lg:min-sm` we will set the bitfield to `0101` and those for the individual variants would result in `0100` (for `min-sm`) and `0001` (for `max-lg`). We then convert this bitfield to a number and order based on that number. This ensures that the stack always sorts higher. The issue now arises from the fact that the variant order also include the unprefixed variants for a breakpoint. So in our case of `lg` and `sm`, the full list would look like this: | Order | Variant | | ------------- | ------------- | | 0 | `max-lg` | | 1 | `max-sm` | | 2 | `min-sm` | | 3 | `sm` | | 4 | `min-lg` | | 5 | `lg` | This logic now breaks when you start to compute a stack for something like `max-lg:min-lg` _while also using the `lg` utility: | Stack | Bitmap | Integer Value | | ------------- | ------------- | ------------- | | `max-lg:min-lg` | `010001` | 17 | | `lg` | `100000` | 18 | As you can see here, the sole `lg` variant will now sort higher than the compound of `max-lg:min-lg`. That's not something we want! ## Proposed solution To fix this, we need to encode the information of _same_ variant order somehow. A single array like the example above is not sufficient for this, since it will remove the information of the similar sort order. Instead, we now computed a list of nested arrays for the order lookup that will combine variants of similar values (while keeping the order the same). So from the 6 item array above, we now have the following nested array: | Order | Variant | | ------------- | ------------- | | 0 | [`max-lg`] | | 1 | [`max-sm`] | | 2 | [`min-sm`, `sm`] | | 3 | [`min-lg`, `lg`] | When we use the first layer index for the bitfield, we can now see how this solves the issue: | Stack | Bitmap | Integer Value | | ------------- | ------------- | ------------- | | `max-lg:min-lg` | `1001` | 9 | | `lg` | `1000` | 8 | That's pretty-much it! There are a few other changes in this PR that mostly handles with a small regression by this change where now, named `group` variants and unnamed `group` variants would now have the same order (something that was undefined behavior before). --------- Co-authored-by: Adam Wathan --- CHANGELOG.md | 1 + .../tailwindcss/src/compat/plugin-api.test.ts | 52 ++++++------- packages/tailwindcss/src/compat/plugin-api.ts | 7 +- packages/tailwindcss/src/compile.ts | 7 +- packages/tailwindcss/src/design-system.ts | 35 +++++++-- packages/tailwindcss/src/variants.test.ts | 73 ++++++++++++++++--- packages/tailwindcss/src/variants.ts | 17 ++++- 7 files changed, 137 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b14b8cef..a0b7719c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Support `borderRadius.*` as an alias for `--radius-*` when using dot notation inside the `theme()` function ([#14436](https://github.com/tailwindlabs/tailwindcss/pull/14436)) +- Ensure individual variants from groups are always sorted earlier than stacked variants from the same groups ([#14431](https://github.com/tailwindlabs/tailwindcss/pull/14431)) ## [4.0.0-alpha.24] - 2024-09-11 diff --git a/packages/tailwindcss/src/compat/plugin-api.test.ts b/packages/tailwindcss/src/compat/plugin-api.test.ts index 1a11882f3..8fc6bf63b 100644 --- a/packages/tailwindcss/src/compat/plugin-api.test.ts +++ b/packages/tailwindcss/src/compat/plugin-api.test.ts @@ -1404,13 +1404,13 @@ describe('matchVariant', () => { expect(optimizeCss(compiled).trim()).toMatchInlineSnapshot(` "@layer utilities { - .potato-yellow .potato-\\[yellow\\]\\:underline { - text-decoration-line: underline; - } - .potato-baked .potato-\\[baked\\]\\:flex { display: flex; } + + .potato-yellow .potato-\\[yellow\\]\\:underline { + text-decoration-line: underline; + } }" `) }) @@ -1435,17 +1435,17 @@ describe('matchVariant', () => { expect(optimizeCss(compiled).trim()).toMatchInlineSnapshot(` "@layer utilities { - @media (potato: yellow) { - .potato-\\[yellow\\]\\:underline { - text-decoration-line: underline; - } - } - @media (potato: baked) { .potato-\\[baked\\]\\:flex { display: flex; } } + + @media (potato: yellow) { + .potato-\\[yellow\\]\\:underline { + text-decoration-line: underline; + } + } }" `) }) @@ -1473,14 +1473,6 @@ describe('matchVariant', () => { expect(optimizeCss(compiled).trim()).toMatchInlineSnapshot(` "@layer utilities { - @media (potato: yellow) { - @supports (font: bold) { - .potato-\\[yellow\\]\\:underline:large-potato { - text-decoration-line: underline; - } - } - } - @media (potato: baked) { @supports (font: bold) { .potato-\\[baked\\]\\:flex:large-potato { @@ -1488,6 +1480,14 @@ describe('matchVariant', () => { } } } + + @media (potato: yellow) { + @supports (font: bold) { + .potato-\\[yellow\\]\\:underline:large-potato { + text-decoration-line: underline; + } + } + } }" `) }) @@ -1541,10 +1541,10 @@ describe('matchVariant', () => { return ({ matchVariant }: PluginAPI) => { matchVariant('alphabet', (side) => `&${side}`, { values: { - a: '[data-value="a"]', - b: '[data-value="b"]', - c: '[data-value="c"]', - d: '[data-value="d"]', + d: '[data-order="1"]', + a: '[data-order="2"]', + c: '[data-order="3"]', + b: '[data-order="4"]', }, }) } @@ -1560,19 +1560,19 @@ describe('matchVariant', () => { expect(optimizeCss(compiled).trim()).toMatchInlineSnapshot(` "@layer utilities { - .alphabet-a\\:underline[data-value="a"] { + .alphabet-d\\:underline[data-order="1"] { text-decoration-line: underline; } - .alphabet-b\\:underline[data-value="b"] { + .alphabet-a\\:underline[data-order="2"] { text-decoration-line: underline; } - .alphabet-c\\:underline[data-value="c"] { + .alphabet-c\\:underline[data-order="3"] { text-decoration-line: underline; } - .alphabet-d\\:underline[data-value="d"] { + .alphabet-b\\:underline[data-order="4"] { text-decoration-line: underline; } }" diff --git a/packages/tailwindcss/src/compat/plugin-api.ts b/packages/tailwindcss/src/compat/plugin-api.ts index 69fafbf1b..f6d8d91a5 100644 --- a/packages/tailwindcss/src/compat/plugin-api.ts +++ b/packages/tailwindcss/src/compat/plugin-api.ts @@ -150,10 +150,10 @@ export function buildPluginApi( return 0 } - if (options && typeof options.sort === 'function') { - let aValue = options.values?.[a.value.value] ?? a.value.value - let zValue = options.values?.[z.value.value] ?? z.value.value + let aValue = options?.values?.[a.value.value] ?? a.value.value + let zValue = options?.values?.[z.value.value] ?? z.value.value + if (options && typeof options.sort === 'function') { return options.sort( { value: aValue, modifier: a.modifier?.value ?? null }, { value: zValue, modifier: z.modifier?.value ?? null }, @@ -163,6 +163,7 @@ export function buildPluginApi( let aOrder = defaultOptionKeys.indexOf(a.value.value) let zOrder = defaultOptionKeys.indexOf(z.value.value) + if (aOrder - zOrder === 0) return aValue < zValue ? -1 : 1 return aOrder - zOrder }, ) diff --git a/packages/tailwindcss/src/compile.ts b/packages/tailwindcss/src/compile.ts index 6450a288a..931d16ecd 100644 --- a/packages/tailwindcss/src/compile.ts +++ b/packages/tailwindcss/src/compile.ts @@ -30,10 +30,7 @@ export function compileCandidates( matches.set(rawCandidate, candidates) } - // Sort the variants - let variants = designSystem.getUsedVariants().sort((a, z) => { - return designSystem.variants.compare(a, z) - }) + let variantOrderMap = designSystem.getVariantOrder() // Create the AST for (let [rawCandidate, candidates] of matches) { @@ -51,7 +48,7 @@ export function compileCandidates( // variants used. let variantOrder = 0n for (let variant of candidate.variants) { - variantOrder |= 1n << BigInt(variants.indexOf(variant)) + variantOrder |= 1n << BigInt(variantOrderMap.get(variant)!) } nodeSorting.set(node, { diff --git a/packages/tailwindcss/src/design-system.ts b/packages/tailwindcss/src/design-system.ts index 0c317dbfd..0bac8102d 100644 --- a/packages/tailwindcss/src/design-system.ts +++ b/packages/tailwindcss/src/design-system.ts @@ -1,5 +1,5 @@ import { toCss } from './ast' -import { parseCandidate, parseVariant, type Candidate } from './candidate' +import { parseCandidate, parseVariant, type Candidate, type Variant } from './candidate' import { compileAstNodes, compileCandidates } from './compile' import { getClassList, getVariants, type ClassEntry, type VariantEntry } from './intellisense' import { getClassOrder } from './sort' @@ -13,17 +13,19 @@ export type DesignSystem = { utilities: Utilities variants: Variants - candidatesToCss(classes: string[]): (string | null)[] getClassOrder(classes: string[]): [string, bigint | null][] getClassList(): ClassEntry[] getVariants(): VariantEntry[] parseCandidate(candidate: string): Candidate[] - parseVariant(variant: string): ReturnType + parseVariant(variant: string): Variant | null compileAstNodes(candidate: Candidate): ReturnType - getUsedVariants(): ReturnType[] + getVariantOrder(): Map resolveThemeValue(path: string): string | undefined + + // Used by IntelliSense + candidatesToCss(classes: string[]): (string | null)[] } export function buildDesignSystem(theme: Theme): DesignSystem { @@ -77,8 +79,29 @@ export function buildDesignSystem(theme: Theme): DesignSystem { compileAstNodes(candidate: Candidate) { return compiledAstNodes.get(candidate) }, - getUsedVariants() { - return Array.from(parsedVariants.values()) + getVariantOrder() { + let variants = Array.from(parsedVariants.values()) + variants.sort((a, z) => this.variants.compare(a, z)) + + let order = new Map() + let prevVariant: Variant | undefined = undefined + let index: number = 0 + + for (let variant of variants) { + if (variant === null) { + continue + } + // This variant is not the same order as the previous one + // so it goes into a new group + if (prevVariant !== undefined && this.variants.compare(prevVariant, variant) !== 0) { + index++ + } + + order.set(variant, index) + prevVariant = variant + } + + return order }, resolveThemeValue(path: `${ThemeKey}` | `${ThemeKey}${string}`) { diff --git a/packages/tailwindcss/src/variants.test.ts b/packages/tailwindcss/src/variants.test.ts index 5b0b1855e..40042045c 100644 --- a/packages/tailwindcss/src/variants.test.ts +++ b/packages/tailwindcss/src/variants.test.ts @@ -1265,6 +1265,57 @@ test('sorting stacked min-* and max-* variants', async () => { `) }) +test('stacked min-* and max-* variants should come after unprefixed variants', async () => { + expect( + await compileCss( + css` + @theme { + /* Explicitly ordered in a strange way */ + --breakpoint-sm: 640px; + --breakpoint-lg: 1024px; + --breakpoint-md: 768px; + } + @tailwind utilities; + `, + ['sm:flex', 'min-sm:max-lg:flex', 'md:flex', 'min-md:max-lg:flex'], + ), + ).toMatchInlineSnapshot(` + ":root { + --breakpoint-sm: 640px; + --breakpoint-lg: 1024px; + --breakpoint-md: 768px; + } + + @media (width >= 640px) { + .sm\\:flex { + display: flex; + } + } + + @media (width >= 640px) { + @media (width < 1024px) { + .min-sm\\:max-lg\\:flex { + display: flex; + } + } + } + + @media (width >= 768px) { + .md\\:flex { + display: flex; + } + } + + @media (width >= 768px) { + @media (width < 1024px) { + .min-md\\:max-lg\\:flex { + display: flex; + } + } + }" + `) +}) + test('min, max and unprefixed breakpoints', async () => { expect( await compileCss( @@ -2246,14 +2297,14 @@ test('container queries', async () => { --width-lg: 1024px; } - @container (width < 1024px) { - .\\@max-lg\\:flex { + @container name (width < 1024px) { + .\\@max-lg\\/name\\:flex { display: flex; } } - @container name (width < 1024px) { - .\\@max-lg\\/name\\:flex { + @container (width < 1024px) { + .\\@max-lg\\:flex { display: flex; } } @@ -2294,12 +2345,6 @@ test('container queries', async () => { } } - @container (width >= 1024px) { - .\\@lg\\:flex { - display: flex; - } - } - @container name (width >= 1024px) { .\\@lg\\/name\\:flex { display: flex; @@ -2307,7 +2352,7 @@ test('container queries', async () => { } @container (width >= 1024px) { - .\\@min-lg\\:flex { + .\\@lg\\:flex { display: flex; } } @@ -2316,6 +2361,12 @@ test('container queries', async () => { .\\@min-lg\\/name\\:flex { display: flex; } + } + + @container (width >= 1024px) { + .\\@min-lg\\:flex { + display: flex; + } }" `) }) diff --git a/packages/tailwindcss/src/variants.ts b/packages/tailwindcss/src/variants.ts index 7811043ad..2bb9def3d 100644 --- a/packages/tailwindcss/src/variants.ts +++ b/packages/tailwindcss/src/variants.ts @@ -117,13 +117,22 @@ export class Variants { if (orderedByVariant !== 0) return orderedByVariant if (a.kind === 'compound' && z.kind === 'compound') { - return this.compare(a.variant, z.variant) + let order = this.compare(a.variant, z.variant) + if (order === 0) { + if (a.modifier && z.modifier) { + return a.modifier.value < z.modifier.value ? -1 : 1 + } else if (a.modifier) { + return 1 + } else if (z.modifier) { + return -1 + } + } + return order } let compareFn = this.compareFns.get(aOrder) - if (compareFn === undefined) return 0 - - return compareFn(a, z) || (a.root < z.root ? -1 : 1) + if (compareFn === undefined) return a.root < z.root ? -1 : 1 + return compareFn(a, z) } keys() {