Skip to content

Commit cea0838

Browse files
thecrypticacethecrypticace
authored
Sort arbitrary variants deterministically regardless of content order (#10016)
* Fix off-by-one error in variant sort mapping This didn’t actually have any negative effect because of how it was implemented. But it only happened to work right :D * Make arbitrary variant sorting deterministic * Update changelog * Use faster byte-order comparison We really only care that the order is _always_ the same. localeCompare is still locale dependent based on environment. This ensures it’s dependent on content only. * Fix CS
1 parent fde30c3 commit cea0838

File tree

5 files changed

+214
-14
lines changed

5 files changed

+214
-14
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2323
- Improve return value of `resolveConfig`, unwrap `ResolvableTo` ([#9972](https://github.com/tailwindlabs/tailwindcss/pull/9972))
2424
- Clip unbalanced brackets in arbitrary values ([#9973](https://github.com/tailwindlabs/tailwindcss/pull/9973))
2525
- Don’t reorder webkit scrollbar pseudo elements ([#9991](https://github.com/tailwindlabs/tailwindcss/pull/9991))
26+
- Deterministic sorting of arbitrary variants ([#10016](https://github.com/tailwindlabs/tailwindcss/pull/10016))
2627

2728
### Changed
2829

‎src/lib/offsets.js

+82-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// @ts-check
22

33
import bigSign from '../util/bigSign'
4+
import { remapBitfield } from './remap-bitfield.js'
45

56
/**
67
* @typedef {'base' | 'defaults' | 'components' | 'utilities' | 'variants' | 'user'} Layer
@@ -190,7 +191,7 @@ export class Offsets {
190191

191192
return {
192193
...this.create('variants'),
193-
variants: 1n << this.reservedVariantBits,
194+
variants: this.variantOffsets.get(variant),
194195
}
195196
}
196197

@@ -243,12 +244,68 @@ export class Offsets {
243244
return a.index - b.index
244245
}
245246

247+
/**
248+
* Arbitrary variants are recorded in the order they're encountered.
249+
* This means that the order is not stable between environments and sets of content files.
250+
*
251+
* In order to make the order stable, we need to remap the arbitrary variant offsets to
252+
* be in alphabetical order starting from the offset of the first arbitrary variant.
253+
*/
254+
recalculateVariantOffsets() {
255+
// Sort the variants by their name
256+
let variants = Array.from(this.variantOffsets.entries())
257+
.filter(([v]) => v.startsWith('['))
258+
.sort(([a], [z]) => fastCompare(a, z))
259+
260+
// Sort the list of offsets
261+
// This is not necessarily a discrete range of numbers which is why
262+
// we're using sort instead of creating a range from min/max
263+
let newOffsets = variants.map(([, offset]) => offset).sort((a, z) => bigSign(a - z))
264+
265+
// Create a map from the old offsets to the new offsets in the new sort order
266+
/** @type {[bigint, bigint][]} */
267+
let mapping = variants.map(([, oldOffset], i) => [oldOffset, newOffsets[i]])
268+
269+
// Remove any variants that will not move letting us skip
270+
// remapping if everything happens to be in order
271+
return mapping.filter(([a, z]) => a !== z)
272+
}
273+
274+
/**
275+
* @template T
276+
* @param {[RuleOffset, T][]} list
277+
* @returns {[RuleOffset, T][]}
278+
*/
279+
remapArbitraryVariantOffsets(list) {
280+
let mapping = this.recalculateVariantOffsets()
281+
282+
// No arbitrary variants? Nothing to do.
283+
// Everyhing already in order? Nothing to do.
284+
if (mapping.length === 0) {
285+
return list
286+
}
287+
288+
// Remap every variant offset in the list
289+
return list.map((item) => {
290+
let [offset, rule] = item
291+
292+
offset = {
293+
...offset,
294+
variants: remapBitfield(offset.variants, mapping),
295+
}
296+
297+
return [offset, rule]
298+
})
299+
}
300+
246301
/**
247302
* @template T
248303
* @param {[RuleOffset, T][]} list
249304
* @returns {[RuleOffset, T][]}
250305
*/
251306
sort(list) {
307+
list = this.remapArbitraryVariantOffsets(list)
308+
252309
return list.sort(([a], [b]) => bigSign(this.compare(a, b)))
253310
}
254311
}
@@ -268,3 +325,27 @@ function max(nums) {
268325

269326
return max
270327
}
328+
329+
/**
330+
* A fast ASCII order string comparison function.
331+
*
332+
* Using `.sort()` without a custom compare function is faster
333+
* But you can only use that if you're sorting an array of
334+
* only strings. If you're sorting strings inside objects
335+
* or arrays, you need must use a custom compare function.
336+
*
337+
* @param {string} a
338+
* @param {string} b
339+
*/
340+
function fastCompare(a, b) {
341+
let aLen = a.length
342+
let bLen = b.length
343+
let minLen = aLen < bLen ? aLen : bLen
344+
345+
for (let i = 0; i < minLen; i++) {
346+
let cmp = a.charCodeAt(i) - b.charCodeAt(i)
347+
if (cmp !== 0) return cmp
348+
}
349+
350+
return aLen - bLen
351+
}

‎src/lib/remap-bitfield.js

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// @ts-check
2+
3+
/**
4+
* We must remap all the old bits to new bits for each set variant
5+
* Only arbitrary variants are considered as those are the only
6+
* ones that need to be re-sorted at this time
7+
*
8+
* An iterated process that removes and sets individual bits simultaneously
9+
* will not work because we may have a new bit that is also a later old bit
10+
* This means that we would be removing a previously set bit which we don't
11+
* want to do
12+
*
13+
* For example (assume `bN` = `1<<N`)
14+
* Given the "total" mapping `[[b1, b3], [b2, b4], [b3, b1], [b4, b2]]`
15+
* The mapping is "total" because:
16+
* 1. Every input and output is accounted for
17+
* 2. All combinations are unique
18+
* 3. No one input maps to multiple outputs and vice versa
19+
* And, given an offset with all bits set:
20+
* V = b1 | b2 | b3 | b4
21+
*
22+
* Let's explore the issue with removing and setting bits simultaneously:
23+
* V & ~b1 | b3 = b2 | b3 | b4
24+
* V & ~b2 | b4 = b3 | b4
25+
* V & ~b3 | b1 = b1 | b4
26+
* V & ~b4 | b2 = b1 | b2
27+
*
28+
* As you can see, we end up with the wrong result.
29+
* This is because we're removing a bit that was previously set.
30+
* And, thus the final result is missing b3 and b4.
31+
*
32+
* Now, let's explore the issue with removing the bits first:
33+
* V & ~b1 = b2 | b3 | b4
34+
* V & ~b2 = b3 | b4
35+
* V & ~b3 = b4
36+
* V & ~b4 = 0
37+
*
38+
* And then setting the bits:
39+
* V | b3 = b3
40+
* V | b4 = b3 | b4
41+
* V | b1 = b1 | b3 | b4
42+
* V | b2 = b1 | b2 | b3 | b4
43+
*
44+
* We get the correct result because we're not removing any bits that were
45+
* previously set thus properly remapping the bits to the new order
46+
*
47+
* To collect this into a single operation that can be done simultaneously
48+
* we must first create a mask for the old bits that are set and a mask for
49+
* the new bits that are set. Then we can remove the old bits and set the new
50+
* bits simultaneously in a "single" operation like so:
51+
* OldMask = b1 | b2 | b3 | b4
52+
* NewMask = b3 | b4 | b1 | b2
53+
*
54+
* So this:
55+
* V & ~oldMask | newMask
56+
*
57+
* Expands to this:
58+
* V & ~b1 & ~b2 & ~b3 & ~b4 | b3 | b4 | b1 | b2
59+
*
60+
* Which becomes this:
61+
* b1 | b2 | b3 | b4
62+
*
63+
* Which is the correct result!
64+
*
65+
* @param {bigint} num
66+
* @param {[bigint, bigint][]} mapping
67+
*/
68+
export function remapBitfield(num, mapping) {
69+
// Create masks for the old and new bits that are set
70+
let oldMask = 0n
71+
let newMask = 0n
72+
for (let [oldBit, newBit] of mapping) {
73+
if (num & oldBit) {
74+
oldMask = oldMask | oldBit
75+
newMask = newMask | newBit
76+
}
77+
}
78+
79+
// Remove all old bits
80+
// Set all new bits
81+
return (num & ~oldMask) | newMask
82+
}

‎tests/arbitrary-variants.test.js

+44-8
Original file line numberDiff line numberDiff line change
@@ -517,11 +517,11 @@ test('allows attribute variants with quotes', () => {
517517
expect(result.css).toMatchFormattedCss(css`
518518
${defaults}
519519
520-
.\[\&\[data-test\=\'2\'\]\]\:underline[data-test="2"] {
520+
.\[\&\[data-test\=\"2\"\]\]\:underline[data-test='2'] {
521521
text-decoration-line: underline;
522522
}
523523
524-
.\[\&\[data-test\=\"2\"\]\]\:underline[data-test='2'] {
524+
.\[\&\[data-test\=\'2\'\]\]\:underline[data-test='2'] {
525525
text-decoration-line: underline;
526526
}
527527
`)
@@ -554,12 +554,12 @@ test('classes in arbitrary variants should not be prefixed', () => {
554554

555555
return run(input, config).then((result) => {
556556
expect(result.css).toMatchFormattedCss(css`
557-
.foo .\[\.foo_\&\]\:tw-text-red-400 {
557+
.\[\&_\.foo\]\:tw-text-red-400 .foo {
558558
--tw-text-opacity: 1;
559559
color: rgb(248 113 113 / var(--tw-text-opacity));
560560
}
561561
562-
.\[\&_\.foo\]\:tw-text-red-400 .foo {
562+
.foo .\[\.foo_\&\]\:tw-text-red-400 {
563563
--tw-text-opacity: 1;
564564
color: rgb(248 113 113 / var(--tw-text-opacity));
565565
}
@@ -593,22 +593,22 @@ test('classes in the same arbitrary variant should not be prefixed', () => {
593593

594594
return run(input, config).then((result) => {
595595
expect(result.css).toMatchFormattedCss(css`
596-
.foo .\[\.foo_\&\]\:tw-bg-white {
596+
.\[\&_\.foo\]\:tw-bg-white .foo {
597597
--tw-bg-opacity: 1;
598598
background-color: rgb(255 255 255 / var(--tw-bg-opacity));
599599
}
600600
601-
.foo .\[\.foo_\&\]\:tw-text-red-400 {
601+
.\[\&_\.foo\]\:tw-text-red-400 .foo {
602602
--tw-text-opacity: 1;
603603
color: rgb(248 113 113 / var(--tw-text-opacity));
604604
}
605605
606-
.\[\&_\.foo\]\:tw-bg-white .foo {
606+
.foo .\[\.foo_\&\]\:tw-bg-white {
607607
--tw-bg-opacity: 1;
608608
background-color: rgb(255 255 255 / var(--tw-bg-opacity));
609609
}
610610
611-
.\[\&_\.foo\]\:tw-text-red-400 .foo {
611+
.foo .\[\.foo_\&\]\:tw-text-red-400 {
612612
--tw-text-opacity: 1;
613613
color: rgb(248 113 113 / var(--tw-text-opacity));
614614
}
@@ -1063,3 +1063,39 @@ it('should be possible to use modifiers and arbitrary peers', () => {
10631063
`)
10641064
})
10651065
})
1066+
1067+
it('Arbitrary variants are ordered alphabetically', () => {
1068+
let config = {
1069+
content: [
1070+
{
1071+
raw: html`
1072+
<div>
1073+
<div class="[&::b]:underline"></div>
1074+
<div class="[&::a]:underline"></div>
1075+
<div class="[&::c]:underline"></div>
1076+
<div class="[&::b]:underline"></div>
1077+
</div>
1078+
`,
1079+
},
1080+
],
1081+
corePlugins: { preflight: false },
1082+
}
1083+
1084+
let input = css`
1085+
@tailwind utilities;
1086+
`
1087+
1088+
return run(input, config).then((result) => {
1089+
expect(result.css).toMatchFormattedCss(css`
1090+
.\[\&\:\:a\]\:underline::a {
1091+
text-decoration-line: underline;
1092+
}
1093+
.\[\&\:\:b\]\:underline::b {
1094+
text-decoration-line: underline;
1095+
}
1096+
.\[\&\:\:c\]\:underline::c {
1097+
text-decoration-line: underline;
1098+
}
1099+
`)
1100+
})
1101+
})

‎tests/variants.test.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -1122,25 +1122,25 @@ test('arbitrary variant selectors should not re-order scrollbar pseudo classes',
11221122
let result = await run(input, config)
11231123

11241124
expect(result.css).toMatchFormattedCss(css`
1125-
.\[\&\:\:-webkit-scrollbar\:hover\]\:underline::-webkit-scrollbar:hover {
1125+
.\[\&\:\:-webkit-resizer\:hover\]\:underline::-webkit-resizer:hover {
11261126
text-decoration-line: underline;
11271127
}
11281128
.\[\&\:\:-webkit-scrollbar-button\:hover\]\:underline::-webkit-scrollbar-button:hover {
11291129
text-decoration-line: underline;
11301130
}
1131-
.\[\&\:\:-webkit-scrollbar-thumb\:hover\]\:underline::-webkit-scrollbar-thumb:hover {
1131+
.\[\&\:\:-webkit-scrollbar-corner\:hover\]\:underline::-webkit-scrollbar-corner:hover {
11321132
text-decoration-line: underline;
11331133
}
1134-
.\[\&\:\:-webkit-scrollbar-track\:hover\]\:underline::-webkit-scrollbar-track:hover {
1134+
.\[\&\:\:-webkit-scrollbar-thumb\:hover\]\:underline::-webkit-scrollbar-thumb:hover {
11351135
text-decoration-line: underline;
11361136
}
11371137
.\[\&\:\:-webkit-scrollbar-track-piece\:hover\]\:underline::-webkit-scrollbar-track-piece:hover {
11381138
text-decoration-line: underline;
11391139
}
1140-
.\[\&\:\:-webkit-scrollbar-corner\:hover\]\:underline::-webkit-scrollbar-corner:hover {
1140+
.\[\&\:\:-webkit-scrollbar-track\:hover\]\:underline::-webkit-scrollbar-track:hover {
11411141
text-decoration-line: underline;
11421142
}
1143-
.\[\&\:\:-webkit-resizer\:hover\]\:underline::-webkit-resizer:hover {
1143+
.\[\&\:\:-webkit-scrollbar\:hover\]\:underline::-webkit-scrollbar:hover {
11441144
text-decoration-line: underline;
11451145
}
11461146
`)

0 commit comments

Comments
 (0)