Improve whitespace handling during canonicalization (#19986)
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
This commit is contained in:
parent
107e451e37
commit
3aac5dafe8
3 changed files with 92 additions and 5 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string, string>((input) => {
|
|||
|
||||
let drop = new Set<ValueParser.ValueAstNode>()
|
||||
|
||||
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<string, string>((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) {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue