archive-gh-me/tailwindcss
1
0
Fork 0
mirror of https://github.com/tailwindlabs/tailwindcss.git synced 2025-12-08 21:36:08 +00:00
Code Issues Projects Releases Wiki Activity

Fix non-deterministic variant sorting (#14835)

Browse Source 
Right now our variant sorting is sensitive to the authoring order of
classes or the order in which we scan files because we weren't comparing
variant values when sorting.

This PR addresses this by:
- "default" values in variants appear first
- Then any named values
- Then any arbitrary values
- Finally, compare the values themselves when all else is equal

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
Co-authored-by: Adam Wathan <4323180+adamwathan@users.noreply.github.com>
Co-authored-by: Adam Wathan <adam.wathan@gmail.com>
This commit is contained in:
Jordan Pittman 2024-10-31 10:39:56 -04:00 committed by GitHub
parent 0b5736f1b2
commit 7cc2d89073
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 276 additions and 111 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Detect classes in new files when using `@tailwindcss/postcss` ([#14829](https://github.com/tailwindlabs/tailwindcss/pull/14829))
- Fix crash when using `@source` containing `..` ([#14831](https://github.com/tailwindlabs/tailwindcss/pull/14831))
- Ensure instances of the same variant with different values are always sorted deterministically (e.g. `data-focus:flex` and `data-active:flex`) ([#14835](https://github.com/tailwindlabs/tailwindcss/pull/14835))
- _Upgrade (experimental)_: Install `@tailwindcss/postcss` next to `tailwindcss` ([#14830](https://github.com/tailwindlabs/tailwindcss/pull/14830))
- _Upgrade (experimental)_: Remove whitespace around `,` separator when print arbitrary values ([#14838](https://github.com/tailwindlabs/tailwindcss/pull/14838))

12
packages/tailwindcss/src/compat/config.test.ts
View File

@ -1109,16 +1109,16 @@ test('creates variants for `data`, `supports`, and `aria` theme options at the s
'print:flex',
]),
).toMatchInlineSnapshot(`
".aria-polite\\:underline {
&[aria-live="polite"] {
text-decoration-line: underline;
}
}
.aria-hidden\\:flex {
".aria-hidden\\:flex {
&[aria-hidden="true"] {
display: flex;
}
}
.aria-polite\\:underline {
&[aria-live="polite"] {
text-decoration-line: underline;
}
}
.data-checked\\:underline {
&[data-ui~="checked"] {
text-decoration-line: underline;

26
packages/tailwindcss/src/compat/plugin-api.ts
View File

@ -156,12 +156,12 @@ export function buildPluginApi(
if (a.kind !== 'functional' || z.kind !== 'functional') {
return 0
}
if (!a.value || !z.value) {
return 0
}
let aValue = options?.values?.[a.value.value] ?? a.value.value
let zValue = options?.values?.[z.value.value] ?? z.value.value
let aValueKey = a.value ? a.value.value : 'DEFAULT'
let zValueKey = z.value ? z.value.value : 'DEFAULT'
let aValue = options?.values?.[aValueKey] ?? aValueKey
let zValue = options?.values?.[zValueKey] ?? zValueKey
if (options && typeof options.sort === 'function') {
return options.sort(
@ -170,11 +170,19 @@ export function buildPluginApi(
)
}
let aOrder = defaultOptionKeys.indexOf(a.value.value)
let zOrder = defaultOptionKeys.indexOf(z.value.value)
let aOrder = defaultOptionKeys.indexOf(aValueKey)
let zOrder = defaultOptionKeys.indexOf(zValueKey)
if (aOrder - zOrder === 0) return aValue < zValue ? -1 : 1
return aOrder - zOrder
// Sort arbitrary values after configured values
aOrder = aOrder === -1 ? defaultOptionKeys.length : aOrder
zOrder = zOrder === -1 ? defaultOptionKeys.length : zOrder
if (aOrder !== zOrder) return aOrder - zOrder
// SAFETY: The values don't need to be checked for equality as they
// are guaranteed to be unique since we sort a list of de-duped
// variants and different (valid) variants cannot produce the same AST.
return aValue < zValue ? -1 : 1
},
)
},

295
packages/tailwindcss/src/variants.test.ts
View File

@ -1,4 +1,6 @@
import dedent from 'dedent'
import { expect, test } from 'vitest'
import createPlugin from './plugin'
import { compileCss, run } from './test-utils/run'
import { Compounds, compoundsForSelectors } from './variants'
@ -1623,14 +1625,20 @@ test('supports', async () => {
}
}
@supports (display: grid) {
.supports-\\[display\\:grid\\]\\:flex {
@supports (display: grid) and font-format(opentype) {
.supports-\\[\\(display\\:grid\\)_and_font-format\\(opentype\\)\\]\\:grid {
display: grid;
}
}
@supports (--test: var(--tw)) {
.supports-\\[--test\\]\\:flex {
display: flex;
}
}
@supports selector(A > B) {
.supports-\\[selector\\(A_\\>_B\\)\\]\\:flex {
@supports (display: grid) {
.supports-\\[display\\:grid\\]\\:flex {
display: flex;
}
}
@ -1641,26 +1649,20 @@ test('supports', async () => {
}
}
@supports (display: grid) and font-format(opentype) {
.supports-\\[\\(display\\:grid\\)_and_font-format\\(opentype\\)\\]\\:grid {
display: grid;
}
}
@supports font-tech(color-COLRv1) {
.supports-\\[font-tech\\(color-COLRv1\\)\\]\\:flex {
display: flex;
}
}
@supports var(--test) {
.supports-\\[var\\(--test\\)\\]\\:flex {
@supports selector(A > B) {
.supports-\\[selector\\(A_\\>_B\\)\\]\\:flex {
display: flex;
}
}
@supports (--test: var(--tw)) {
.supports-\\[--test\\]\\:flex {
@supports var(--test) {
.supports-\\[var\\(--test\\)\\]\\:flex {
display: flex;
}
}"
@ -2399,24 +2401,12 @@ test('aria', async () => {
'peer-aria-[modal]:flex',
'peer-aria-checked:flex',
'peer-aria-[valuenow=1]:flex',
'peer-aria-[modal]/parent-name:flex',
'peer-aria-checked/parent-name:flex',
'peer-aria-[valuenow=1]/parent-name:flex',
'peer-aria-[modal]/sibling-name:flex',
'peer-aria-checked/sibling-name:flex',
'peer-aria-[valuenow=1]/sibling-name:flex',
]),
).toMatchInlineSnapshot(`
".group-aria-\\[modal\\]\\:flex:is(:where(.group)[aria-modal] *) {
display: flex;
}
.group-aria-checked\\:flex:is(:where(.group)[aria-checked="true"] *) {
display: flex;
}
.group-aria-\\[valuenow\\=1\\]\\:flex:is(:where(.group)[aria-valuenow="1"] *) {
display: flex;
}
.group-aria-\\[modal\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[aria-modal] *) {
".group-aria-checked\\:flex:is(:where(.group)[aria-checked="true"] *) {
display: flex;
}
@ -2424,11 +2414,19 @@ test('aria', async () => {
display: flex;
}
.group-aria-\\[valuenow\\=1\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[aria-valuenow="1"] *) {
.group-aria-\\[modal\\]\\:flex:is(:where(.group)[aria-modal] *) {
display: flex;
}
.peer-aria-\\[modal\\]\\:flex:is(:where(.peer)[aria-modal] ~ *) {
.group-aria-\\[modal\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[aria-modal] *) {
display: flex;
}
.group-aria-\\[valuenow\\=1\\]\\:flex:is(:where(.group)[aria-valuenow="1"] *) {
display: flex;
}
.group-aria-\\[valuenow\\=1\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[aria-valuenow="1"] *) {
display: flex;
}
@ -2436,19 +2434,23 @@ test('aria', async () => {
display: flex;
}
.peer-aria-checked\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[aria-checked="true"] ~ *) {
display: flex;
}
.peer-aria-\\[modal\\]\\:flex:is(:where(.peer)[aria-modal] ~ *) {
display: flex;
}
.peer-aria-\\[modal\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[aria-modal] ~ *) {
display: flex;
}
.peer-aria-\\[valuenow\\=1\\]\\:flex:is(:where(.peer)[aria-valuenow="1"] ~ *) {
display: flex;
}
.peer-aria-\\[modal\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[aria-modal] ~ *) {
display: flex;
}
.peer-aria-checked\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[aria-checked="true"] ~ *) {
display: flex;
}
.peer-aria-\\[valuenow\\=1\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[aria-valuenow="1"] ~ *) {
.peer-aria-\\[valuenow\\=1\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[aria-valuenow="1"] ~ *) {
display: flex;
}
@ -2460,11 +2462,11 @@ test('aria', async () => {
display: flex;
}
.aria-\\[valuenow\\=1\\]\\:flex[aria-valuenow="1"] {
.aria-\\[valuenow_\\=_\\"1\\"\\]\\:flex[aria-valuenow="1"] {
display: flex;
}
.aria-\\[valuenow_\\=_\\"1\\"\\]\\:flex[aria-valuenow="1"] {
.aria-\\[valuenow\\=1\\]\\:flex[aria-valuenow="1"] {
display: flex;
}"
`)
@ -2493,12 +2495,12 @@ test('data', async () => {
'group-data-[foo$=bar_baz_i]/parent-name:flex',
'peer-data-[disabled]:flex',
'peer-data-[disabled]/parent-name:flex',
'peer-data-[disabled]/sibling-name:flex',
'peer-data-[foo=1]:flex',
'peer-data-[foo=1]/parent-name:flex',
'peer-data-[foo=bar baz]/parent-name:flex',
"peer-data-[foo$='bar'_i]/parent-name:flex",
'peer-data-[foo$=bar_baz_i]/parent-name:flex',
'peer-data-[foo=1]/sibling-name:flex',
'peer-data-[foo=bar baz]/sibling-name:flex',
"peer-data-[foo$='bar'_i]/sibling-name:flex",
'peer-data-[foo$=bar_baz_i]/sibling-name:flex',
]),
).toMatchInlineSnapshot(`
".group-data-\\[disabled\\]\\:flex:is(:where(.group)[data-disabled] *) {
@ -2509,6 +2511,14 @@ test('data', async () => {
display: flex;
}
.group-data-\\[foo\\$\\=\\'bar\\'_i\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[data-foo$="bar" i] *) {
display: flex;
}
.group-data-\\[foo\\$\\=bar_baz_i\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[data-foo$="bar baz" i] *) {
display: flex;
}
.group-data-\\[foo\\=1\\]\\:flex:is(:where(.group)[data-foo="1"] *) {
display: flex;
}
@ -2521,19 +2531,19 @@ test('data', async () => {
display: flex;
}
.group-data-\\[foo\\$\\=\\'bar\\'_i\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[data-foo$="bar" i] *) {
display: flex;
}
.group-data-\\[foo\\$\\=bar_baz_i\\]\\/parent-name\\:flex:is(:where(.group\\/parent-name)[data-foo$="bar baz" i] *) {
display: flex;
}
.peer-data-\\[disabled\\]\\:flex:is(:where(.peer)[data-disabled] ~ *) {
display: flex;
}
.peer-data-\\[disabled\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[data-disabled] ~ *) {
.peer-data-\\[disabled\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[data-disabled] ~ *) {
display: flex;
}
.peer-data-\\[foo\\$\\=\\'bar\\'_i\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[data-foo$="bar" i] ~ *) {
display: flex;
}
.peer-data-\\[foo\\$\\=bar_baz_i\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[data-foo$="bar baz" i] ~ *) {
display: flex;
}
@ -2541,19 +2551,11 @@ test('data', async () => {
display: flex;
}
.peer-data-\\[foo\\=1\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[data-foo="1"] ~ *) {
.peer-data-\\[foo\\=1\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[data-foo="1"] ~ *) {
display: flex;
}
.peer-data-\\[foo\\=bar\\ baz\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[data-foo="bar baz"] ~ *) {
display: flex;
}
.peer-data-\\[foo\\$\\=\\'bar\\'_i\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[data-foo$="bar" i] ~ *) {
display: flex;
}
.peer-data-\\[foo\\$\\=bar_baz_i\\]\\/parent-name\\:flex:is(:where(.peer\\/parent-name)[data-foo$="bar baz" i] ~ *) {
.peer-data-\\[foo\\=bar\\ baz\\]\\/sibling-name\\:flex:is(:where(.peer\\/sibling-name)[data-foo="bar baz"] ~ *) {
display: flex;
}
@ -2561,7 +2563,19 @@ test('data', async () => {
display: flex;
}
.data-\\[potato\\=salad\\]\\:flex[data-potato="salad"] {
.data-\\[foo\\$\\=\\'bar\\'_i\\]\\:flex[data-foo$="bar" i] {
display: flex;
}
.data-\\[foo\\$\\=bar_baz_i\\]\\:flex[data-foo$="bar baz" i] {
display: flex;
}
.data-\\[foo\\=1\\]\\:flex[data-foo="1"] {
display: flex;
}
.data-\\[foo\\=bar_baz\\]\\:flex[data-foo="bar baz"] {
display: flex;
}
@ -2577,19 +2591,7 @@ test('data', async () => {
display: flex;
}
.data-\\[foo\\=1\\]\\:flex[data-foo="1"] {
display: flex;
}
.data-\\[foo\\=bar_baz\\]\\:flex[data-foo="bar baz"] {
display: flex;
}
.data-\\[foo\\$\\=\\'bar\\'_i\\]\\:flex[data-foo$="bar" i] {
display: flex;
}
.data-\\[foo\\$\\=bar_baz_i\\]\\:flex[data-foo$="bar baz" i] {
.data-\\[potato\\=salad\\]\\:flex[data-potato="salad"] {
display: flex;
}"
`)
@ -2881,6 +2883,7 @@ test('variant order', async () => {
'contrast-less:flex',
'contrast-more:flex',
'dark:flex',
'data-custom:flex',
'data-[custom=true]:flex',
'default:flex',
'disabled:flex',
@ -3123,10 +3126,6 @@ test('variant order', async () => {
display: flex;
}
.aria-\\[custom\\=true\\]\\:flex[aria-custom="true"] {
display: flex;
}
.aria-busy\\:flex[aria-busy="true"] {
display: flex;
}
@ -3163,6 +3162,14 @@ test('variant order', async () => {
display: flex;
}
.aria-\\[custom\\=true\\]\\:flex[aria-custom="true"] {
display: flex;
}
.data-custom\\:flex[data-custom] {
display: flex;
}
.data-\\[custom\\=true\\]\\:flex[data-custom="true"] {
display: flex;
}
@ -3285,6 +3292,124 @@ test('variant order', async () => {
`)
})
test('variants with the same root are sorted deterministically', async () => {
function permute(arr: string[]): string[][] {
if (arr.length <= 1) return [arr]
return arr.flatMap((item, i) =>
permute([...arr.slice(0, i), ...arr.slice(i + 1)]).map((permutation) => [
item,
...permutation,
]),
)
}
let classLists = permute([
'data-hover:flex',
'data-focus:flex',
'data-active:flex',
'data-[foo]:flex',
'data-[bar]:flex',
'data-[baz]:flex',
])
for (let classList of classLists) {
let output = await compileCss('@tailwind utilities;', classList)
expect(output.trim()).toEqual(
dedent(css`
.data-active\:flex[data-active] {
display: flex;
}
.data-focus\:flex[data-focus] {
display: flex;
}
.data-hover\:flex[data-hover] {
display: flex;
}
.data-\[bar\]\:flex[data-bar] {
display: flex;
}
.data-\[baz\]\:flex[data-baz] {
display: flex;
}
.data-\[foo\]\:flex[data-foo] {
display: flex;
}
`),
)
}
})
test('matchVariant sorts deterministically', async () => {
function permute(arr: string[]): string[][] {
if (arr.length <= 1) return [arr]
return arr.flatMap((item, i) =>
permute([...arr.slice(0, i), ...arr.slice(i + 1)]).map((permutation) => [
item,
...permutation,
]),
)
}
let classLists = permute([
'is-data:flex',
'is-data-foo:flex',
'is-data-bar:flex',
'is-data-[potato]:flex',
'is-data-[sandwich]:flex',
])
for (let classList of classLists) {
let output = await compileCss('@tailwind utilities; @plugin "./plugin.js";', classList, {
loadModule(id: string) {
return {
base: '/',
module: createPlugin(({ matchVariant }) => {
matchVariant('is-data', (value) => `&:is([data-${value}])`, {
values: {
DEFAULT: 'default',
foo: 'foo',
bar: 'bar',
},
})
}),
}
},
})
expect(output.trim()).toEqual(
dedent(css`
.is-data\:flex[data-default] {
display: flex;
}
.is-data-foo\:flex[data-foo] {
display: flex;
}
.is-data-bar\:flex[data-bar] {
display: flex;
}
.is-data-\[potato\]\:flex[data-potato] {
display: flex;
}
.is-data-\[sandwich\]\:flex[data-sandwich] {
display: flex;
}
`),
)
}
})
test.each([
// These are style rules
[['.foo'], Compounds.StyleRules],

53
packages/tailwindcss/src/variants.ts
View File

@ -198,6 +198,8 @@ export class Variants {
if (z === null) return 1
if (a.kind === 'arbitrary' && z.kind === 'arbitrary') {
// SAFETY: The selectors don't need to be checked for equality as they
// are guaranteed to be unique since we sort a list of de-duped variants
return a.selector < z.selector ? -1 : 1
} else if (a.kind === 'arbitrary') {
return 1
@ -213,21 +215,50 @@ export class Variants {
if (a.kind === 'compound' && z.kind === 'compound') {
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
}
if (order !== 0) return order
if (a.modifier && z.modifier) {
// SAFETY: The modifiers don't need to be checked for equality as they
// are guaranteed to be unique since we sort a list of de-duped variants
return a.modifier.value < z.modifier.value ? -1 : 1
} else if (a.modifier) {
return 1
} else if (z.modifier) {
return -1
} else {
return 0
}
return order
}
let compareFn = this.compareFns.get(aOrder)
if (compareFn === undefined) return a.root < z.root ? -1 : 1
return compareFn(a, z)
if (compareFn !== undefined) return compareFn(a, z)
if (a.root !== z.root) return a.root < z.root ? -1 : 1
// SAFETY: Variants `a` and `z` are both functional at this point. Static
// variants are de-duped by the `DefaultMap` and checked earlier.
let aValue = (a as Extract<Variant, { kind: 'functional' }>).value
let zValue = (z as Extract<Variant, { kind: 'functional' }>).value
// While no functional variant in core supports a "default" value the parser
// will see something like `data:flex` and still parse and store it as a
// functional variant even though it actually produces no CSS. This means
// that we need to handle the case where the value is `null` here. Even
// though _for valid utilities_ this never happens.
if (aValue === null) return -1
if (zValue === null) return 1
// Variants with arbitrary values should appear after any with named values
if (aValue.kind === 'arbitrary' && zValue.kind !== 'arbitrary') return 1
if (aValue.kind !== 'arbitrary' && zValue.kind === 'arbitrary') return -1
// SAFETY: The values don't need to be checked for equality as they are
// guaranteed to be unique since we sort a list of de-duped variants. The
// only way this could matter would be when two different variants parse to
// the same AST. That is only possible with arbitrary values when spaces are
// involved. e.g. `data-[a_b]:flex` and `data-[a ]:flex` but this is not a
// concern for us because spaces are not allowed in variant names.
return aValue.value < zValue.value ? -1 : 1
}
keys() {