Skip to content

Commit

Permalink
Fix sorting of utilities that share multiple candidates (#12173)
Browse files Browse the repository at this point in the history
* Fix sorting of utilities that share multiple candidates

* Update changelog
  • Loading branch information
thecrypticace authored Oct 10, 2023
1 parent 31a80b1 commit ad66635
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Don’t crash when important and parent selectors are equal in `@apply` ([#12112](https://github.com/tailwindlabs/tailwindcss/pull/12112))
- Eliminate irrelevant rules when applying variants ([#12113](https://github.com/tailwindlabs/tailwindcss/pull/12113))
- Improve RegEx parser, reduce possibilities as the key for arbitrary properties ([#12121](https://github.com/tailwindlabs/tailwindcss/pull/12121))
- Fix sorting of utilities that share multiple candidates ([#12173](https://github.com/tailwindlabs/tailwindcss/pull/12173))

### Added

Expand Down
6 changes: 4 additions & 2 deletions src/lib/generateRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ function getImportantStrategy(important) {
}
}

function generateRules(candidates, context) {
function generateRules(candidates, context, isSorting = false) {
let allRules = []
let strategy = getImportantStrategy(context.tailwindConfig.important)

Expand Down Expand Up @@ -912,7 +912,9 @@ function generateRules(candidates, context) {
rule = container.nodes[0]
}

let newEntry = [sort, rule]
// Note: We have to clone rules during sorting
// so we eliminate some shared mutable state
let newEntry = [sort, isSorting ? rule.clone() : rule]
rules.add(newEntry)
context.ruleCache.add(newEntry)
allRules.push(newEntry)
Expand Down
2 changes: 1 addition & 1 deletion src/lib/setupContextUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ function registerPlugins(plugins, context) {

// Sort all classes in order
// Non-tailwind classes won't be generated and will be left as `null`
let rules = generateRules(new Set(sorted), context)
let rules = generateRules(new Set(sorted), context, true)
rules = context.offsets.sort(rules)

let idx = BigInt(parasiteUtilities.length)
Expand Down
39 changes: 39 additions & 0 deletions tests/getSortOrder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,42 @@ it('sorts based on first occurrence of a candidate / rule', () => {
expect(defaultSort(context.getClassOrder(input.split(' ')))).toEqual(output)
}
})

it('Sorting is unchanged when multiple candidates share the same rule / object', () => {
let classes = [
['x y', 'x y'],
['a', 'a'],
['x y', 'x y'],
]

let config = {
theme: {},
plugins: [
function ({ addComponents }) {
addComponents({
'.x': { color: 'red' },
'.a': { color: 'red' },

// This rule matches both the candidate `a` and `y`
// When sorting x and y first we would keep that sort order
// Then sorting `a` we would end up replacing the candidate on the rule
// Thus causing `y` to no longer have a sort order causing it to be sorted
// first by accident
'.y .a': { color: 'red' },
})
},
],
}

// Same context, different class lists
let context = createContext(resolveConfig(config))
for (const [input, output] of classes) {
expect(defaultSort(context.getClassOrder(input.split(' ')))).toEqual(output)
}

// Different context, different class lists
for (const [input, output] of classes) {
context = createContext(resolveConfig(config))
expect(defaultSort(context.getClassOrder(input.split(' ')))).toEqual(output)
}
})

0 comments on commit ad66635

Please sign in to comment.