From 2abf22812411d95f2cce2dfcb5dadacd8a175bce Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 18 Oct 2024 22:44:25 +0200 Subject: [PATCH] Minify arbitrary values when printing candidates (#14720) This PR will optimize and simplify the candidates when printing the candidate again after running codemods. When we parse a candidate, we will add spaces around operators, for example `p-[calc(1px+1px)]]` will internally be handled as `calc(1px + 1px)`. Before this change, we would re-print this as: `p-[calc(1px_+_1px)]`. This PR changes that by simplifying the candidate again so that the output is `p-[calc(1px+1px)]`. In addition, if _you_ wrote `p-[calc(1px_+_1px)]` then we will also simplify it to the concise form `p-[calc(1px_+_1px)]`. Some examples: Input: ```html
``` Output before: ```html
``` Output after: ```html
``` --- This is alternative implementation to #14717 and #14718 Closes: #14717 Closes: #14718 --- CHANGELOG.md | 1 + .../src/template/candidates.test.ts | 65 +++++++++---- .../src/template/candidates.ts | 93 +++++++++++++++++-- .../src/template/codemods/important.test.ts | 2 +- .../template/codemods/theme-to-var.test.ts | 2 +- packages/tailwindcss/src/value-parser.ts | 8 +- 6 files changed, 137 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ad3fa3e2..1bbc4667a 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 - Allow spaces spaces around operators in attribute selector variants ([#14703](https://github.com/tailwindlabs/tailwindcss/pull/14703)) - _Upgrade (experimental)_: Migrate `flex-grow` to `grow` and `flex-shrink` to `shrink` ([#14721](https://github.com/tailwindlabs/tailwindcss/pull/14721)) +- _Upgrade (experimental)_: Minify arbitrary values when printing candidates ([#14720](https://github.com/tailwindlabs/tailwindcss/pull/14720)) ### Changed diff --git a/packages/@tailwindcss-upgrade/src/template/candidates.test.ts b/packages/@tailwindcss-upgrade/src/template/candidates.test.ts index dbab76ddd..296a5cc5e 100644 --- a/packages/@tailwindcss-upgrade/src/template/candidates.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/candidates.test.ts @@ -108,40 +108,65 @@ const candidates = [ ['bg-[#0088cc]/[0.5]', 'bg-[#0088cc]/[0.5]'], ['bg-[#0088cc]!', 'bg-[#0088cc]!'], ['!bg-[#0088cc]', 'bg-[#0088cc]!'], + ['bg-[var(--spacing)-1px]', 'bg-[var(--spacing)-1px]'], + ['bg-[var(--spacing)_-_1px]', 'bg-[var(--spacing)-1px]'], + ['bg-[-1px_-1px]', 'bg-[-1px_-1px]'], + ['p-[round(to-zero,1px)]', 'p-[round(to-zero,1px)]'], ['w-1/2', 'w-1/2'], + ['p-[calc((100vw-theme(maxWidth.2xl))_/_2)]', 'p-[calc((100vw-theme(maxWidth.2xl))/2)]'], + + // Keep spaces in strings + ['content-["hello_world"]', 'content-["hello_world"]'], + ['content-[____"hello_world"___]', 'content-["hello_world"]'], ] const variants = [ - '', // no variant - '*:', - 'focus:', - 'group-focus:', + ['', ''], // no variant + ['*:', '*:'], + ['focus:', 'focus:'], + ['group-focus:', 'group-focus:'], - 'hover:focus:', - 'hover:group-focus:', - 'group-hover:focus:', - 'group-hover:group-focus:', + ['hover:focus:', 'hover:focus:'], + ['hover:group-focus:', 'hover:group-focus:'], + ['group-hover:focus:', 'group-hover:focus:'], + ['group-hover:group-focus:', 'group-hover:group-focus:'], - 'min-[10px]:', - // TODO: This currently expands `calc(1000px+12em)` to `calc(1000px_+_12em)` - 'min-[calc(1000px_+_12em)]:', + ['min-[10px]:', 'min-[10px]:'], - 'peer-[&_p]:', - 'peer-[&_p]:hover:', - 'hover:peer-[&_p]:', - 'hover:peer-[&_p]:focus:', - 'peer-[&:hover]:peer-[&_p]:', + // Normalize spaces + ['min-[calc(1000px_+_12em)]:', 'min-[calc(1000px+12em)]:'], + ['min-[calc(1000px_+12em)]:', 'min-[calc(1000px+12em)]:'], + ['min-[calc(1000px+_12em)]:', 'min-[calc(1000px+12em)]:'], + ['min-[calc(1000px___+___12em)]:', 'min-[calc(1000px+12em)]:'], + + ['peer-[&_p]:', 'peer-[&_p]:'], + ['peer-[&_p]:hover:', 'peer-[&_p]:hover:'], + ['hover:peer-[&_p]:', 'hover:peer-[&_p]:'], + ['hover:peer-[&_p]:focus:', 'hover:peer-[&_p]:focus:'], + ['peer-[&:hover]:peer-[&_p]:', 'peer-[&:hover]:peer-[&_p]:'], + + ['[p]:', '[p]:'], + ['[_p_]:', '[p]:'], + ['has-[p]:', 'has-[p]:'], + ['has-[_p_]:', 'has-[p]:'], + + // Simplify `&:is(p)` to `p` + ['[&:is(p)]:', '[p]:'], + ['[&:is(_p_)]:', '[p]:'], + ['has-[&:is(p)]:', 'has-[p]:'], + ['has-[&:is(_p_)]:', 'has-[p]:'], ] let combinations: [string, string][] = [] -for (let variant of variants) { - for (let [input, output] of candidates) { - combinations.push([`${variant}${input}`, `${variant}${output}`]) + +for (let [inputVariant, outputVariant] of variants) { + for (let [inputCandidate, outputCandidate] of candidates) { + combinations.push([`${inputVariant}${inputCandidate}`, `${outputVariant}${outputCandidate}`]) } } describe('printCandidate()', () => { - test.each(combinations)('%s', async (candidate: string, result: string) => { + test.each(combinations)('%s -> %s', async (candidate: string, result: string) => { let designSystem = await __unstable__loadDesignSystem('@import "tailwindcss";', { base: __dirname, }) diff --git a/packages/@tailwindcss-upgrade/src/template/candidates.ts b/packages/@tailwindcss-upgrade/src/template/candidates.ts index 87d4e345e..b97ff8ed0 100644 --- a/packages/@tailwindcss-upgrade/src/template/candidates.ts +++ b/packages/@tailwindcss-upgrade/src/template/candidates.ts @@ -1,6 +1,7 @@ import { Scanner } from '@tailwindcss/oxide' import type { Candidate, Variant } from '../../../tailwindcss/src/candidate' import type { DesignSystem } from '../../../tailwindcss/src/design-system' +import * as ValueParser from '../../../tailwindcss/src/value-parser' export async function extractRawCandidates( content: string, @@ -51,9 +52,9 @@ export function printCandidate(designSystem: DesignSystem, candidate: Candidate) if (candidate.value === null) { base += '' } else if (candidate.value.dataType) { - base += `-[${candidate.value.dataType}:${escapeArbitrary(candidate.value.value)}]` + base += `-[${candidate.value.dataType}:${printArbitraryValue(candidate.value.value)}]` } else { - base += `-[${escapeArbitrary(candidate.value.value)}]` + base += `-[${printArbitraryValue(candidate.value.value)}]` } } else if (candidate.value.kind === 'named') { base += `-${candidate.value.value}` @@ -63,14 +64,14 @@ export function printCandidate(designSystem: DesignSystem, candidate: Candidate) // Handle arbitrary if (candidate.kind === 'arbitrary') { - base += `[${candidate.property}:${escapeArbitrary(candidate.value)}]` + base += `[${candidate.property}:${printArbitraryValue(candidate.value)}]` } // Handle modifier if (candidate.kind === 'arbitrary' || candidate.kind === 'functional') { if (candidate.modifier) { if (candidate.modifier.kind === 'arbitrary') { - base += `/[${escapeArbitrary(candidate.modifier.value)}]` + base += `/[${printArbitraryValue(candidate.modifier.value)}]` } else if (candidate.modifier.kind === 'named') { base += `/${candidate.modifier.value}` } @@ -95,7 +96,7 @@ function printVariant(variant: Variant) { // Handle arbitrary variants if (variant.kind === 'arbitrary') { - return `[${escapeArbitrary(variant.selector)}]` + return `[${printArbitraryValue(simplifyArbitraryVariant(variant.selector))}]` } let base: string = '' @@ -105,7 +106,7 @@ function printVariant(variant: Variant) { base += variant.root if (variant.value) { if (variant.value.kind === 'arbitrary') { - base += `-[${escapeArbitrary(variant.value.value)}]` + base += `-[${printArbitraryValue(variant.value.value)}]` } else if (variant.value.kind === 'named') { base += `-${variant.value.value}` } @@ -123,7 +124,7 @@ function printVariant(variant: Variant) { if (variant.kind === 'functional' || variant.kind === 'compound') { if (variant.modifier) { if (variant.modifier.kind === 'arbitrary') { - base += `/[${escapeArbitrary(variant.modifier.value)}]` + base += `/[${printArbitraryValue(variant.modifier.value)}]` } else if (variant.modifier.kind === 'named') { base += `/${variant.modifier.value}` } @@ -133,8 +134,82 @@ function printVariant(variant: Variant) { return base } -function escapeArbitrary(input: string) { - return input +function printArbitraryValue(input: string) { + let ast = ValueParser.parse(input) + + let drop = new Set() + + ValueParser.walk(ast, (node, { parent }) => { + let parentArray = parent === null ? ast : (parent.nodes ?? []) + + // Handle operators (e.g.: inside of `calc(…)`) + if ( + node.kind === 'word' && + // Operators + (node.value === '+' || node.value === '-' || node.value === '*' || node.value === '/') + ) { + let idx = parentArray.indexOf(node) ?? -1 + + // This should not be possible + if (idx === -1) return + + let previous = parentArray[idx - 1] + if (previous?.kind !== 'separator' || previous.value !== ' ') return + + let next = parentArray[idx + 1] + if (next?.kind !== 'separator' || next.value !== ' ') return + + drop.add(previous) + drop.add(next) + } + + // The value parser handles `/` as a separator in some scenarios. E.g.: + // `theme(colors.red/50%)`. Because of this, we have to handle this case + // separately. + else if (node.kind === 'separator' && node.value.trim() === '/') { + node.value = '/' + } + + // Leading and trailing whitespace + else if (node.kind === 'separator' && node.value.length > 0 && node.value.trim() === '') { + if (parentArray[0] === node || parentArray[parentArray.length - 1] === node) { + drop.add(node) + } + } + }) + + if (drop.size > 0) { + ValueParser.walk(ast, (node, { replaceWith }) => { + if (drop.has(node)) { + drop.delete(node) + replaceWith([]) + } + }) + } + + return ValueParser.toCss(ast) .replaceAll('_', String.raw`\_`) // Escape underscores to keep them as-is .replaceAll(' ', '_') // Replace spaces with underscores } + +function simplifyArbitraryVariant(input: string) { + let ast = ValueParser.parse(input) + + // &:is(…) + if ( + ast.length === 3 && + // & + ast[0].kind === 'word' && + ast[0].value === '&' && + // : + ast[1].kind === 'separator' && + ast[1].value === ':' && + // is(…) + ast[2].kind === 'function' && + ast[2].value === 'is' + ) { + return ValueParser.toCss(ast[2].nodes) + } + + return input +} diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts index dfbce064c..4a53f8d4a 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/important.test.ts @@ -4,7 +4,7 @@ import { important } from './important' test.each([ ['!flex', 'flex!'], - ['min-[calc(1000px+12em)]:!flex', 'min-[calc(1000px_+_12em)]:flex!'], + ['min-[calc(1000px+12em)]:!flex', 'min-[calc(1000px+12em)]:flex!'], ['md:!block', 'md:block!'], // Does not change non-important candidates diff --git a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts index d8b0a238f..ef3e5234f 100644 --- a/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts +++ b/packages/@tailwindcss-upgrade/src/template/codemods/theme-to-var.test.ts @@ -26,7 +26,7 @@ test.each([ ], // Use `theme(…)` (deeply nested) inside of a `calc(…)` function - ['text-[calc(theme(fontSize.xs)*2)]', 'text-[calc(var(--font-size-xs)_*_2)]'], + ['text-[calc(theme(fontSize.xs)*2)]', 'text-[calc(var(--font-size-xs)*2)]'], // Multiple `theme(… / …)` calls should result in modern syntax of `theme(…)` // - Can't convert to `var(…)` because that would lose the modifier. diff --git a/packages/tailwindcss/src/value-parser.ts b/packages/tailwindcss/src/value-parser.ts index 8f41b5d3f..1b6677cfb 100644 --- a/packages/tailwindcss/src/value-parser.ts +++ b/packages/tailwindcss/src/value-parser.ts @@ -15,6 +15,7 @@ export type ValueSeparatorNode = { } export type ValueAstNode = ValueWordNode | ValueFunctionNode | ValueSeparatorNode +type ValueParentNode = ValueFunctionNode | null function word(value: string): ValueWordNode { return { @@ -54,11 +55,11 @@ export function walk( visit: ( node: ValueAstNode, utils: { - parent: ValueAstNode | null + parent: ValueParentNode replaceWith(newNode: ValueAstNode | ValueAstNode[]): void }, ) => void | ValueWalkAction, - parent: ValueAstNode | null = null, + parent: ValueParentNode = null, ) { for (let i = 0; i < ast.length; i++) { let node = ast[i] @@ -149,7 +150,7 @@ export function parse(input: string) { case GREATER_THAN: case EQUALS: { // 1. Handle everything before the separator as a word - // Handle everything before the closing paren a word + // Handle everything before the closing paren as a word if (buffer.length > 0) { let node = word(buffer) if (parent) { @@ -169,6 +170,7 @@ export function parse(input: string) { peekChar !== COLON && peekChar !== COMMA && peekChar !== SPACE && + peekChar !== SLASH && peekChar !== LESS_THAN && peekChar !== GREATER_THAN && peekChar !== EQUALS