Skip to content

Commit

Permalink
Don't prefix classes in arbitrary variants (#10214)
Browse files Browse the repository at this point in the history
* Add tests

* Refactor

refactor

* Allow `prefixSelector` to take an AST

* Consider multiple formats in `finalizeSelector`

The functions `finalizeSelector` and `formatVariantSelector` together were using a mix for AST and string-based parsing. This now does the full transformation using the selector AST. This also parses the format strings AST as early as possible and is set up to parse them only once for a given set of rules.

All of this will allow considering metadata per format string. For instance, we now know if the format string `.foo &` was produced by a normal variant or by an arbitrary variant. We use this information to control the prefixing behavior for individual format strings.

* Update changelog

* Cleanup code a bit
  • Loading branch information
thecrypticace committed Jan 3, 2023
1 parent 2b885ef commit 7d8eb21
Show file tree
Hide file tree
Showing 7 changed files with 487 additions and 249 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Update list of length units ([#10100](https://github.com/tailwindlabs/tailwindcss/pull/10100))
- Fix not matching arbitrary properties when closely followed by square brackets ([#10212](https://github.com/tailwindlabs/tailwindcss/pull/10212))
- Allow direct nesting in `root` or `@layer` nodes ([#10229](https://github.com/tailwindlabs/tailwindcss/pull/10229))
- Don't prefix classes in arbitrary variants ([#10214](https://github.com/tailwindlabs/tailwindcss/pull/10214))

### Changed

Expand Down
116 changes: 74 additions & 42 deletions src/lib/generateRules.js
Expand Up @@ -201,6 +201,7 @@ function applyVariant(variant, matches, context) {
}

if (context.variantMap.has(variant)) {
let isArbitraryVariant = isArbitraryValue(variant)
let variantFunctionTuples = context.variantMap.get(variant).slice()
let result = []

Expand Down Expand Up @@ -262,7 +263,10 @@ function applyVariant(variant, matches, context) {
clone.append(wrapper)
},
format(selectorFormat) {
collectedFormats.push(selectorFormat)
collectedFormats.push({
format: selectorFormat,
isArbitraryVariant,
})
},
args,
})
Expand All @@ -288,7 +292,10 @@ function applyVariant(variant, matches, context) {
}

if (typeof ruleWithVariant === 'string') {
collectedFormats.push(ruleWithVariant)
collectedFormats.push({
format: ruleWithVariant,
isArbitraryVariant,
})
}

if (ruleWithVariant === null) {
Expand Down Expand Up @@ -329,7 +336,10 @@ function applyVariant(variant, matches, context) {
// modified (by plugin): .foo .foo\\:markdown > p
// rebuiltBase (internal): .foo\\:markdown > p
// format: .foo &
collectedFormats.push(modified.replace(rebuiltBase, '&'))
collectedFormats.push({
format: modified.replace(rebuiltBase, '&'),
isArbitraryVariant,
})
rule.selector = before
})
}
Expand All @@ -349,7 +359,6 @@ function applyVariant(variant, matches, context) {
Object.assign(args, context.variantOptions.get(variant))
),
collectedFormats: (meta.collectedFormats ?? []).concat(collectedFormats),
isArbitraryVariant: isArbitraryValue(variant),
},
clone.nodes[0],
]
Expand Down Expand Up @@ -733,48 +742,15 @@ function* resolveMatches(candidate, context, original = candidate) {
}

for (let match of matches) {
let isValid = true

match[1].raws.tailwind = { ...match[1].raws.tailwind, candidate }

// Apply final format selector
if (match[0].collectedFormats) {
let finalFormat = formatVariantSelector('&', ...match[0].collectedFormats)
let container = postcss.root({ nodes: [match[1].clone()] })
container.walkRules((rule) => {
if (inKeyframes(rule)) return

let selectorOptions = {
selector: rule.selector,
candidate: original,
base: candidate
.split(new RegExp(`\\${context?.tailwindConfig?.separator ?? ':'}(?![^[]*\\])`))
.pop(),
isArbitraryVariant: match[0].isArbitraryVariant,

context,
}

try {
rule.selector = finalizeSelector(finalFormat, selectorOptions)
} catch {
// The selector we produced is invalid
// This could be because:
// - A bug exists
// - A plugin introduced an invalid variant selector (ex: `addVariant('foo', '&;foo')`)
// - The user used an invalid arbitrary variant (ex: `[&;foo]:underline`)
// Either way the build will fail because of this
// We would rather that the build pass "silently" given that this could
// happen because of picking up invalid things when scanning content
// So we'll throw out the candidate instead
isValid = false
return false
}
})
match[1] = container.nodes[0]
}
match = applyFinalFormat(match, { context, candidate, original })

if (!isValid) {
// Skip rules with invalid selectors
// This will cause the candidate to be added to the "not class"
// cache skipping it entirely for future builds
if (match === null) {
continue
}

Expand All @@ -783,6 +759,62 @@ function* resolveMatches(candidate, context, original = candidate) {
}
}

function applyFinalFormat(match, { context, candidate, original }) {
if (!match[0].collectedFormats) {
return match
}

let isValid = true
let finalFormat

try {
finalFormat = formatVariantSelector(match[0].collectedFormats, {
context,
candidate,
})
} catch {
// The format selector we produced is invalid
// This could be because:
// - A bug exists
// - A plugin introduced an invalid variant selector (ex: `addVariant('foo', '&;foo')`)
// - The user used an invalid arbitrary variant (ex: `[&;foo]:underline`)
// Either way the build will fail because of this
// We would rather that the build pass "silently" given that this could
// happen because of picking up invalid things when scanning content
// So we'll throw out the candidate instead

return null
}

let container = postcss.root({ nodes: [match[1].clone()] })

container.walkRules((rule) => {
if (inKeyframes(rule)) {
return
}

try {
rule.selector = finalizeSelector(rule.selector, finalFormat, {
candidate: original,
context,
})
} catch {
// If this selector is invalid we also want to skip it
// But it's likely that being invalid here means there's a bug in a plugin rather than too loosely matching content
isValid = false
return false
}
})

if (!isValid) {
return null
}

match[1] = container.nodes[0]

return match
}

function inKeyframes(rule) {
return rule.parent && rule.parent.type === 'atrule' && rule.parent.name === 'keyframes'
}
Expand Down
34 changes: 26 additions & 8 deletions src/lib/setupContextUtils.js
Expand Up @@ -1080,20 +1080,38 @@ function registerPlugins(plugins, context) {
})
}

let result = formatStrings.map((formatString) =>
finalizeSelector(formatVariantSelector('&', ...formatString), {
selector: `.${candidate}`,
candidate,
context,
isArbitraryVariant: !(value in (options.values ?? {})),
})
let isArbitraryVariant = !(value in (options.values ?? {}))

formatStrings = formatStrings.map((format) =>
format.map((str) => ({
format: str,
isArbitraryVariant,
}))
)

manualFormatStrings = manualFormatStrings.map((format) => ({
format,
isArbitraryVariant,
}))

let opts = {
candidate,
context,
}

let result = formatStrings.map((formats) =>
finalizeSelector(`.${candidate}`, formatVariantSelector(formats, opts), opts)
.replace(`.${candidate}`, '&')
.replace('{ & }', '')
.trim()
)

if (manualFormatStrings.length > 0) {
result.push(formatVariantSelector('&', ...manualFormatStrings))
result.push(
formatVariantSelector(manualFormatStrings, opts)
.toString()
.replace(`.${candidate}`, '&')
)
}

return result
Expand Down

0 comments on commit 7d8eb21

Please sign in to comment.