From 4ab2a7fe05f5e943a99c1b5d267adbe0937c5d77 Mon Sep 17 00:00:00 2001
From: Jordan Pittman <jordan@cryptica.me>
Date: Thu, 28 Apr 2022 10:50:01 -0400
Subject: [PATCH] Only check selectors containing base apply candidates for
 circular dependencies
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 src/lib/expandApplyAtRules.js | 24 +++++++++-
 tests/apply.test.js           | 85 +++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/src/lib/expandApplyAtRules.js b/src/lib/expandApplyAtRules.js
index 65fe7950f0a1..0a59e20d8d62 100644
--- a/src/lib/expandApplyAtRules.js
+++ b/src/lib/expandApplyAtRules.js
@@ -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) {
@@ -353,10 +365,18 @@ 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.
diff --git a/tests/apply.test.js b/tests/apply.test.js
index 7f149617e53a..9841d58266d9 100644
--- a/tests/apply.test.js
+++ b/tests/apply.test.js
@@ -658,6 +658,91 @@ 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 },