Skip to content

Commit

Permalink
Don’t replace all instances of the same class
Browse files Browse the repository at this point in the history
This isn’t 100% correct either so we’re backing out this change
  • Loading branch information
thecrypticace committed Aug 15, 2022
1 parent ef74fd3 commit 7924e7b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 5 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Use absolute paths when resolving changed files for resilience against working directory changes ([#9032](https://github.com/tailwindlabs/tailwindcss/pull/9032))
- Fix ring color utility generation when using `respectDefaultRingColorOpacity` ([#9070](https://github.com/tailwindlabs/tailwindcss/pull/9070))
- Replace all occurrences of a class in a selector when using `@apply` ([#9107](https://github.com/tailwindlabs/tailwindcss/pull/9107))
- Sort tags before classes when `@applying` a selector with joined classes ([#9107](https://github.com/tailwindlabs/tailwindcss/pull/9107))

## [3.1.8] - 2022-08-05
Expand Down
16 changes: 16 additions & 0 deletions src/lib/expandApplyAtRules.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,20 +315,36 @@ function processApply(root, context, localCache) {
let replaced = new Set()

utilitySelectorsList.each((utilitySelector) => {
let hasReplaced = false
utilitySelector = utilitySelector.clone()

utilitySelector.walkClasses((node) => {
if (node.value !== candidateClass.value) {
return
}

// Don't replace multiple instances of the same class
// This is theoretically correct but only partially
// We'd need to generate every possible permutation of the replacement
// For example with `.foo + .foo { … }` and `section { @apply foo; }`
// We'd need to generate all of these:
// - `.foo + .foo`
// - `.foo + section`
// - `section + .foo`
// - `section + section`
if (hasReplaced) {
return
}

// Since you can only `@apply` class names this is sufficient
// We want to replace the matched class name with the selector the user is using
// Ex: Replace `.text-blue-500` with `.foo.bar:is(.something-cool)`
node.replaceWith(...sel.nodes.map((node) => node.clone()))

// Record that we did something and we want to use this new selector
replaced.add(utilitySelector)

hasReplaced = true
})
})

Expand Down
13 changes: 9 additions & 4 deletions tests/apply.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,10 @@ it('can apply joined classes when using elements', async () => {
`)
})

it('can produce selectors that replace multiple instances of the same class', async () => {
it('should not replace multiple instances of the same class in a single selector', async () => {
// NOTE: This test is non-normative and is not part of the spec of how `@apply` works per-se
// It describes how it currently works because the "correct" way produces a combinatorial explosion
// of selectors that is not easily doable
let config = {
content: [{ raw: html`<div class="foo-1 -foo-1 new-class"></div>` }],
plugins: [],
Expand Down Expand Up @@ -1674,14 +1677,16 @@ it('can produce selectors that replace multiple instances of the same class', as
.bar + .bar {
color: fuchsia;
}
header + header {
header + .foo {
color: blue;
}
main + main {
main + .foo {
color: blue;
}
main + .bar {
color: fuchsia;
}
footer + footer {
footer + .bar {
color: fuchsia;
}
`)
Expand Down

0 comments on commit 7924e7b

Please sign in to comment.