Skip to content

Commit

Permalink
Only check selectors containing apply candidates for circular depende…
Browse files Browse the repository at this point in the history
…ncies (#8222)

* Only check selectors containing base apply candidates for circular dependencies

When given a two rule like `html.dark .a, .b { … }` and `html.dark .c { @apply b }` we would see `.dark` in both the base rule and the rule being applied and consider it a circular dependency. However, the selectors `html.dark .a` and `.b` are considered on their own and is therefore do not introduce a circular dependency.

This better matches the user’s mental model that the selectors are just two definitions sharing the same properties.

* Update changelog
  • Loading branch information
thecrypticace committed May 2, 2022
1 parent 3989f77 commit 7c337f2
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Improve type detection for arbitrary color values ([#8201](https://github.com/tailwindlabs/tailwindcss/pull/8201))
- Support PostCSS config options in config file in CLI ([#8226](https://github.com/tailwindlabs/tailwindcss/pull/8226))
- Remove default `[hidden]` style in preflight ([#8248](https://github.com/tailwindlabs/tailwindcss/pull/8248))
- Only check selectors containing base apply candidates for circular dependencies ([#8222](https://github.com/tailwindlabs/tailwindcss/pull/8222))

### Added

Expand Down
29 changes: 27 additions & 2 deletions src/lib/expandApplyAtRules.js
Expand Up @@ -8,18 +8,30 @@ import escapeClassName from '../util/escapeClassName'
/** @typedef {Map<string, [any, import('postcss').Rule[]]>} ApplyCache */

function extractClasses(node) {
let classes = new Set()
/** @type {Map<string, Set<string>>} */
let groups = new Map()

let container = postcss.root({ nodes: [node.clone()] })

container.walkRules((rule) => {
parser((selectors) => {
selectors.walkClasses((classSelector) => {
let parentSelector = classSelector.parent.toString()

let classes = groups.get(parentSelector)
if (!classes) {
groups.set(parentSelector, (classes = new Set()))
}

classes.add(classSelector.value)
})
}).processSync(rule.selector)
})

return Array.from(classes)
let normalizedGroups = Array.from(groups.values(), (classes) => Array.from(classes))
let classes = normalizedGroups.flat()

return Object.assign(classes, { groups: normalizedGroups })
}

function extractBaseCandidates(candidates, separator) {
Expand Down Expand Up @@ -353,10 +365,23 @@ function processApply(root, context, localCache) {
let siblings = []

for (let [applyCandidate, important, rules] of candidates) {
let potentialApplyCandidates = [
applyCandidate,
...extractBaseCandidates([applyCandidate], context.tailwindConfig.separator),
]

for (let [meta, node] of rules) {
let parentClasses = extractClasses(parent)
let nodeClasses = extractClasses(node)

// When we encounter a rule like `.dark .a, .b { … }` we only want to be left with `[.dark, .a]` if the base applyCandidate is `.a` or with `[.b]` if the base applyCandidate is `.b`
// So we've split them into groups
nodeClasses = nodeClasses.groups
.filter((classList) =>
classList.some((className) => potentialApplyCandidates.includes(className))
)
.flat()

// 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.
Expand Down
88 changes: 88 additions & 0 deletions tests/apply.test.js
Expand Up @@ -658,6 +658,94 @@ it('should throw when trying to apply an indirect circular dependency with a mod
})
})

it('should not throw when the circular dependency is part of a different selector (1)', () => {
let config = {
content: [{ raw: html`<div class="c"></div>` }],
plugins: [],
}

let input = css`
@tailwind utilities;
@layer utilities {
html.dark .a,
.b {
color: red;
}
}
html.dark .c {
@apply b;
}
`

return run(input, config).then((result) => {
expect(result.css).toMatchFormattedCss(css`
html.dark .c {
color: red;
}
`)
})
})

it('should not throw when the circular dependency is part of a different selector (2)', () => {
let config = {
content: [{ raw: html`<div class="c"></div>` }],
plugins: [],
}

let input = css`
@tailwind utilities;
@layer utilities {
html.dark .a,
.b {
color: red;
}
}
html.dark .c {
@apply hover:b;
}
`

return run(input, config).then((result) => {
expect(result.css).toMatchFormattedCss(css`
html.dark .c:hover {
color: red;
}
`)
})
})

it('should throw when the circular dependency is part of the same selector', () => {
let config = {
content: [{ raw: html`<div class="c"></div>` }],
plugins: [],
}

let input = css`
@tailwind utilities;
@layer utilities {
html.dark .a,
html.dark .b {
color: red;
}
}
html.dark .c {
@apply hover:b;
}
`

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

it('rules with vendor prefixes are still separate when optimizing defaults rules', () => {
let config = {
experimental: { optimizeUniversalDefaults: true },
Expand Down

0 comments on commit 7c337f2

Please sign in to comment.