From 005c1be2edf85c9ace9d441cb6b8a9c973491566 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Wed, 28 Jun 2023 12:37:52 -0400 Subject: [PATCH] Don't prefix arbitrary classes in `peer`/`group` variants (#11454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refactor * Don’t prefix classes in arbitrary values for group and peer * use `foo` instead of `lol` * handle the prefix inside the group/peer variants Then add the `NoPrefix` feature to the variant itself, which will skip prefixing any other class in the generated selector (because we already took care of prefixing `.group` and `.peer`). We are using an internal symbol such that: - We can keep it as a private API - We don't introduce a breaking change * refactor to simple object instead We will still use a symbol as an internal/private marker, but the data itself will be a simple object for now. If we want to refactor this (and more) in the future using bitflags then we can refactor that in a separate PR. --------- Co-authored-by: Robin Malfait --- CHANGELOG.md | 1 + src/corePlugins.js | 18 ++++-- src/lib/generateRules.js | 15 +++-- src/lib/setupContextUtils.js | 13 +++- src/util/formatVariantSelector.js | 4 +- src/util/prefixSelector.js | 1 + tests/format-variant-selector.test.js | 86 +++++++++++++-------------- tests/getVariants.test.js | 19 ++++++ tests/prefix.test.js | 30 ++++++++++ 9 files changed, 130 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7842e1125c8f..7e142bb4d849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Ensure `repeating-conic-gradient` is detected as an image ([#11180](https://github.com/tailwindlabs/tailwindcss/pull/11180)) - Move unknown pseudo-elements outside of `:is` by default ([#11345](https://github.com/tailwindlabs/tailwindcss/pull/11345)) - Escape animation names when prefixes contain special characters ([#11470](https://github.com/tailwindlabs/tailwindcss/pull/11470)) +- Don't prefix arbitrary classes in `group` and `peer` variants ([#11454](https://github.com/tailwindlabs/tailwindcss/pull/11454)) - Sort classes using position of first matching rule ([#11504](https://github.com/tailwindlabs/tailwindcss/pull/11504)) - Allow variant to be an at-rule without a prelude ([#11589](https://github.com/tailwindlabs/tailwindcss/pull/11589)) - Make PostCSS plugin async to improve performance ([#11548](https://github.com/tailwindlabs/tailwindcss/pull/11548)) diff --git a/src/corePlugins.js b/src/corePlugins.js index 606cfd7bbecf..5db1fdb74e7b 100644 --- a/src/corePlugins.js +++ b/src/corePlugins.js @@ -22,6 +22,7 @@ import { formatBoxShadowValue, parseBoxShadowValue } from './util/parseBoxShadow import { removeAlphaVariables } from './util/removeAlphaVariables' import { flagEnabled } from './featureFlags' import { normalize } from './util/dataTypes' +import { INTERNAL_FEATURES } from './lib/setupContextUtils' export let variantPlugins = { pseudoElementVariants: ({ addVariant }) => { @@ -80,7 +81,7 @@ export let variantPlugins = { }) }, - pseudoClassVariants: ({ addVariant, matchVariant, config }) => { + pseudoClassVariants: ({ addVariant, matchVariant, config, prefix }) => { let pseudoVariants = [ // Positional ['first', '&:first-child'], @@ -151,12 +152,12 @@ export let variantPlugins = { let variants = { group: (_, { modifier }) => modifier - ? [`:merge(.group\\/${escapeClassName(modifier)})`, ' &'] - : [`:merge(.group)`, ' &'], + ? [`:merge(${prefix('.group')}\\/${escapeClassName(modifier)})`, ' &'] + : [`:merge(${prefix('.group')})`, ' &'], peer: (_, { modifier }) => modifier - ? [`:merge(.peer\\/${escapeClassName(modifier)})`, ' ~ &'] - : [`:merge(.peer)`, ' ~ &'], + ? [`:merge(${prefix('.peer')}\\/${escapeClassName(modifier)})`, ' ~ &'] + : [`:merge(${prefix('.peer')})`, ' ~ &'], } for (let [name, fn] of Object.entries(variants)) { @@ -192,7 +193,12 @@ export let variantPlugins = { return result.slice(0, start) + a + result.slice(start + 1, end) + b + result.slice(end) }, - { values: Object.fromEntries(pseudoVariants) } + { + values: Object.fromEntries(pseudoVariants), + [INTERNAL_FEATURES]: { + respectPrefix: false, + }, + } ) } }, diff --git a/src/lib/generateRules.js b/src/lib/generateRules.js index cbd671075b29..6ced76937f1a 100644 --- a/src/lib/generateRules.js +++ b/src/lib/generateRules.js @@ -13,7 +13,7 @@ import { } from '../util/formatVariantSelector' import { asClass } from '../util/nameClass' import { normalize } from '../util/dataTypes' -import { isValidVariantFormatString, parseVariant } from './setupContextUtils' +import { isValidVariantFormatString, parseVariant, INTERNAL_FEATURES } from './setupContextUtils' import isValidArbitraryValue from '../util/isSyntacticallyValidPropertyValue' import { splitAtTopLevelOnly } from '../util/splitAtTopLevelOnly.js' import { flagEnabled } from '../featureFlags' @@ -230,9 +230,16 @@ function applyVariant(variant, matches, context) { if (context.variantMap.has(variant)) { let isArbitraryVariant = isArbitraryValue(variant) + let internalFeatures = context.variantOptions.get(variant)?.[INTERNAL_FEATURES] ?? {} let variantFunctionTuples = context.variantMap.get(variant).slice() let result = [] + let respectPrefix = (() => { + if (isArbitraryVariant) return false + if (internalFeatures.respectPrefix === false) return false + return true + })() + for (let [meta, rule] of matches) { // Don't generate variants for user css if (meta.layer === 'user') { @@ -293,7 +300,7 @@ function applyVariant(variant, matches, context) { format(selectorFormat) { collectedFormats.push({ format: selectorFormat, - isArbitraryVariant, + respectPrefix, }) }, args, @@ -322,7 +329,7 @@ function applyVariant(variant, matches, context) { if (typeof ruleWithVariant === 'string') { collectedFormats.push({ format: ruleWithVariant, - isArbitraryVariant, + respectPrefix, }) } @@ -366,7 +373,7 @@ function applyVariant(variant, matches, context) { // format: .foo & collectedFormats.push({ format: modified.replace(rebuiltBase, '&'), - isArbitraryVariant, + respectPrefix, }) rule.selector = before }) diff --git a/src/lib/setupContextUtils.js b/src/lib/setupContextUtils.js index 7844627fea73..4d85e9d8ce67 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -24,6 +24,8 @@ import { Offsets } from './offsets.js' import { flagEnabled } from '../featureFlags.js' import { finalizeSelector, formatVariantSelector } from '../util/formatVariantSelector' +export const INTERNAL_FEATURES = Symbol() + const VARIANT_TYPES = { AddVariant: Symbol.for('ADD_VARIANT'), MatchVariant: Symbol.for('MATCH_VARIANT'), @@ -1123,17 +1125,24 @@ function registerPlugins(plugins, context) { } let isArbitraryVariant = !(value in (options.values ?? {})) + let internalFeatures = options[INTERNAL_FEATURES] ?? {} + + let respectPrefix = (() => { + if (isArbitraryVariant) return false + if (internalFeatures.respectPrefix === false) return false + return true + })() formatStrings = formatStrings.map((format) => format.map((str) => ({ format: str, - isArbitraryVariant, + respectPrefix, })) ) manualFormatStrings = manualFormatStrings.map((format) => ({ format, - isArbitraryVariant, + respectPrefix, })) let opts = { diff --git a/src/util/formatVariantSelector.js b/src/util/formatVariantSelector.js index e3016e804779..d2594358e494 100644 --- a/src/util/formatVariantSelector.js +++ b/src/util/formatVariantSelector.js @@ -9,7 +9,7 @@ import { movePseudos } from './pseudoElements' /** @typedef {import('postcss-selector-parser').Pseudo} Pseudo */ /** @typedef {import('postcss-selector-parser').Node} Node */ -/** @typedef {{format: string, isArbitraryVariant: boolean}[]} RawFormats */ +/** @typedef {{format: string, respectPrefix: boolean}[]} RawFormats */ /** @typedef {import('postcss-selector-parser').Root} ParsedFormats */ /** @typedef {RawFormats | ParsedFormats} AcceptedFormats */ @@ -29,7 +29,7 @@ export function formatVariantSelector(formats, { context, candidate }) { return { ...format, - ast: format.isArbitraryVariant ? ast : prefixSelector(prefix, ast), + ast: format.respectPrefix ? prefixSelector(prefix, ast) : ast, } }) diff --git a/src/util/prefixSelector.js b/src/util/prefixSelector.js index 0e7bb445bdd9..93cbeb957682 100644 --- a/src/util/prefixSelector.js +++ b/src/util/prefixSelector.js @@ -17,6 +17,7 @@ export default function (prefix, selector, prependNegative = false) { return selector } + /** @type {import('postcss-selector-parser').Root} */ let ast = typeof selector === 'string' ? parser().astSync(selector) : selector ast.walkClasses((classSelector) => { diff --git a/tests/format-variant-selector.test.js b/tests/format-variant-selector.test.js index 9a53f3aacbeb..c94fb95f615c 100644 --- a/tests/format-variant-selector.test.js +++ b/tests/format-variant-selector.test.js @@ -6,7 +6,7 @@ crosscheck(() => { let selector = '.text-center' let candidate = 'hover:text-center' - let formats = [{ format: '&:hover', isArbitraryVariant: false }] + let formats = [{ format: '&:hover', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual('.hover\\:text-center:hover') }) @@ -16,8 +16,8 @@ crosscheck(() => { let candidate = 'focus:hover:text-center' let formats = [ - { format: '&:hover', isArbitraryVariant: false }, - { format: '&:focus', isArbitraryVariant: false }, + { format: '&:hover', respectPrefix: true }, + { format: '&:focus', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -29,7 +29,7 @@ crosscheck(() => { let selector = '.bg-\\[rgba\\(0\\,0\\,0\\)\\]' let candidate = 'hover:bg-[rgba(0,0,0)]' - let formats = [{ format: '&:hover', isArbitraryVariant: false }] + let formats = [{ format: '&:hover', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.hover\\:bg-\\[rgba\\(0\\2c 0\\2c 0\\)\\]:hover' @@ -40,7 +40,7 @@ crosscheck(() => { let selector = '.bg-\\[rgba\\(0\\2c 0\\2c 0\\)\\]' let candidate = 'hover:bg-[rgba(0,0,0)]' - let formats = [{ format: '&:hover', isArbitraryVariant: false }] + let formats = [{ format: '&:hover', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.hover\\:bg-\\[rgba\\(0\\2c 0\\2c 0\\)\\]:hover' @@ -51,7 +51,7 @@ crosscheck(() => { let selector = '.space-x-4 > :not([hidden]) ~ :not([hidden])' let candidate = 'hover:space-x-4' - let formats = [{ format: '&:hover', isArbitraryVariant: false }] + let formats = [{ format: '&:hover', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.hover\\:space-x-4:hover > :not([hidden]) ~ :not([hidden])' @@ -63,9 +63,9 @@ crosscheck(() => { let candidate = 'disabled:focus:hover:space-x-4' let formats = [ - { format: '&:hover', isArbitraryVariant: false }, - { format: '&:focus', isArbitraryVariant: false }, - { format: '&:disabled', isArbitraryVariant: false }, + { format: '&:hover', respectPrefix: true }, + { format: '&:focus', respectPrefix: true }, + { format: '&:disabled', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -77,7 +77,7 @@ crosscheck(() => { let selector = '.text-center' let candidate = 'group-hover:text-center' - let formats = [{ format: ':merge(.group):hover &', isArbitraryVariant: false }] + let formats = [{ format: ':merge(.group):hover &', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.group:hover .group-hover\\:text-center' @@ -89,8 +89,8 @@ crosscheck(() => { let candidate = 'group-focus:group-hover:text-center' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, - { format: ':merge(.group):focus &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, + { format: ':merge(.group):focus &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -102,7 +102,7 @@ crosscheck(() => { let selector = '.space-x-4 ~ :not([hidden]) ~ :not([hidden])' let candidate = 'group-hover:space-x-4' - let formats = [{ format: ':merge(.group):hover &', isArbitraryVariant: false }] + let formats = [{ format: ':merge(.group):hover &', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.group:hover .group-hover\\:space-x-4 ~ :not([hidden]) ~ :not([hidden])' @@ -114,8 +114,8 @@ crosscheck(() => { let candidate = 'group-focus:group-hover:space-x-4' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, - { format: ':merge(.group):focus &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, + { format: ':merge(.group):focus &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -128,7 +128,7 @@ crosscheck(() => { let candidate = 'peer-focus:group-hover:text-center' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, { format: ':merge(.peer):focus ~ &' }, ] @@ -142,8 +142,8 @@ crosscheck(() => { let candidate = 'group-hover:peer-focus:text-center' let formats = [ - { format: ':merge(.peer):focus ~ &', isArbitraryVariant: false }, - { format: ':merge(.group):hover &', isArbitraryVariant: false }, + { format: ':merge(.peer):focus ~ &', respectPrefix: true }, + { format: ':merge(.group):hover &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -156,10 +156,10 @@ crosscheck(() => { let candidate = 'peer-focus:group-focus:peer-hover:group-hover:foo' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, - { format: ':merge(.peer):hover ~ &', isArbitraryVariant: false }, - { format: ':merge(.group):focus &', isArbitraryVariant: false }, - { format: ':merge(.peer):focus ~ &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, + { format: ':merge(.peer):hover ~ &', respectPrefix: true }, + { format: ':merge(.group):focus &', respectPrefix: true }, + { format: ':merge(.peer):focus ~ &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -171,8 +171,8 @@ crosscheck(() => { let selector = '.text-center' let candidate = 'group-hover:prose-headings:text-center' let formats = [ - { format: ':where(&) :is(h1, h2, h3, h4)', isArbitraryVariant: false }, // Prose Headings - { format: ':merge(.group):hover &', isArbitraryVariant: false }, // Group Hover + { format: ':where(&) :is(h1, h2, h3, h4)', respectPrefix: true }, // Prose Headings + { format: ':merge(.group):hover &', respectPrefix: true }, // Group Hover ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -184,8 +184,8 @@ crosscheck(() => { let selector = '.text-center' let candidate = 'prose-headings:group-hover:text-center' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, // Group Hover - { format: ':where(&) :is(h1, h2, h3, h4)', isArbitraryVariant: false }, // Prose Headings + { format: ':merge(.group):hover &', respectPrefix: true }, // Group Hover + { format: ':where(&) :is(h1, h2, h3, h4)', respectPrefix: true }, // Prose Headings ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -197,12 +197,12 @@ crosscheck(() => { let selector = '.space-x-4 > :not([hidden]) ~ :not([hidden])' let candidate = 'peer-disabled:peer-first-child:group-hover:group-focus:focus:hover:space-x-4' let formats = [ - { format: '&:hover', isArbitraryVariant: false }, // Hover - { format: '&:focus', isArbitraryVariant: false }, // Focus - { format: ':merge(.group):focus &', isArbitraryVariant: false }, // Group focus - { format: ':merge(.group):hover &', isArbitraryVariant: false }, // Group hover - { format: ':merge(.peer):first-child ~ &', isArbitraryVariant: false }, // Peer first-child - { format: ':merge(.peer):disabled ~ &', isArbitraryVariant: false }, // Peer disabled + { format: '&:hover', respectPrefix: true }, // Hover + { format: '&:focus', respectPrefix: true }, // Focus + { format: ':merge(.group):focus &', respectPrefix: true }, // Group focus + { format: ':merge(.group):hover &', respectPrefix: true }, // Group hover + { format: ':merge(.peer):first-child ~ &', respectPrefix: true }, // Peer first-child + { format: ':merge(.peer):disabled ~ &', respectPrefix: true }, // Peer disabled ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -223,7 +223,7 @@ crosscheck(() => { let context = { tailwindConfig: { prefix: 'tw-' } } let selector = '.tw-text-center' let candidate = 'foo:tw-text-center' - let formats = [{ format: '.foo &', isArbitraryVariant: false }] + let formats = [{ format: '.foo &', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate, context })).toEqual( '.tw-foo .foo\\:tw-text-center' @@ -234,7 +234,7 @@ crosscheck(() => { let context = { tailwindConfig: { prefix: 'tw-' } } let selector = '.tw-text-center' let candidate = '[.foo_&]:tw-text-center' - let formats = [{ format: '.foo &', isArbitraryVariant: true }] + let formats = [{ format: '.foo &', respectPrefix: false }] expect(finalizeSelector(selector, formats, { candidate, context })).toEqual( '.foo .\\[\\.foo_\\&\\]\\:tw-text-center' @@ -247,8 +247,8 @@ crosscheck(() => { let selector = '.text-center' let candidate = 'text-center' let formats = [ - { format: ':merge(.group):focus > &', isArbitraryVariant: true }, - { format: ':merge(.group):hover &', isArbitraryVariant: true }, + { format: ':merge(.group):focus > &', respectPrefix: false }, + { format: ':merge(.group):hover &', respectPrefix: false }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -261,7 +261,7 @@ crosscheck(() => { let selector = '.placeholder-red-500::placeholder' let candidate = 'hover:placeholder-red-500' - let formats = [{ format: '&:hover', isArbitraryVariant: false }] + let formats = [{ format: '&:hover', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.hover\\:placeholder-red-500:hover::placeholder' @@ -273,8 +273,8 @@ crosscheck(() => { let candidate = 'group-hover:hover:space-x-4' let formats = [ - { format: '&:hover', isArbitraryVariant: false }, - { format: ':merge(.group):hover &', isArbitraryVariant: false }, + { format: '&:hover', respectPrefix: true }, + { format: ':merge(.group):hover &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -287,8 +287,8 @@ crosscheck(() => { let candidate = 'dark:group-hover:text-center' let formats = [ - { format: ':merge(.group):hover &', isArbitraryVariant: false }, - { format: '.dark &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, + { format: '.dark &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -302,7 +302,7 @@ crosscheck(() => { let formats = [ { format: '.dark &' }, - { format: ':merge(.group):hover &', isArbitraryVariant: false }, + { format: ':merge(.group):hover &', respectPrefix: true }, ] expect(finalizeSelector(selector, formats, { candidate })).toEqual( @@ -357,7 +357,7 @@ crosscheck(() => { ${'.parent::placeholder input'} | ${'.parent input::placeholder'} ${'.parent::backdrop dialog'} | ${'.parent dialog::backdrop'} `('should translate "$before" into "$after"', ({ before, after }) => { - let result = finalizeSelector('.a', [{ format: before, isArbitraryVariant: false }], { + let result = finalizeSelector('.a', [{ format: before, respectPrefix: true }], { candidate: 'a', }) diff --git a/tests/getVariants.test.js b/tests/getVariants.test.js index 8d79f82433a8..8c4022186a98 100644 --- a/tests/getVariants.test.js +++ b/tests/getVariants.test.js @@ -67,6 +67,25 @@ crosscheck(() => { expect(variant.selectors({ modifier: 'foo', value: '.foo_&' })).toEqual(['.foo .group\\/foo &']) }) + it('should provide selectors for complex matchVariant variants like `group` (when using a prefix)', () => { + let config = { prefix: 'tw-' } + let context = createContext(resolveConfig(config)) + + let variants = context.getVariants() + + let variant = variants.find((v) => v.name === 'group') + expect(variant.selectors()).toEqual(['.tw-group &']) + expect(variant.selectors({})).toEqual(['.tw-group &']) + expect(variant.selectors({ value: 'hover' })).toEqual(['.tw-group:hover &']) + expect(variant.selectors({ value: '.foo_&' })).toEqual(['.foo .tw-group &']) + expect(variant.selectors({ modifier: 'foo', value: 'hover' })).toEqual([ + '.tw-group\\/foo:hover &', + ]) + expect(variant.selectors({ modifier: 'foo', value: '.foo_&' })).toEqual([ + '.foo .tw-group\\/foo &', + ]) + }) + it('should provide selectors for variants with atrules', () => { let config = {} let context = createContext(resolveConfig(config)) diff --git a/tests/prefix.test.js b/tests/prefix.test.js index ea3d041ffeac..cf5085509d7a 100644 --- a/tests/prefix.test.js +++ b/tests/prefix.test.js @@ -623,3 +623,33 @@ crosscheck(({ stable, oxide }) => { `) }) }) + +test('does not prefix arbitrary group/peer classes', async () => { + let config = { + prefix: 'tw-', + content: [ + { + raw: html` +
+
+
+
+ `, + }, + ], + corePlugins: { preflight: false }, + } + + let input = css` + @tailwind utilities; + ` + + const result = await run(input, config) + + expect(result.css).toMatchFormattedCss(css` + .tw-group.foo .group-\[\&\.foo\]\:tw-flex, + .tw-peer.foo ~ .peer-\[\&\.foo\]\:tw-flex { + display: flex; + } + `) +})