Improve sorting candidates containing numbers (#13507)

* implement custom `compare` for sorting purposes

This `compare` function compares two strings. However, once a number is
reached the numbers are compared as actual numbers instead of the string
representation.

E.g.:

```
p-1
p-2
p-10
p-20
```

Will be sorted as expected in this order, instead of

```
p-1
p-10
p-2
p-20
```

---

This should also make suggestions in the vscode extension more logical.

* update tests to reflect order changes

* update changelog

* reset `i` correctly

This makes the code more correct _and_ improves performance because the
`Number(…)` will now always deal with numbers.

On the tailwindcss.com codebase, sorting now goes from `~3.29ms` to
`~3.10ms`

* drop unreachable code

In this branch, it's guaranteed that numbers are _different_ which means
that they are never going to be the same thus unreachable code.

When we compare two strings such as:

```
foo-123-bar
foo-123-baz
```

Then all characters until the last character is the same character in
both positions. This means that "numbers" that are the same in the same
position will be compared as strings instead of numbers. But that is
fine because they are the same anyway.

* add fallback in case numbers are the same but strings are not

This can happen if we are sorting `0123` and `123`. The `Number`
representation will be equal, but the string is not.

Will rarely or even never happen. But if it does, this makes it
deterministic.

* re-word comment

* add more test cases with numbers in different spots with various lengths

* Update CHANGELOG.md

* cleanup, simplify which variables we increment

This also gets rid of some explanation that can now be omitted entirely.

---------

Co-authored-by: Adam Wathan <adam.wathan@gmail.com>
This commit is contained in:
Robin Malfait 2024-04-15 17:56:30 +02:00 committed by GitHub
parent b07cc4d3bd
commit cd0c308afe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 284 additions and 108 deletions

View File

@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## Changed
- Use `rem` units for breakpoints by default instead of `px` ([#13469](https://github.com/tailwindlabs/tailwindcss/pull/13469))
- Use natural sorting when sorting classes ([#13507](https://github.com/tailwindlabs/tailwindcss/pull/13507))
## [4.0.0-alpha.14] - 2024-04-09

View File

@ -18,11 +18,6 @@ exports[`border-* 1`] = `
border-width: 0;
}
.border-123 {
border-style: var(--tw-border-style);
border-width: 123px;
}
.border-2 {
border-style: var(--tw-border-style);
border-width: 2px;
@ -33,6 +28,11 @@ exports[`border-* 1`] = `
border-width: 4px;
}
.border-123 {
border-style: var(--tw-border-style);
border-width: 123px;
}
.border-\\[12px\\] {
border-style: var(--tw-border-style);
border-width: 12px;
@ -131,11 +131,6 @@ exports[`border-b-* 1`] = `
border-bottom-width: 0;
}
.border-b-123 {
border-bottom-style: var(--tw-border-style);
border-bottom-width: 123px;
}
.border-b-2 {
border-bottom-style: var(--tw-border-style);
border-bottom-width: 2px;
@ -146,6 +141,11 @@ exports[`border-b-* 1`] = `
border-bottom-width: 4px;
}
.border-b-123 {
border-bottom-style: var(--tw-border-style);
border-bottom-width: 123px;
}
.border-b-\\[12px\\] {
border-bottom-style: var(--tw-border-style);
border-bottom-width: 12px;
@ -244,11 +244,6 @@ exports[`border-e-* 1`] = `
border-inline-end-width: 0;
}
.border-e-123 {
border-inline-end-style: var(--tw-border-style);
border-inline-end-width: 123px;
}
.border-e-2 {
border-inline-end-style: var(--tw-border-style);
border-inline-end-width: 2px;
@ -259,6 +254,11 @@ exports[`border-e-* 1`] = `
border-inline-end-width: 4px;
}
.border-e-123 {
border-inline-end-style: var(--tw-border-style);
border-inline-end-width: 123px;
}
.border-e-\\[12px\\] {
border-inline-end-style: var(--tw-border-style);
border-inline-end-width: 12px;
@ -357,11 +357,6 @@ exports[`border-l-* 1`] = `
border-left-width: 0;
}
.border-l-123 {
border-left-style: var(--tw-border-style);
border-left-width: 123px;
}
.border-l-2 {
border-left-style: var(--tw-border-style);
border-left-width: 2px;
@ -372,6 +367,11 @@ exports[`border-l-* 1`] = `
border-left-width: 4px;
}
.border-l-123 {
border-left-style: var(--tw-border-style);
border-left-width: 123px;
}
.border-l-\\[12px\\] {
border-left-style: var(--tw-border-style);
border-left-width: 12px;
@ -470,11 +470,6 @@ exports[`border-r-* 1`] = `
border-right-width: 0;
}
.border-r-123 {
border-right-style: var(--tw-border-style);
border-right-width: 123px;
}
.border-r-2 {
border-right-style: var(--tw-border-style);
border-right-width: 2px;
@ -485,6 +480,11 @@ exports[`border-r-* 1`] = `
border-right-width: 4px;
}
.border-r-123 {
border-right-style: var(--tw-border-style);
border-right-width: 123px;
}
.border-r-\\[12px\\] {
border-right-style: var(--tw-border-style);
border-right-width: 12px;
@ -583,11 +583,6 @@ exports[`border-s-* 1`] = `
border-inline-start-width: 0;
}
.border-s-123 {
border-inline-start-style: var(--tw-border-style);
border-inline-start-width: 123px;
}
.border-s-2 {
border-inline-start-style: var(--tw-border-style);
border-inline-start-width: 2px;
@ -598,6 +593,11 @@ exports[`border-s-* 1`] = `
border-inline-start-width: 4px;
}
.border-s-123 {
border-inline-start-style: var(--tw-border-style);
border-inline-start-width: 123px;
}
.border-s-\\[12px\\] {
border-inline-start-style: var(--tw-border-style);
border-inline-start-width: 12px;
@ -696,11 +696,6 @@ exports[`border-t-* 1`] = `
border-top-width: 0;
}
.border-t-123 {
border-top-style: var(--tw-border-style);
border-top-width: 123px;
}
.border-t-2 {
border-top-style: var(--tw-border-style);
border-top-width: 2px;
@ -711,6 +706,11 @@ exports[`border-t-* 1`] = `
border-top-width: 4px;
}
.border-t-123 {
border-top-style: var(--tw-border-style);
border-top-width: 123px;
}
.border-t-\\[12px\\] {
border-top-style: var(--tw-border-style);
border-top-width: 12px;
@ -813,13 +813,6 @@ exports[`border-x-* 1`] = `
border-right-width: 0;
}
.border-x-123 {
border-left-style: var(--tw-border-style);
border-right-style: var(--tw-border-style);
border-left-width: 123px;
border-right-width: 123px;
}
.border-x-2 {
border-left-style: var(--tw-border-style);
border-right-style: var(--tw-border-style);
@ -834,6 +827,13 @@ exports[`border-x-* 1`] = `
border-right-width: 4px;
}
.border-x-123 {
border-left-style: var(--tw-border-style);
border-right-style: var(--tw-border-style);
border-left-width: 123px;
border-right-width: 123px;
}
.border-x-\\[12px\\] {
border-left-style: var(--tw-border-style);
border-right-style: var(--tw-border-style);
@ -958,13 +958,6 @@ exports[`border-y-* 1`] = `
border-bottom-width: 0;
}
.border-y-123 {
border-top-style: var(--tw-border-style);
border-bottom-style: var(--tw-border-style);
border-top-width: 123px;
border-bottom-width: 123px;
}
.border-y-2 {
border-top-style: var(--tw-border-style);
border-bottom-style: var(--tw-border-style);
@ -979,6 +972,13 @@ exports[`border-y-* 1`] = `
border-bottom-width: 4px;
}
.border-y-123 {
border-top-style: var(--tw-border-style);
border-bottom-style: var(--tw-border-style);
border-top-width: 123px;
border-bottom-width: 123px;
}
.border-y-\\[12px\\] {
border-top-style: var(--tw-border-style);
border-bottom-style: var(--tw-border-style);

View File

@ -2,6 +2,7 @@ import { rule, type AstNode, type Rule } from './ast'
import { type Candidate, type Variant } from './candidate'
import { type DesignSystem } from './design-system'
import GLOBAL_PROPERTY_ORDER from './property-order'
import { compare } from './utils/compare'
import { escape } from './utils/escape'
import type { Variants } from './variants'
@ -87,7 +88,7 @@ export function compileCandidates(
// Sort by most properties first, then by least properties
zSorting.properties.length - aSorting.properties.length ||
// Sort alphabetically
(aSorting.candidate < zSorting.candidate ? -1 : 1)
compare(aSorting.candidate, zSorting.candidate)
)
})

View File

@ -681,14 +681,14 @@ test('col', () => {
grid-column: auto;
}
.col-span-17 {
grid-column: span 17 / span 17;
}
.col-span-4 {
grid-column: span 4 / span 4;
}
.col-span-17 {
grid-column: span 17 / span 17;
}
.col-span-\\[--my-variable\\] {
grid-column: span var(--my-variable) / span var(--my-variable);
}
@ -762,14 +762,14 @@ test('row', () => {
grid-row: auto;
}
.row-span-17 {
grid-row: span 17 / span 17;
}
.row-span-4 {
grid-row: span 4 / span 4;
}
.row-span-17 {
grid-row: span 17 / span 17;
}
.row-span-\\[--my-variable\\] {
grid-row: span var(--my-variable) / span var(--my-variable);
}
@ -5405,18 +5405,18 @@ test('divide-x', () => {
border-inline-end-width: calc(1px * calc(1 - var(--tw-divide-x-reverse)));
}
:where(.divide-x-123 > :not(:last-child)) {
border-inline-style: var(--tw-border-style);
border-inline-start-width: calc(123px * var(--tw-divide-x-reverse));
border-inline-end-width: calc(123px * calc(1 - var(--tw-divide-x-reverse)));
}
:where(.divide-x-4 > :not(:last-child)) {
border-inline-style: var(--tw-border-style);
border-inline-start-width: calc(4px * var(--tw-divide-x-reverse));
border-inline-end-width: calc(4px * calc(1 - var(--tw-divide-x-reverse)));
}
:where(.divide-x-123 > :not(:last-child)) {
border-inline-style: var(--tw-border-style);
border-inline-start-width: calc(123px * var(--tw-divide-x-reverse));
border-inline-end-width: calc(123px * calc(1 - var(--tw-divide-x-reverse)));
}
:where(.divide-x-\\[4px\\] > :not(:last-child)) {
border-inline-style: var(--tw-border-style);
border-inline-start-width: calc(4px * var(--tw-divide-x-reverse));
@ -5490,13 +5490,6 @@ test('divide-y', () => {
border-bottom-width: calc(1px * calc(1 - var(--tw-divide-y-reverse)));
}
:where(.divide-y-123 > :not(:last-child)) {
border-bottom-style: var(--tw-border-style);
border-top-style: var(--tw-border-style);
border-top-width: calc(123px * var(--tw-divide-y-reverse));
border-bottom-width: calc(123px * calc(1 - var(--tw-divide-y-reverse)));
}
:where(.divide-y-4 > :not(:last-child)) {
border-bottom-style: var(--tw-border-style);
border-top-style: var(--tw-border-style);
@ -5504,6 +5497,13 @@ test('divide-y', () => {
border-bottom-width: calc(4px * calc(1 - var(--tw-divide-y-reverse)));
}
:where(.divide-y-123 > :not(:last-child)) {
border-bottom-style: var(--tw-border-style);
border-top-style: var(--tw-border-style);
border-top-width: calc(123px * var(--tw-divide-y-reverse));
border-bottom-width: calc(123px * calc(1 - var(--tw-divide-y-reverse)));
}
:where(.divide-y-\\[4px\\] > :not(:last-child)) {
border-bottom-style: var(--tw-border-style);
border-top-style: var(--tw-border-style);
@ -7525,19 +7525,15 @@ test('bg', () => {
background-attachment: scroll;
}
.bg-\\[120px\\] {
background-position: 120px;
}
.bg-\\[120px_120px\\] {
background-position: 120px 120px;
}
.bg-\\[50\\%\\] {
background-position: 50%;
}
.bg-\\[position\\:120px_120px\\] {
.bg-\\[120px\\] {
background-position: 120px;
}
.bg-\\[120px_120px\\], .bg-\\[position\\:120px_120px\\] {
background-position: 120px 120px;
}
@ -7792,14 +7788,14 @@ test('from', () => {
--tw-gradient-from-position: 0%;
}
.from-100\\% {
--tw-gradient-from-position: 100%;
}
.from-5\\% {
--tw-gradient-from-position: 5%;
}
.from-100\\% {
--tw-gradient-from-position: 100%;
}
.from-\\[50\\%\\] {
--tw-gradient-from-position: 50%;
}
@ -8014,14 +8010,14 @@ test('via', () => {
--tw-gradient-via-position: 0%;
}
.via-100\\% {
--tw-gradient-via-position: 100%;
}
.via-5\\% {
--tw-gradient-via-position: 5%;
}
.via-100\\% {
--tw-gradient-via-position: 100%;
}
.via-\\[50\\%\\] {
--tw-gradient-via-position: 50%;
}
@ -8224,14 +8220,14 @@ test('to', () => {
--tw-gradient-to-position: 0%;
}
.to-100\\% {
--tw-gradient-to-position: 100%;
}
.to-5\\% {
--tw-gradient-to-position: 5%;
}
.to-100\\% {
--tw-gradient-to-position: 100%;
}
.to-\\[50\\%\\] {
--tw-gradient-to-position: 50%;
}
@ -9424,12 +9420,12 @@ test('font-style', () => {
test('font-stretch', () => {
expect(run(['font-stretch-ultra-expanded', 'font-stretch-50%', 'font-stretch-200%']))
.toMatchInlineSnapshot(`
".font-stretch-200\\% {
font-stretch: 200%;
".font-stretch-50\\% {
font-stretch: 50%;
}
.font-stretch-50\\% {
font-stretch: 50%;
.font-stretch-200\\% {
font-stretch: 200%;
}
.font-stretch-ultra-expanded {
@ -9720,10 +9716,6 @@ test('decoration', () => {
text-decoration-thickness: 1px;
}
.decoration-123 {
text-decoration-thickness: 123px;
}
.decoration-2 {
text-decoration-thickness: 2px;
}
@ -9732,6 +9724,10 @@ test('decoration', () => {
text-decoration-thickness: 4px;
}
.decoration-123 {
text-decoration-thickness: 123px;
}
.decoration-\\[12px\\] {
text-decoration-thickness: 12px;
}
@ -11157,26 +11153,26 @@ test('underline-offset', () => {
],
),
).toMatchInlineSnapshot(`
".-underline-offset-123 {
text-underline-offset: calc(123px * -1);
".-underline-offset-4 {
text-underline-offset: calc(4px * -1);
}
.-underline-offset-4 {
text-underline-offset: calc(4px * -1);
.-underline-offset-123 {
text-underline-offset: calc(123px * -1);
}
.-underline-offset-\\[--value\\] {
text-underline-offset: calc(var(--value) * -1);
}
.underline-offset-123 {
text-underline-offset: 123px;
}
.underline-offset-4 {
text-underline-offset: 4px;
}
.underline-offset-123 {
text-underline-offset: 123px;
}
.underline-offset-\\[--value\\] {
text-underline-offset: var(--value);
}

View File

@ -0,0 +1,126 @@
import { expect, it } from 'vitest'
import { compare } from './compare'
const LESS = -1
const EQUAL = 0
const GREATER = 1
it.each([
// Same strings
['abc', 'abc', EQUAL],
// Shorter string comes first
['abc', 'abcd', LESS],
// Longer string comes first
['abcd', 'abc', GREATER],
// Numbers
['1', '1', EQUAL],
['1', '2', LESS],
['2', '1', GREATER],
['1', '10', LESS],
['10', '1', GREATER],
])('should compare "%s" with "%s" as "%d"', (a, b, expected) => {
expect(Math.sign(compare(a, b))).toBe(expected)
})
it('should sort strings with numbers consistently using the `compare` function', () => {
expect(
['p-0', 'p-0.5', 'p-1', 'p-1.5', 'p-10', 'p-12', 'p-2', 'p-20', 'p-21']
.sort(() => Math.random() - 0.5) // Shuffle the array
.sort(compare), // Sort the array
).toMatchInlineSnapshot(`
[
"p-0",
"p-0.5",
"p-1",
"p-1.5",
"p-2",
"p-10",
"p-12",
"p-20",
"p-21",
]
`)
})
it('should sort strings with modifiers consistently using the `compare` function', () => {
expect(
[
'text-5xl',
'text-6xl',
'text-6xl/loose',
'text-6xl/wide',
'bg-red-500',
'bg-red-500/50',
'bg-red-500/70',
'bg-red-500/60',
'bg-red-50',
'bg-red-50/50',
'bg-red-50/70',
'bg-red-50/60',
]
.sort(() => Math.random() - 0.5) // Shuffle the array
.sort(compare), // Sort the array
).toMatchInlineSnapshot(`
[
"bg-red-50",
"bg-red-50/50",
"bg-red-50/60",
"bg-red-50/70",
"bg-red-500",
"bg-red-500/50",
"bg-red-500/60",
"bg-red-500/70",
"text-5xl",
"text-6xl",
"text-6xl/loose",
"text-6xl/wide",
]
`)
})
it('should sort strings with multiple numbers consistently using the `compare` function', () => {
expect(
[
'foo-123-bar-456-baz-789',
'foo-123-bar-456-baz-788',
'foo-123-bar-456-baz-790',
'foo-123-bar-455-baz-789',
'foo-123-bar-456-baz-789',
'foo-123-bar-457-baz-789',
'foo-123-bar-456-baz-789',
'foo-124-bar-456-baz-788',
'foo-125-bar-456-baz-790',
'foo-126-bar-455-baz-789',
'foo-127-bar-456-baz-789',
'foo-128-bar-457-baz-789',
'foo-1-bar-2-baz-3',
'foo-12-bar-34-baz-45',
'foo-12-bar-34-baz-4',
'foo-12-bar-34-baz-456',
]
.sort(() => Math.random() - 0.5) // Shuffle the array
.sort(compare), // Sort the array
).toMatchInlineSnapshot(`
[
"foo-1-bar-2-baz-3",
"foo-12-bar-34-baz-4",
"foo-12-bar-34-baz-45",
"foo-12-bar-34-baz-456",
"foo-123-bar-455-baz-789",
"foo-123-bar-456-baz-788",
"foo-123-bar-456-baz-789",
"foo-123-bar-456-baz-789",
"foo-123-bar-456-baz-789",
"foo-123-bar-456-baz-790",
"foo-123-bar-457-baz-789",
"foo-124-bar-456-baz-788",
"foo-125-bar-456-baz-790",
"foo-126-bar-455-baz-789",
"foo-127-bar-456-baz-789",
"foo-128-bar-457-baz-789",
]
`)
})

View File

@ -0,0 +1,52 @@
const ZERO = 48
const NINE = 57
/**
* Compare two strings alphanumerically, where numbers are compared as numbers
* instead of strings.
*/
export function compare(a: string, z: string) {
let aLen = a.length
let zLen = z.length
let minLen = aLen < zLen ? aLen : zLen
for (let i = 0; i < minLen; i++) {
let aCode = a.charCodeAt(i)
let zCode = z.charCodeAt(i)
// Continue if the characters are the same
if (aCode === zCode) continue
// If both are numbers, compare them as numbers instead of strings.
if (aCode >= ZERO && aCode <= NINE && zCode >= ZERO && zCode <= NINE) {
let aStart = i
let aEnd = i
let zStart = i
let zEnd = i
// Consume the number
while (a.charCodeAt(aEnd) >= ZERO && a.charCodeAt(aEnd) <= NINE) aEnd++
// Consume the number
while (z.charCodeAt(zEnd) >= ZERO && z.charCodeAt(zEnd) <= NINE) zEnd++
let aNumber = a.slice(aStart, aEnd)
let zNumber = z.slice(zStart, zEnd)
return (
Number(aNumber) - Number(zNumber) ||
// Fallback case if numbers are the same but the string representation
// is not. Fallback to string sorting. E.g.: `0123` vs `123`
(aNumber < zNumber ? -1 : 1)
)
}
// Otherwise, compare them as strings
return aCode - zCode
}
// If we got this far, the strings are equal up to the length of the shortest
// string. The shortest string should come first.
return a.length - z.length
}