From 7089a80ea11d89f40b34b143969841891e22b429 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 17 Dec 2021 13:39:32 +0100 Subject: [PATCH] Improve circular dependency detection when using `@apply` (#6588) * improve circular dependency detection when using `@apply` I also changed the message to the same message we used in V2. * update changelog --- CHANGELOG.md | 1 + src/lib/expandApplyAtRules.js | 73 ++++++++++++++++++++++++++--------- tests/apply.test.js | 46 ++++++++++++++++++++-- 3 files changed, 97 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b28ccfe6386..c9e349b8e5e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js index 561343266945..60499329041f 100644 --- a/src/lib/expandApplyAtRules.js +++ b/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) { @@ -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.` ) } @@ -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) } diff --git a/tests/apply.test.js b/tests/apply.test.js index b4932a702c40..7b74286b4783 100644 --- a/tests/apply.test.js +++ b/tests/apply.test.js @@ -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.' + ) }) }) @@ -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`
` }], + 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)); + } + `) }) }) @@ -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.' + ) }) }) @@ -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.' + ) }) })