From 3aac5dafe8cf26a96f29f95edf82f309ca4a1436 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 27 Apr 2026 00:20:53 +0200 Subject: [PATCH] Improve whitespace handling during canonicalization (#19986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR fixes a printing bug during canonicalization where it converts: ``` [&:has(~_*_*:checked)]:text-green-500 ``` into: ``` [&:has(~**:checked)]:text-green-500 ``` This is because the `_` was marked as insignificant and therefore removed. This PR fixes that and maintains the whitespace (`_`) characters when needed. Additionally, in the comments of the linked issue somebody mentioned that: ``` w-[calc(100%_-_--spacing(60))] ``` was turned into: ``` w-[calc(100%---spacing(60))] ``` ...and while that's still correct and parseable, it's not the prettiest. This PR will still get rid of the whitespace, but introduce wrapping parens `(…)` instead, in case readability is not ideal. In this case, we will turn it into: ```diff - w-[calc(100%_-_--spacing(60))] - w-[calc(100%---spacing(60))] + w-[calc(100%-(--spacing(60)))] ``` Of course there are some cases where we don't need to introduce `(…)` unnecessarily: - `shadow-[inset_0px_1px_--theme(--color-white/15%)]` would not be turned into `shadow-[inset_0px_1px_(--theme(--color-white/15%))]` because no readability is gained when it's part of a normal space separated list - `m-[--spacing(12.34)]` would not be turned into `m-[(--spacing(12.34))]` because there is nothing else it can conflict with - `m-[calc(--spacing(12.34)*2)]` would not be turned into `m-[calc((--spacing(12.34))*2)]` because it's the first argument and doesn't conflict with the `*` - `m-[min(100%,--spacing(12.34))]` would not be turned into `m-[min(100%,(--spacing(12.34)))]` because a `,` doesn't cause readability issues Fixes: https://github.com/tailwindlabs/tailwindcss-intellisense/issues/1544 ## Test plan 1. Added new tests to ensure the bug is fixed 2. Added new tests to ensure readability is improved (and not degraged) when whitespace was used to improve readability --- CHANGELOG.md | 2 + packages/tailwindcss/src/candidate.ts | 71 +++++++++++++++++-- .../src/canonicalize-candidates.test.ts | 24 +++++++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2768ffbc4..710bbd8d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix relative `@import` and `@plugin` paths resolving from the wrong directory when using `@tailwindcss/vite` ([#19965](https://github.com/tailwindlabs/tailwindcss/pull/19965)) - Ensure CSS files containing `@variant` are processed by `@tailwindcss/vite` ([#19966](https://github.com/tailwindlabs/tailwindcss/pull/19966)) - Resolve imports relative to `base` when `result.opts.from` is not provided when using `@tailwindcss/postcss` ([#19980](https://github.com/tailwindlabs/tailwindcss/pull/19980)) +- Canonicalization: preserve significant `_` whitespace in arbitrary values ([#19986](https://github.com/tailwindlabs/tailwindcss/pull/19986)) +- Canonicalization: add parentheses when removing whitespace from arbitrary values would hurt readability ([#19986](https://github.com/tailwindlabs/tailwindcss/pull/19986)) ## [4.2.4] - 2026-04-21 diff --git a/packages/tailwindcss/src/candidate.ts b/packages/tailwindcss/src/candidate.ts index 4e79b818e..f3ff00f52 100644 --- a/packages/tailwindcss/src/candidate.ts +++ b/packages/tailwindcss/src/candidate.ts @@ -3,6 +3,7 @@ import { decodeArbitraryValue } from './utils/decode-arbitrary-value' import { DefaultMap } from './utils/default-map' import { isValidArbitrary } from './utils/is-valid-arbitrary' import { segment } from './utils/segment' +import type { ValueFunctionNode } from './value-parser' import * as ValueParser from './value-parser' import { walk, WalkAction } from './walk' @@ -1035,26 +1036,51 @@ const printArbitraryValueCache = new DefaultMap((input) => { let drop = new Set() + let symbols = new Set([ + // Selectors + '~', // Subsequent sibling combinator + '>', // Child combinator + + // Math operators + '+', // or next sibling combinator + '-', + '*', // or universal selector + '/', + ]) walk(ast, (node, ctx) => { let parentArray = ctx.parent === null ? ast : (ctx.parent.nodes ?? []) // Handle operators (e.g.: inside of `calc(…)`) - if ( - node.kind === 'word' && - // Operators - (node.value === '+' || node.value === '-' || node.value === '*' || node.value === '/') - ) { + if (node.kind === 'word' && symbols.has(node.value)) { let idx = parentArray.indexOf(node) ?? -1 // This should not be possible if (idx === -1) return + // a + b + // ^ node + // ^ previous (whitespace) let previous = parentArray[idx - 1] if (previous?.kind !== 'separator' || previous.value !== ' ') return + // a + b + // ^ node + // ^ next (whitespace) let next = parentArray[idx + 1] if (next?.kind !== 'separator' || next.value !== ' ') return + // a + b + // ^ node + // ^ previous (node) + let previousPrevious = parentArray[idx - 2] + if (previousPrevious && symbols.has(previousPrevious.value)) return + + // a + b + // ^ node + // ^ next (node) + let nextNext = parentArray[idx + 2] + if (nextNext && symbols.has(nextNext.value)) return + drop.add(previous) drop.add(next) } @@ -1071,6 +1097,41 @@ const printArbitraryValueCache = new DefaultMap((input) => { else if (node.kind === 'separator' && node.value.trim() === ',') { node.value = ',' } + + // Wrap custom functions starting with `--`, in parentheses if preceeded by + // a symbol. E.g.: `calc(100%---spacing(2))` → `calc(100%-(--spacing(2)))` + else if (node.kind === 'function' && node.value.startsWith('--')) { + let idx = parentArray.indexOf(node) ?? -1 + + // When it's the first argument, then we don't have to wrap it in `(…)` + // + // E.g.: `--spacing(…)`, no need to wrap + // `calc(100%---spacing(…))`, we want to wrap + // + if (idx <= 0) return + + // When it's not the first argument, we likely want to wrap it. However, + // when it's separated by a `,` then it's readable enough. + // + // E.g.: `min(100%,--spacing(2))` is readable, in fact + // `min(100%,(--spacing(2)))` would make it worse + let previous = parentArray[idx - 1] + if (previous?.kind === 'separator' && previous.value === ',') return + + // When it's part of a bigger list, aka no special symbols were used, then + // we don't have to wrap it either. + // + // E.g.: `shadow-[inset_0px_1px_--theme(--color-white/15%)]`, wrapping would look unnecessary: + // `shadow-[inset_0px_1px_(--theme(--color-white/15%))]` + let previousPrevious = parentArray[idx - 2] + if (previousPrevious && !symbols.has(previousPrevious.value)) return + + return WalkAction.ReplaceSkip({ + kind: 'function', + value: '', // Unnamed, so will result in `(…)` + nodes: [node], + } satisfies ValueFunctionNode) + } }) if (drop.size > 0) { diff --git a/packages/tailwindcss/src/canonicalize-candidates.test.ts b/packages/tailwindcss/src/canonicalize-candidates.test.ts index df029834f..6c1821baa 100644 --- a/packages/tailwindcss/src/canonicalize-candidates.test.ts +++ b/packages/tailwindcss/src/canonicalize-candidates.test.ts @@ -1186,6 +1186,30 @@ describe.each([['default'], ['with-variant'], ['important'], ['prefix']])('%s', expected, ) }) + + test.each([ + // Keep whitespace characters that are significant + ['[&:has(~_*_*:checked)]:flex', '[&:has(~_*_*:checked)]:flex'], + [ + 'shadow-[inset_0px_1px_--theme(--color-white/15%)]', + 'shadow-[inset_0px_1px_--theme(--color-white/15%)]', + ], + + // Improve readability when whitespace was used for readability + ['w-[calc(100%_-_calc(var(--spacing)*60))]', 'w-[calc(100%-(--spacing(60)))]'], + ['w-[calc(100%_-_--spacing(60))]', 'w-[calc(100%-(--spacing(60)))]'], + + // No need to to wrap in `(…)` after a `,` + ['m-[min(100%,_--spacing(6))]', 'm-[min(100%,--spacing(6))]'], + ['m-[min(100%_,_--spacing(6))]', 'm-[min(100%,--spacing(6))]'], + ['m-[min(100%,--spacing(6))]', 'm-[min(100%,--spacing(6))]'], + ])(testName, async (candidate, expected) => { + let input = css` + @import 'tailwindcss'; + ` + + await expectCanonicalization(input, candidate, expected) + }) }) describe('theme to var', () => {