Skip to content
Prev Previous commit
Next Next commit
Make sure matchVariant sorts values deterministically
  • Loading branch information
thecrypticace committed Oct 30, 2024
commit 1129eb1e04e125972db2e699ff026ad0df675839
22 changes: 13 additions & 9 deletions packages/tailwindcss/src/compat/plugin-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RobinMalfait I'm not 100% sold that this is the "right" fix. Thoughts?


let aValue = options?.values?.[aValueKey] ?? aValueKey
let zValue = options?.values?.[zValueKey] ?? zValueKey

if (options && typeof options.sort === 'function') {
return options.sort(
Expand All @@ -170,11 +170,15 @@ 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) return aOrder - zOrder

if (aOrder - zOrder === 0) return aValue < zValue ? -1 : 1
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
},
)
},
Expand Down
51 changes: 51 additions & 0 deletions packages/tailwindcss/src/variants.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +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'

Expand Down Expand Up @@ -3345,6 +3346,56 @@ test('variants with the same root are sorted deterministically', async () => {
}
})

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'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a couple of arbitrary values here as well?


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

test.each([
// These are style rules
[['.foo'], Compounds.StyleRules],
Expand Down