Skip to content

Commit

Permalink
Use amount of properties when sorting (#16715)
Browse files Browse the repository at this point in the history
Right now we sort the nodes based on a pre-defined sort order based on
the properties that are being used. The property sort order is defined
in a list we maintain.

We also have to make sure that the property count is taken into account
such that if all the "sorts" are the same, that we fallback to the
property count. Most amount of properties should be first such that we
can override it with more specific utilities that have fewer properties.

However, if a property doesn't exist, then it wouldn't be included in a
list of properties therefore the total count was off.

This PR fixes that by counting all the used properties. If a property
already exists it is counted twice. E.g.:
```css
.foo {
  color: red;

  &:hover {
    color: blue;
  }
}
```

In this case, we have 2 properties, not 1 even though it's the same
`color` property.

## Test plan:

1. Updated the tests that are now sorted correctly
2. Added an integration test to make sure that `prose-invert` is defined
after the `prose-stone` classes when using the `@tailwindcss/typography`
plugin where this problem originated from.

Note how in this play (https://play.tailwindcss.com/wt3LYDaljN) the
`prose-invert` comes _before_ the `prose-stone` which means that you
can't apply the `prose-invert` classes to invert `prose-stone`.
  • Loading branch information
RobinMalfait authored Feb 21, 2025
1 parent f8d7623 commit 113142a
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 92 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Vite: Ensure Astro production builds contain classes for client-only components ([#16631](https://github.com/tailwindlabs/tailwindcss/pull/16631))
- Vite: Ensure utility classes are read without escaping special characters ([#16631](https://github.com/tailwindlabs/tailwindcss/pull/16631))
- Allow `theme(…)` options when using `@import` ([#16514](https://github.com/tailwindlabs/tailwindcss/pull/16514))
- Use amount of properties when sorting ([#16715](https://github.com/tailwindlabs/tailwindcss/pull/16715))

## [4.0.7] - 2025-02-18

Expand Down
13 changes: 11 additions & 2 deletions integrations/cli/plugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ test(
}
`,
'index.html': html`
<div className="prose">
<div className="prose prose-stone prose-invert">
<h1>Headline</h1>
<p>
Until now, trying to style an article, document, or blog post with Tailwind has been a
Expand All @@ -28,9 +28,18 @@ test(
`,
},
},
async ({ fs, exec }) => {
async ({ fs, exec, expect }) => {
await exec('pnpm tailwindcss --input src/index.css --output dist/out.css')

// Verify that `prose-stone` is defined before `prose-invert`
{
let contents = await fs.read('dist/out.css')
let proseInvertIdx = contents.indexOf('.prose-invert')
let proseStoneIdx = contents.indexOf('.prose-stone')

expect(proseStoneIdx).toBeLessThan(proseInvertIdx)
}

await fs.expectFileToContain('dist/out.css', [
candidate`prose`,
':where(h1):not(:where([class~="not-prose"],[class~="not-prose"] *))',
Expand Down
16 changes: 8 additions & 8 deletions packages/tailwindcss/src/compat/plugin-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3047,6 +3047,14 @@ describe('addUtilities()', () => {
).toMatchInlineSnapshot(
`
"@layer utilities {
.j {
&.j {
color: red;
}
.j& {
color: red;
}
}
.a {
& .b:hover .c {
color: red;
Expand Down Expand Up @@ -3087,14 +3095,6 @@ describe('addUtilities()', () => {
color: red;
}
}
.j {
&.j {
color: red;
}
.j& {
color: red;
}
}
}"
`,
)
Expand Down
40 changes: 28 additions & 12 deletions packages/tailwindcss/src/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function compileCandidates(
) {
let nodeSorting = new Map<
AstNode,
{ properties: number[]; variants: bigint; candidate: string }
{ properties: { order: number[]; count: number }; variants: bigint; candidate: string }
>()
let astNodes: AstNode[] = []
let matches = new Map<string, Candidate[]>()
Expand Down Expand Up @@ -95,18 +95,19 @@ export function compileCandidates(
// Find the first property that is different between the two rules
let offset = 0
while (
aSorting.properties.length < offset &&
zSorting.properties.length < offset &&
aSorting.properties[offset] === zSorting.properties[offset]
offset < aSorting.properties.order.length &&
offset < zSorting.properties.order.length &&
aSorting.properties.order[offset] === zSorting.properties.order[offset]
) {
offset += 1
}

return (
// Sort by lowest property index first
(aSorting.properties[offset] ?? Infinity) - (zSorting.properties[offset] ?? Infinity) ||
(aSorting.properties.order[offset] ?? Infinity) -
(zSorting.properties.order[offset] ?? Infinity) ||
// Sort by most properties first, then by least properties
zSorting.properties.length - aSorting.properties.length ||
zSorting.properties.count - aSorting.properties.count ||
// Sort alphabetically
compare(aSorting.candidate, zSorting.candidate)
)
Expand All @@ -124,7 +125,10 @@ export function compileAstNodes(candidate: Candidate, designSystem: DesignSystem

let rules: {
node: AstNode
propertySort: number[]
propertySort: {
order: number[]
count: number
}
}[] = []

let selector = `.${escape(candidate.raw)}`
Expand Down Expand Up @@ -310,30 +314,42 @@ function applyImportant(ast: AstNode[]): void {

function getPropertySort(nodes: AstNode[]) {
// Determine sort order based on properties used
let propertySort = new Set<number>()
let order = new Set<number>()
let count = 0
let q: AstNode[] = nodes.slice()

let seenTwSort = false

while (q.length > 0) {
// SAFETY: At this point it is safe to use TypeScript's non-null assertion
// operator because we guarded against `q.length > 0` above.
let node = q.shift()!
if (node.kind === 'declaration') {
// Empty strings should still be counted, e.g.: `--tw-foo:;` is valid
if (node.value !== undefined) count++

if (seenTwSort) continue

if (node.property === '--tw-sort') {
let idx = GLOBAL_PROPERTY_ORDER.indexOf(node.value ?? '')
if (idx !== -1) {
propertySort.add(idx)
break
order.add(idx)
seenTwSort = true
continue
}
}

let idx = GLOBAL_PROPERTY_ORDER.indexOf(node.property)
if (idx !== -1) propertySort.add(idx)
if (idx !== -1) order.add(idx)
} else if (node.kind === 'rule' || node.kind === 'at-rule') {
for (let child of node.nodes) {
q.push(child)
}
}
}

return Array.from(propertySort).sort((a, z) => a - z)
return {
order: Array.from(order).sort((a, z) => a - z),
count,
}
}
1 change: 1 addition & 0 deletions packages/tailwindcss/src/property-order.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export default [
'translate',
'--tw-translate-x',
'--tw-translate-y',
'--tw-translate-z',
'scale',
'--tw-scale-x',
'--tw-scale-y',
Expand Down
Loading

0 comments on commit 113142a

Please # to comment.