Throw out compound variants using variants with nested selectors (#14018

)
tailwindlabs · Jul 17, 2024 · 6ab2893 · 6ab2893
1 parent 7052555
commit 6ab2893
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 44 deletions.
diff --git a/packages/tailwindcss/src/index.test.ts b/packages/tailwindcss/src/index.test.ts
@@ -1812,50 +1812,25 @@ describe('@variant', () => {
     `)
   })
 
-  test('combining multiple complex variants', () => {
-    let compiled = compile(css`
-      @variant one {
-        &.foo-1 {
-          &.bar-1 {
-            @slot;
-          }
-        }
-      }
+  test('at-rule-only variants cannot be used with compound variants', () => {
+    expect(
+      compileCss(
+        css`
+          @variant foo (@media foo);
 
-      @variant two {
-        &.foo-2 {
-          &.bar-2 {
-            @slot;
+          @layer utilities {
+            @tailwind utilities;
           }
-        }
-      }
-
-      @layer utilities {
-        @tailwind utilities;
-      }
-    `).build([
-      'group-one:two:underline',
-      'one:group-two:underline',
-      'peer-one:two:underline',
-      'one:peer-two:underline',
-    ])
+        `,
 
-    expect(optimizeCss(compiled).trim()).toMatchInlineSnapshot(`
+        ['foo:flex', 'group-foo:flex', 'peer-foo:flex', 'has-foo:flex', 'not-foo:flex'],
+      ),
+    ).toMatchInlineSnapshot(`
       "@layer utilities {
-        .one\\:group-two\\:underline.foo-1.bar-1:is(:where(.group).foo-2 *):is(:where(.group).bar-2 *) {
-          text-decoration-line: underline;
-        }
-
-        .one\\:peer-two\\:underline.foo-1.bar-1:is(:where(.peer).foo-2 ~ *):is(:where(.peer).bar-2 ~ *) {
-          text-decoration-line: underline;
-        }
-
-        .group-one\\:two\\:underline:is(:where(.group).foo-1 *):is(:where(.group).bar-1 *).foo-2.bar-2 {
-          text-decoration-line: underline;
-        }
-
-        .peer-one\\:two\\:underline:is(:where(.peer).foo-1 ~ *):is(:where(.peer).bar-1 ~ *).foo-2.bar-2 {
-          text-decoration-line: underline;
+        @media foo {
+          .foo\\:flex {
+            display: flex;
+          }
         }
       }"
     `)

diff --git a/packages/tailwindcss/src/variants.test.ts b/packages/tailwindcss/src/variants.test.ts
@@ -783,9 +783,16 @@ test('group-*', () => {
     compileCss(
       css`
         @variant custom-at-rule (@media foo);
+        @variant nested-selectors {
+          &:hover {
+            &:focus {
+              @slot;
+            }
+          }
+        }
         @tailwind utilities;
       `,
-      ['group-custom-at-rule:flex'],
+      ['group-custom-at-rule:flex', 'group-nested-selectors:flex'],
     ),
   ).toEqual('')
 })
@@ -877,9 +884,16 @@ test('peer-*', () => {
     compileCss(
       css`
         @variant custom-at-rule (@media foo);
+        @variant nested-selectors {
+          &:hover {
+            &:focus {
+              @slot;
+            }
+          }
+        }
         @tailwind utilities;
       `,
-      ['peer-custom-at-rule:flex'],
+      ['peer-custom-at-rule:flex', 'peer-nested-selectors:flex'],
     ),
   ).toEqual('')
 })
@@ -1616,9 +1630,21 @@ test('not', () => {
     compileCss(
       css`
         @variant custom-at-rule (@media foo);
+        @variant nested-selectors {
+          &:hover {
+            &:focus {
+              @slot;
+            }
+          }
+        }
         @tailwind utilities;
       `,
-      ['not-[:checked]/foo:flex', 'not-[@media_print]:flex', 'not-custom-at-rule:flex'],
+      [
+        'not-[:checked]/foo:flex',
+        'not-[@media_print]:flex',
+        'not-custom-at-rule:flex',
+        'not-nested-selectors:flex',
+      ],
     ),
   ).toEqual('')
 })
