From 6ab289343deaa61095a8f55799239ce0f1ee41ea Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Wed, 17 Jul 2024 15:08:34 -0400 Subject: [PATCH] Throw out compound variants using variants with nested selectors (#14018) --- packages/tailwindcss/src/index.test.ts | 57 +++++++---------------- packages/tailwindcss/src/variants.test.ts | 46 ++++++++++++++++-- packages/tailwindcss/src/variants.ts | 48 +++++++++++++++++++ 3 files changed, 106 insertions(+), 45 deletions(-) diff --git a/packages/tailwindcss/src/index.test.ts b/packages/tailwindcss/src/index.test.ts index c0723af64..bb0f355a6 100644 --- 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); + + @layer utilities { + @tailwind utilities; } - } - } + `, - @variant two { - &.foo-2 { - &.bar-2 { - @slot; - } - } - } - - @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 index 2955e415c..5fb6b4f22 100644 --- 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 index 3611dd0b3..909cfef22 100644 --- 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('&', '*')})`