From cd0c308afe6397b72de169af3be0e4c7e15727dc Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 15 Apr 2024 17:56:30 +0200 Subject: [PATCH] Improve sorting candidates containing numbers (#13507) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- CHANGELOG.md | 1 + .../src/__snapshots__/utilities.test.ts.snap | 98 +++++++------- packages/tailwindcss/src/compile.ts | 3 +- packages/tailwindcss/src/utilities.test.ts | 110 ++++++++------- .../tailwindcss/src/utils/compare.test.ts | 126 ++++++++++++++++++ packages/tailwindcss/src/utils/compare.ts | 52 ++++++++ 6 files changed, 283 insertions(+), 107 deletions(-) create mode 100644 packages/tailwindcss/src/utils/compare.test.ts create mode 100644 packages/tailwindcss/src/utils/compare.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b2569e68fc9..006240df3aec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/packages/tailwindcss/src/__snapshots__/utilities.test.ts.snap b/packages/tailwindcss/src/__snapshots__/utilities.test.ts.snap index feeb8550adfa..999d5f59c113 100644 --- a/packages/tailwindcss/src/__snapshots__/utilities.test.ts.snap +++ b/packages/tailwindcss/src/__snapshots__/utilities.test.ts.snap @@ -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); diff --git a/packages/tailwindcss/src/compile.ts b/packages/tailwindcss/src/compile.ts index ebbe2a74c52b..c9f604919513 100644 --- a/packages/tailwindcss/src/compile.ts +++ b/packages/tailwindcss/src/compile.ts @@ -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) ) }) diff --git a/packages/tailwindcss/src/utilities.test.ts b/packages/tailwindcss/src/utilities.test.ts index 8983840af68a..7357288c9bab 100644 --- a/packages/tailwindcss/src/utilities.test.ts +++ b/packages/tailwindcss/src/utilities.test.ts @@ -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,18 +5490,18 @@ test('divide-y', () => { border-bottom-width: calc(1px * calc(1 - var(--tw-divide-y-reverse))); } - :where(.divide-y-123 > :not(:last-child)) { + :where(.divide-y-4 > :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))); + border-top-width: calc(4px * var(--tw-divide-y-reverse)); + border-bottom-width: calc(4px * calc(1 - var(--tw-divide-y-reverse))); } - :where(.divide-y-4 > :not(:last-child)) { + :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(4px * var(--tw-divide-y-reverse)); - border-bottom-width: calc(4px * calc(1 - var(--tw-divide-y-reverse))); + 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)) { @@ -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); } diff --git a/packages/tailwindcss/src/utils/compare.test.ts b/packages/tailwindcss/src/utils/compare.test.ts new file mode 100644 index 000000000000..d4075d289048 --- /dev/null +++ b/packages/tailwindcss/src/utils/compare.test.ts @@ -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", + ] + `) +}) diff --git a/packages/tailwindcss/src/utils/compare.ts b/packages/tailwindcss/src/utils/compare.ts new file mode 100644 index 000000000000..7153417c7d9c --- /dev/null +++ b/packages/tailwindcss/src/utils/compare.ts @@ -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 +}