From 615c157a271d0a043d9f45471850aaa49d081604 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 --- 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 | 89 +++++++++++++-------------- tests/getVariants.test.js | 19 ++++++ tests/prefix.test.js | 30 +++++++++ 8 files changed, 129 insertions(+), 60 deletions(-) diff --git a/src/corePlugins.js b/src/corePlugins.js index 8d0fa13e5e7e..683b6ef32526 100644 --- a/src/corePlugins.js +++ b/src/corePlugins.js @@ -21,6 +21,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 }) => { @@ -79,7 +80,7 @@ export let variantPlugins = { }) }, - pseudoClassVariants: ({ addVariant, matchVariant, config }) => { + pseudoClassVariants: ({ addVariant, matchVariant, config, prefix }) => { let pseudoVariants = [ // Positional ['first', '&:first-child'], @@ -150,12 +151,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)) { @@ -191,7 +192,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 67344d370f41..b3d6b1a2cc55 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' @@ -226,9 +226,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') { @@ -289,7 +296,7 @@ function applyVariant(variant, matches, context) { format(selectorFormat) { collectedFormats.push({ format: selectorFormat, - isArbitraryVariant, + respectPrefix, }) }, args, @@ -318,7 +325,7 @@ function applyVariant(variant, matches, context) { if (typeof ruleWithVariant === 'string') { collectedFormats.push({ format: ruleWithVariant, - isArbitraryVariant, + respectPrefix, }) } @@ -362,7 +369,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 0d6bc2509ded..0bd5639833c7 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -23,6 +23,8 @@ import { hasContentChanged } from './cacheInvalidation.js' import { Offsets } from './offsets.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'), @@ -1110,17 +1112,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 3873f5d2df1e..9ad2c632e2db 100644 --- a/tests/format-variant-selector.test.js +++ b/tests/format-variant-selector.test.js @@ -4,7 +4,7 @@ it('should be possible to add a simple variant to a simple selector', () => { 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') }) @@ -14,8 +14,8 @@ it('should be possible to add a multiple simple variants to a simple selector', 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( @@ -27,7 +27,7 @@ it('should be possible to add a simple variant to a selector containing escaped 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' @@ -38,7 +38,7 @@ it('should be possible to add a simple variant to a selector containing escaped 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' @@ -49,7 +49,7 @@ it('should be possible to add a simple variant to a more complex selector', () = 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])' @@ -61,9 +61,9 @@ it('should be possible to add multiple simple variants to a more complex selecto 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( @@ -75,7 +75,7 @@ it('should be possible to add a single merge variant to a simple selector', () = 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' @@ -87,8 +87,8 @@ it('should be possible to add multiple merge variants to a simple selector', () 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( @@ -100,7 +100,7 @@ it('should be possible to add a single merge variant to a more complex selector' 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])' @@ -112,8 +112,8 @@ it('should be possible to add multiple merge variants to a more complex selector 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( @@ -126,7 +126,7 @@ it('should be possible to add multiple unique merge variants to a simple selecto 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 ~ &' }, ] @@ -140,8 +140,8 @@ it('should be possible to add multiple unique merge variants to a simple selecto 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( @@ -154,10 +154,10 @@ it('should be possible to use multiple :merge() calls with different "arguments" 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( @@ -169,8 +169,8 @@ it('group hover and prose headings combination', () => { 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( @@ -182,8 +182,8 @@ it('group hover and prose headings combination flipped', () => { 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( @@ -195,12 +195,12 @@ it('should be possible to handle a complex utility', () => { 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( @@ -221,7 +221,7 @@ it('should prefix classes from variants', () => { 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' @@ -232,7 +232,7 @@ it('should not prefix classes from arbitrary variants', () => { 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' @@ -245,8 +245,8 @@ it('Merged selectors with mixed combinators uses the first one', () => { 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( @@ -259,7 +259,7 @@ describe('real examples', () => { 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' @@ -271,8 +271,8 @@ describe('real examples', () => { 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( @@ -285,8 +285,8 @@ describe('real examples', () => { 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( @@ -298,10 +298,7 @@ describe('real examples', () => { let selector = '.text-center' let candidate = 'group-hover:dark:text-center' - let formats = [ - { format: '.dark &' }, - { format: ':merge(.group):hover &', isArbitraryVariant: false }, - ] + let formats = [{ format: '.dark &' }, { format: ':merge(.group):hover &', respectPrefix: true }] expect(finalizeSelector(selector, formats, { candidate })).toEqual( '.group:hover .dark .group-hover\\:dark\\:text-center' @@ -355,7 +352,7 @@ describe('pseudo elements', () => { ${'.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 114d80824df6..16f13975f648 100644 --- a/tests/getVariants.test.js +++ b/tests/getVariants.test.js @@ -65,6 +65,25 @@ it('should provide selectors for complex matchVariant variants like `group`', () 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 c9c0e5f258ac..79b8413e8eb0 100644 --- a/tests/prefix.test.js +++ b/tests/prefix.test.js @@ -607,3 +607,33 @@ test('supports non-word prefixes (2)', async () => { } `) }) + +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; + } + `) +})