@@ -1706,9 +1732,21 @@ test('has', () => {
     compileCss(
       css`
         @variant custom-at-rule (@media foo);
+        @variant nested-selectors {
+          &:hover {
+            &:focus {
+              @slot;
+            }
+          }
+        }
         @tailwind utilities;
       `,
-      ['has-[:checked]/foo:flex', 'has-[@media_print]:flex', 'has-custom-at-rule:flex'],
+      [
+        'has-[:checked]/foo:flex',
+        'has-[@media_print]:flex',
+        'has-custom-at-rule:flex',
+        'has-nested-selectors:flex',
+      ],
     ),
   ).toEqual('')
 })

diff --git a/packages/tailwindcss/src/variants.ts b/packages/tailwindcss/src/variants.ts
@@ -214,6 +214,18 @@ export function createVariants(theme: Theme): Variants {
       // Skip past at-rules, and continue traversing the children of the at-rule
       if (node.selector[0] === '@') return WalkAction.Continue
 
+      // Throw out any candidates with variants using nested selectors
+      if (didApply) {
+        walk([node], (childNode) => {
+          if (childNode.kind !== 'rule' || childNode.selector[0] === '@') return WalkAction.Continue
+
+          didApply = false
+          return WalkAction.Stop
+        })
+
+        return didApply ? WalkAction.Skip : WalkAction.Stop
+      }
+
       // Replace `&` in target variant with `*`, so variants like `&:hover`
       // become `&:not(*:hover)`. The `*` will often be optimized away.
       node.selector = `&:not(${node.selector.replaceAll('&', '*')})`
@@ -244,6 +256,18 @@ export function createVariants(theme: Theme): Variants {
       // Skip past at-rules, and continue traversing the children of the at-rule
       if (node.selector[0] === '@') return WalkAction.Continue
 
+      // Throw out any candidates with variants using nested selectors
+      if (didApply) {
+        walk([node], (childNode) => {
+          if (childNode.kind !== 'rule' || childNode.selector[0] === '@') return WalkAction.Continue
+
+          didApply = false
+          return WalkAction.Stop
+        })
+
+        return didApply ? WalkAction.Skip : WalkAction.Stop
+      }
+
       // For most variants we rely entirely on CSS nesting to build-up the final
       // selector, but there is no way to use CSS nesting to make `&` refer to
       // just the `.group` class the way we'd need to for these variants, so we
@@ -291,6 +315,18 @@ export function createVariants(theme: Theme): Variants {
       // Skip past at-rules, and continue traversing the children of the at-rule
       if (node.selector[0] === '@') return WalkAction.Continue
 
+      // Throw out any candidates with variants using nested selectors
+      if (didApply) {
+        walk([node], (childNode) => {
+          if (childNode.kind !== 'rule' || childNode.selector[0] === '@') return WalkAction.Continue
+
+          didApply = false
+          return WalkAction.Stop
+        })
+
+        return didApply ? WalkAction.Skip : WalkAction.Stop
+      }
+
       // For most variants we rely entirely on CSS nesting to build-up the final
       // selector, but there is no way to use CSS nesting to make `&` refer to
       // just the `.group` class the way we'd need to for these variants, so we
@@ -440,6 +476,18 @@ export function createVariants(theme: Theme): Variants {
       // Skip past at-rules, and continue traversing the children of the at-rule
       if (node.selector[0] === '@') return WalkAction.Continue
 
+      // Throw out any candidates with variants using nested selectors
+      if (didApply) {
+        walk([node], (childNode) => {
+          if (childNode.kind !== 'rule' || childNode.selector[0] === '@') return WalkAction.Continue
+
+          didApply = false
+          return WalkAction.Stop
+        })
+
+        return didApply ? WalkAction.Skip : WalkAction.Stop
+      }
+
       // Replace `&` in target variant with `*`, so variants like `&:hover`
       // become `&:has(*:hover)`. The `*` will often be optimized away.
       node.selector = `&:has(${node.selector.replaceAll('&', '*')})`