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 <div class="[p]:flex"></div> <div class="[&:is(p)]:flex"></div> <div class="has-[p]:flex"></div> <div class="px-[theme(spacing.4)-1px]"></div> ``` Output before: ```html <div class="[&:is(p)]:flex"></div> <div class="[&:is(p)]:flex"></div> <div class="has-[&:is(p)]:flex"></div> <div class="px-[var(--spacing-4)_-_1px]"></div> ``` Output after: ```html <div class="[p]:flex"></div> <div class="[p]:flex"></div> <div class="has-[p]:flex"></div> <div class="px-[var(--spacing-4)-1px]"></div> ``` --- This is alternative implementation to #14717 and #14718 Closes: #14717 Closes: #14718
tailwindlabs · Oct 18, 2024 · 2abf228 · 2abf228
1 parent c4b97f6
commit 2abf228
Show file tree

Hide file tree

Showing 6 changed files with 140 additions and 37 deletions.
diff --git 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
@@ -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:',
-
-  'hover:focus:',
-  'hover:group-focus:',
-  'group-hover:focus:',
-  'group-hover:group-focus:',
-
-  'min-[10px]:',
-  // TODO: This currently expands `calc(1000px+12em)` to `calc(1000px_+_12em)`
-  'min-[calc(1000px_+_12em)]:',
-
-  'peer-[&_p]:',
-  'peer-[&_p]:hover:',
-  'hover:peer-[&_p]:',
-  'hover:peer-[&_p]:focus:',
-  'peer-[&:hover]:peer-[&_p]:',
+  ['', ''], // no variant
+  ['*:', '*:'],
+  ['focus:', 'focus:'],
+  ['group-focus:', '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]:', 'min-[10px]:'],
+
+  // 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
@@ -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.ValueAstNode>()
+
+  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
@@ -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
@@ -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
@@ -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