Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve circular dependency detection when using @apply #6588

Merged
merged 2 commits into from Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Don't mutate custom color palette when overriding per-plugin colors ([#6546](https://github.com/tailwindlabs/tailwindcss/pull/6546))
- Improve circular dependency detection when using `@apply` ([#6588](https://github.com/tailwindlabs/tailwindcss/pull/6588))

## [3.0.6] - 2021-12-16

Expand Down
73 changes: 54 additions & 19 deletions src/lib/expandApplyAtRules.js
@@ -1,22 +1,33 @@
import postcss from 'postcss'
import parser from 'postcss-selector-parser'

import { resolveMatches } from './generateRules'
import bigSign from '../util/bigSign'
import escapeClassName from '../util/escapeClassName'

function containsBase(selector, classCandidateBase, separator) {
return parser((selectors) => {
let contains = false
function extractClasses(node) {
let classes = new Set()
let container = postcss.root({ nodes: [node.clone()] })

selectors.walkClasses((classSelector) => {
if (classSelector.value.split(separator).pop() === classCandidateBase) {
contains = true
return false
}
})
container.walkRules((rule) => {
parser((selectors) => {
selectors.walkClasses((classSelector) => {
classes.add(classSelector.value)
})
}).processSync(rule.selector)
})

return contains
}).transformSync(selector)
return Array.from(classes)
}

function extractBaseCandidates(candidates, separator) {
let baseClasses = new Set()

for (let candidate of candidates) {
baseClasses.add(candidate.split(separator).pop())
}

return Array.from(baseClasses)
}

function prefix(context, selector) {
Expand Down Expand Up @@ -212,15 +223,40 @@ function processApply(root, context) {
let siblings = []

for (let [applyCandidate, important, rules] of candidates) {
let base = applyCandidate.split(context.tailwindConfig.separator).pop()

for (let [meta, node] of rules) {
if (
containsBase(parent.selector, base, context.tailwindConfig.separator) &&
containsBase(node.selector, base, context.tailwindConfig.separator)
) {
let parentClasses = extractClasses(parent)
let nodeClasses = extractClasses(node)

// Add base utility classes from the @apply node to the list of
// classes to check whether it intersects and therefore results in a
// circular dependency or not.
//
// E.g.:
// .foo {
// @apply hover:a; // This applies "a" but with a modifier
// }
//
// We only have to do that with base classes of the `node`, not of the `parent`
// E.g.:
// .hover\:foo {
// @apply bar;
// }
// .bar {
// @apply foo;
// }
//
// This should not result in a circular dependency because we are
// just applying `.foo` and the rule above is `.hover\:foo` which is
// unrelated. However, if we were to apply `hover:foo` then we _did_
// have to include this one.
nodeClasses = nodeClasses.concat(
extractBaseCandidates(nodeClasses, context.tailwindConfig.separator)
)

let intersects = parentClasses.some((selector) => nodeClasses.includes(selector))
if (intersects) {
throw node.error(
`Circular dependency detected when using: \`@apply ${applyCandidate}\``
`You cannot \`@apply\` the \`${applyCandidate}\` utility here because it creates a circular dependency.`
)
}

Expand Down Expand Up @@ -250,7 +286,6 @@ function processApply(root, context) {
// Inject the rules, sorted, correctly
let nodes = siblings.sort(([a], [z]) => bigSign(a.sort - z.sort)).map((s) => s[1])

// console.log(parent)
// `parent` refers to the node at `.abc` in: .abc { @apply mt-2 }
parent.after(nodes)
}
Expand Down
46 changes: 42 additions & 4 deletions tests/apply.test.js
Expand Up @@ -484,7 +484,9 @@ it('should throw when trying to apply a direct circular dependency', () => {
`

return run(input, config).catch((err) => {
expect(err.reason).toBe('Circular dependency detected when using: `@apply text-red-500`')
expect(err.reason).toBe(
'You cannot `@apply` the `text-red-500` utility here because it creates a circular dependency.'
)
})
})

Expand Down Expand Up @@ -514,7 +516,39 @@ it('should throw when trying to apply an indirect circular dependency', () => {
`

return run(input, config).catch((err) => {
expect(err.reason).toBe('Circular dependency detected when using: `@apply a`')
expect(err.reason).toBe(
'You cannot `@apply` the `a` utility here because it creates a circular dependency.'
)
})
})

it('should not throw when the selector is different (but contains the base partially)', () => {
let config = {
content: [{ raw: html`<div class="bg-gray-500"></div>` }],
plugins: [],
}

let input = css`
@tailwind components;
@tailwind utilities;

.focus\:bg-gray-500 {
@apply bg-gray-500;
}
`

return run(input, config).then((result) => {
expect(result.css).toMatchFormattedCss(css`
.bg-gray-500 {
--tw-bg-opacity: 1;
background-color: rgb(107 114 128 / var(--tw-bg-opacity));
}

.focus\:bg-gray-500 {
--tw-bg-opacity: 1;
background-color: rgb(107 114 128 / var(--tw-bg-opacity));
}
`)
})
})

Expand Down Expand Up @@ -544,7 +578,9 @@ it('should throw when trying to apply an indirect circular dependency with a mod
`

return run(input, config).catch((err) => {
expect(err.reason).toBe('Circular dependency detected when using: `@apply hover:a`')
expect(err.reason).toBe(
'You cannot `@apply` the `hover:a` utility here because it creates a circular dependency.'
)
})
})

Expand Down Expand Up @@ -574,7 +610,9 @@ it('should throw when trying to apply an indirect circular dependency with a mod
`

return run(input, config).catch((err) => {
expect(err.reason).toBe('Circular dependency detected when using: `@apply a`')
expect(err.reason).toBe(
'You cannot `@apply` the `a` utility here because it creates a circular dependency.'
)
})
})

Expand Down