Skip to content

Commit

Permalink
Merge pull request #759 from ben-rogerson/bugfix/improved-variant-ord…
Browse files Browse the repository at this point in the history
…ering

Improved variant ordering
  • Loading branch information
ben-rogerson authored Nov 19, 2022
2 parents 6920c23 + 88ea453 commit 02d7824
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 121 deletions.
61 changes: 16 additions & 45 deletions src/core/lib/convertClassName.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,17 @@ import replaceThemeValue from './util/replaceThemeValue'
import isShortCss from './util/isShortCss'
import splitOnFirst from './util/splitOnFirst'
import { splitAtTopLevelOnly } from './util/twImports'
import type {
Assert,
AssertContext,
CoreContext,
TailwindConfig,
} from 'core/types'
import type { AssertContext, CoreContext, TailwindConfig } from 'core/types'
// eslint-disable-next-line import/no-relative-parent-imports
import { SPACE_ID, SPACES } from '../constants'

const ALL_COMMAS = /,/g
const ALL_AMPERSANDS = /&/g
const ENDING_AMP_THEN_WHITESPACE = /&[\s_]*$/
const ALL_CLASS_DOTS = /(?<!\\)(\.)(?=\w)/g
const ALL_CLASS_ATS = /(?<!\\)(@)(?=\w)(?!media)/g
const ALL_WRAPPABLE_PARENT_SELECTORS = /&(?=([^ $)*+,.:>[_~])[\w-])/g
const BASIC_SELECTOR_TYPES = /^#|^\\./
const BASIC_SELECTOR_TYPES = /^#|^\\.|[^\W_]/

