Fix stacking variant order when variants inside a group are treated as equal (#14431)

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 <adam.wathan@gmail.com>
This commit is contained in:
Philipp Spiess 2024-09-17 17:00:12 +02:00 • committed by GitHub
parent c7cbdb8472
commit ebaff18a9f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 137 additions and 55 deletions

View file

@ -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

View file

@ -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;
}
}"

View file

@ -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
},
)

View file

@ -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, {

View file

@ -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<typeof parseVariant>
parseVariant(variant: string): Variant | null
compileAstNodes(candidate: Candidate): ReturnType<typeof compileAstNodes>
getUsedVariants(): ReturnType<typeof parseVariant>[]
getVariantOrder(): Map<Variant, number>
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<Variant, number>()
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}`) {

View file

@ -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;
}
}"
`)
})

View file

@ -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() {