type ConvertShortCssToArbitraryPropertyParameters = {
disableShortCss: CoreContext['twinConfig']['disableShortCss']
Expand Down Expand Up @@ -166,7 +162,7 @@ function convertClassName(
className = replaceThemeValue(className, { assert, theme })

// Add missing parent selectors and collapse arbitrary variants
className = sassifyArbitraryVariants(className, { assert, tailwindConfig })
className = sassifyArbitraryVariants(className, { tailwindConfig })

debug('class after format', className)

Expand All @@ -183,7 +179,7 @@ function unbracket(variant: string): string {

function sassifyArbitraryVariants(
fullClassName: string,
{ assert, tailwindConfig }: { assert: Assert; tailwindConfig: TailwindConfig }
{ tailwindConfig }: { tailwindConfig: TailwindConfig }
): string {
const splitArray = [
...splitAtTopLevelOnly(fullClassName, tailwindConfig.separator ?? ':'),
Expand Down Expand Up @@ -249,36 +245,17 @@ function sassifyArbitraryVariants(
collapsed[collapsed.length - 1] = mergedWithPrev
})

// Use that list to add the parent selector
let hasArbitraryVariant = false
let hasNormalVariant = false
const variantsWithParentSelectors = collapsed.map((v, idx) => {
if (!isArbitraryVariant(v)) {
if (idx > 0 && hasArbitraryVariant) hasNormalVariant = true
return v
}

// The ordering gets screwy in the selector when using a mix of arbitrary variants, normal variants and the auto parent feature - so notify in that case
const isMissingParentSelectorOkay =
hasArbitraryVariant && hasNormalVariant && !v.includes('&')
assert(
!isMissingParentSelectorOkay,
({ color }: AssertContext) =>
`${color(
`✕ ${String(
color(fullClassName, 'errorLight')
)} had trouble with the auto parent selector feature`
)}\n\nYou’ll need to add the parent selector manually within the arbitrary variant(s), eg: ${String(
color(`[section &]:block`, 'success')
)}`
)

hasArbitraryVariant = true
// The supplied class requires the reversal of it's variants as resolveMatches adds them in reverse order
const reversedVariantList = [...collapsed].slice().reverse()
const allVariants = reversedVariantList.map((v, idx) => {
if (!isArbitraryVariant(v)) return v

const unwrappedVariant = unbracket(v)
// Escape class dots in the selector - otherwise tailwindcss adds the prefix within arbitrary variants (only when `prefix` is set in tailwind config)
// Unescaped dots incorrectly add the prefix within arbitrary variants (only when`prefix` is set in tailwind config)
// eg: tw`[.a]:first:tw-block` -> `.tw-a &:first-child`
.replace(ALL_CLASS_DOTS, '\\.')
// Unescaped ats will throw a conversion error
.replace(ALL_CLASS_ATS, '\\@')

const variantList = unwrappedVariant.startsWith('@')
? [unwrappedVariant]
Expand All @@ -297,9 +274,7 @@ function sassifyArbitraryVariants(
return `[${out}]`
})

return [...variantsWithParentSelectors, className].join(
tailwindConfig.separator ?? ':'
)
return [...allVariants, className].join(tailwindConfig.separator ?? ':')
}

function addParentSelector(
Expand All @@ -311,18 +286,14 @@ function addParentSelector(
if (selector.includes('&') || selector.startsWith('@')) return selector

// Arbitrary variants
// Variants that start with a class/id get treated as a child
if (BASIC_SELECTOR_TYPES.test(selector) && !prev) return `& ${selector}`
// Pseudo elements get an auto parent selector prefixed
if (selector.startsWith(':')) return `&${selector}`
// Variants that start with a class/id get treated as a child
if (BASIC_SELECTOR_TYPES.test(selector) && !prev) return `& ${selector}`
// When there's more than one variant and it's at the end then prefix it
if (!next && prev) return `&${selector}`
// When a non arbitrary variant follows then we combine it with the current
// selector by adding the parent selector at the end
// eg: `[input&]:focus:...` -> `input:focus:...`
if (next && !isArbitraryVariant(next)) return `${selector}&`

return `&_${selector}`
return `& ${selector}`
}

export default convertClassName
7 changes: 4 additions & 3 deletions src/core/lib/util/sassifySelector.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { ExtractRuleStyles } from 'core/types'

const SELECTOR_PARENT_CANDIDATE = /^[ #.[]/
const SELECTOR_SPECIAL_STARTS = /^ [>@]/
const SELECTOR_ROOT = /(^| ):root(?!\w)/g
const UNDERSCORE_ESCAPING = /\\+(_)/g
const WRAPPED_PARENT_SELECTORS = /(\({3}&(.*?)\){3})/g
Expand Down Expand Up @@ -58,11 +59,11 @@ const sassifySelectorTasks: SassifySelectorTasks = [
if (selector.includes('&')) return selector

const addParentSelector = SELECTOR_PARENT_CANDIDATE.test(selector)

if (!addParentSelector) return selector

// ` > :not([hidden]) ~ :not([hidden])` / ` > *`
if (selector.startsWith(' >')) return selector
// Fix: ` > :not([hidden]) ~ :not([hidden])` / ` > *`
// Fix: `[@page]:x`
if (SELECTOR_SPECIAL_STARTS.test(selector)) return selector

return `&${selector}`
},
Expand Down
82 changes: 41 additions & 41 deletions tests/__snapshots__/plugin.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ const StatePlaceholderImportant = _styled.input({
})

const StateStatePlaceholderImportant = _styled.input({
':hover:active::placeholder': {
':active:hover::placeholder': {
'--tw-placeholder-opacity': '1 !important',
color: 'rgb(239 68 68 / var(--tw-placeholder-opacity)) !important',
},
Expand Down Expand Up @@ -264,7 +264,7 @@ const StatePlaceholderImportantPrefix = _styled.input({
})

const StateStatePlaceholderImportantPrefix = _styled.input({
':hover:active::placeholder': {
':active:hover::placeholder': {
'--tw-placeholder-opacity': '1 !important',
color: 'rgb(239 68 68 / var(--tw-placeholder-opacity)) !important',
},
Expand Down Expand Up @@ -1222,7 +1222,7 @@ const nestedGroups = {
display: 'inline',
backgroundColor: 'rgb(0 0 0 / .20)',
},
':last-child:first-child': {
':first-child:last-child': {
display: 'block',
backgroundColor: 'rgb(0 0 0 / .20)',
},
Expand All @@ -1236,13 +1236,13 @@ const nestedGroups = {
;({
'@media (min-width: 768px)': {
marginLeft: '1rem !important',
one: {
'& one': {
marginTop: '1.25rem !important',
},
'one two': {
'& one two': {
display: 'inline !important',
},
'one two three': {
'& one two three': {
display: 'inline !important',
},
},
Expand Down Expand Up @@ -1935,31 +1935,31 @@ tw\`xl:placeholder-red-500! first:md:block sm:disabled:flex\`
},
})
;({
'@media (prefers-color-scheme: dark)': {
'@media (min-width: 640px)': {
'@media (min-width: 640px)': {
'@media (prefers-color-scheme: dark)': {
display: 'block',
},
},
})
;({
'@media (prefers-color-scheme: light)': {
'@media (min-width: 640px)': {
'@media (min-width: 640px)': {
'@media (prefers-color-scheme: light)': {
display: 'block',
},
},
})
;({
'@media (prefers-color-scheme: dark)': {
'@media (min-width: 640px)': {
'@media (min-width: 640px)': {
'@media (prefers-color-scheme: dark)': {
'.group:hover &': {
display: 'block',
},
},
},
})
;({
'@media (prefers-color-scheme: light)': {
'@media (min-width: 640px)': {
'@media (min-width: 640px)': {
'@media (prefers-color-scheme: light)': {
'.group:hover &': {
display: 'block',
},
Expand Down Expand Up @@ -4171,7 +4171,7 @@ tw\`[&.pre,& section,]:block\`
},
})
;({
'p:hover': {
':hover p': {
display: 'block',
},
})
Expand Down Expand Up @@ -4268,7 +4268,7 @@ tw\`[&.pre,& section,]:block\`
})
;({
'@media (min-width: 768px)': {
section: {
'& section': {
display: 'block',
},
},
Expand All @@ -4289,32 +4289,32 @@ tw\`[&.pre,& section,]:block\`
},
})
;({
'section pre &:first-child': {
'pre section &:first-child': {
display: 'block',
},
})
;({
'section & pre:first-child': {
'section &:first-child pre': {
display: 'block',
},
})
;({
'pre section:first-child': {
':first-child pre section': {
display: 'block',
},
})
;({
'section pre:first-child': {
':first-child section pre': {
display: 'block',
},
})
;({
'section pre:first-child': {
':first-child section pre': {
marginTop: '2px',
},
})
;({
'section pre:first-child': {
':first-child section pre': {
display: 'inline',
},
})
Expand Down Expand Up @@ -4343,7 +4343,7 @@ tw\`[&.pre,& section,]:block\`
stroke: '#000',
},
'@media (min-width: 768px)': {
path: {
'& path': {
stroke: '#000',
},
},
Expand All @@ -4353,7 +4353,7 @@ tw\`[&.pre,& section,]:block\`
display: 'block',
},
'@media (min-width: 768px)': {
path: {
'& path': {
stroke: '#000',
},
},
Expand Down Expand Up @@ -58450,10 +58450,10 @@ const addUtilitiesTest2Media = {
},
}
const addUtilitiesTest2Variants = {
':visited:nth-child(even)': {
':nth-child(even):visited': {
transform: 'skewY(-15deg)',
},
':active:hover': {
':hover:active': {
transform: 'skewY(-15deg)',
},
}
Expand Down Expand Up @@ -58489,14 +58489,14 @@ const addComponentsTestMedia = {
},
}
const addComponentsTestVariants = {
':visited:nth-child(even)': {
':nth-child(even):visited': {
backgroundColor: '#e3342f',
color: '#fff',
},
':visited:nth-child(even):hover': {
':nth-child(even):visited:hover': {
backgroundColor: '#cc1f1a',
},
':active:hover': {
':hover:active': {
padding: '.5rem 1rem',
borderRadius: '.25rem',
fontWeight: '600',
Expand Down Expand Up @@ -78440,12 +78440,12 @@ tw\`any-pointer-fine:portrait:print:dark:contrast-more:motion-safe:rtl:valid:bef

// @ts-nocheck
;({
'@media (prefers-reduced-motion: no-preference)': {
'@media (prefers-contrast: more)': {
'@media (prefers-color-scheme: dark)': {
'@media print': {
'@media (orientation: portrait)': {
'@media (any-pointer: fine)': {
'@media (any-pointer: fine)': {
'@media (orientation: portrait)': {
'@media print': {
'@media (prefers-color-scheme: dark)': {
'@media (prefers-contrast: more)': {
'@media (prefers-reduced-motion: no-preference)': {
'[dir="rtl"] &:valid::before': {
content: 'var(--tw-content)',
display: 'block',
Expand All @@ -78458,12 +78458,12 @@ tw\`any-pointer-fine:portrait:print:dark:contrast-more:motion-safe:rtl:valid:bef
},
})
;({
'@media (any-pointer: fine)': {
'@media (orientation: portrait)': {
'@media print': {
'@media (prefers-color-scheme: dark)': {
'@media (prefers-contrast: more)': {
'@media (prefers-reduced-motion: no-preference)': {
'@media (prefers-reduced-motion: no-preference)': {
'@media (prefers-contrast: more)': {
'@media (prefers-color-scheme: dark)': {
'@media print': {
'@media (orientation: portrait)': {
'@media (any-pointer: fine)': {
'[dir="rtl"] &:valid::before': {
content: 'var(--tw-content)',
marginTop: '1.25rem',
Expand Down
4 changes: 2 additions & 2 deletions tests/arbitraryProperties.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ test('arbitrary properties with modifiers', async () => {
return run(input).then(result => {
expect(result).toMatchFormattedJavaScript(`
({
'@media (prefers-color-scheme: dark)': {
'@media (min-width: 1024px)': { ':hover': { paintOrder: 'markers' } },
'@media (min-width: 1024px)': {
'@media (prefers-color-scheme: dark)': { ':hover': { paintOrder: 'markers' } },
},
})
`)
Expand Down
Loading

0 comments on commit 02d7824

Please sign in to comment.