Canonicalization: limit arbitrary to bare values conversion (#20130)
This PR improves the canonicalization process by limiting the bare values to a certain amount. Before this PR, whenever we have an arbitrary value, e.g. `left-[6px]`, then we prefer to use a bare value instead e.g. `left-1.5`. In most cases, this makes sense. However, there are places where this doesn't really make sense (https://x.com/kettanaito/status/2059987396050268589) - `left-[99999px]` → `left-24999.75 ` The hard part is to figure out _why_ this feels wrong. The `.75` could feel wrong, but in the `left-[6px]` → `left-1.5`, the `.5` makes sense. If we reduce that big number to `left-[99996px]` → `left-24999`, then there is no floating point but it still feels wrong. One possibility I can think of is to analyze the incoming value and see if we find certain patterns. All repeating numbers, fun numbers like `1337`, common numbers most programmers know such as `720px`, `1280px`, etc. But instead of that, I think it's more reasonable to limit the bare value such that the `px` based value doesn't exceed a big number. We can improve the logic if there are other cases that don't really make sense. The biggest value we have in our default theme is `--breakpoint-2xl: 96rem`, which is equivalent to `1536px`. So I think any bare value that results in a value `<= 1536px` should probably be fine. In this case, `left-[99999px]` would stay as `left-[99999px]`, but `left-[6px]` is still converted to `left-1.5`. Note: this is only happening for arbitrary values being converted to bare values _if_ they use the `--spacing` variable internally. Values such as `z-[99999999999]` will still be converted to `z-99999999999`, since the intent is still clear. ## Test plan 1. Added new tests for these limitations 2. Other existing tests still pass
This commit is contained in:
parent
d4f24c5f16
commit
6b43b6400a
3 changed files with 48 additions and 2 deletions
|
|
@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
- Fix 'Sourcemap is likely to be incorrect' warnings when using `@tailwindcss/vite` ([#20103](https://github.com/tailwindlabs/tailwindcss/pull/20103))
|
||||
- Ensure `@tailwindcss/webpack` can be installed in Rspack projects without requiring `webpack` as a peer dependency ([#20027](https://github.com/tailwindlabs/tailwindcss/pull/20027))
|
||||
- Canonicalization: don't suggest invalid `calc(…)` expressions (e.g. `px-[calc(1rem+0px)]` → `px-[calc(1rem+0)]`) ([#20127](https://github.com/tailwindlabs/tailwindcss/pull/20127))
|
||||
- Canonicalization: avoid suggesting large spacing-scale values for arbitrary lengths (e.g. `left-[99999px]` → `left-[99999px]`, not `left-24999.75`) ([#20130](https://github.com/tailwindlabs/tailwindcss/pull/20130))
|
||||
|
||||
## [4.3.0] - 2026-05-08
|
||||
|
||||
|
|
|
|||
|
|
@ -871,6 +871,15 @@ describe.each([['default'], ['with-variant'], ['important'], ['prefix']])('%s',
|
|||
['[margin:-0]', 'm-0'],
|
||||
['[margin:0px]', 'm-0'],
|
||||
|
||||
// Limit conversions for big values using the `--spacing` multiplier
|
||||
['left-[99999px]', 'left-[99999px]'], // This would otherwise result in `left-24999.75`
|
||||
['left-[-99999px]', 'left-[-99999px]'], // This would otherwise result in `-left-24999.75`
|
||||
['left-[96rem]', 'left-384'], // Within the limit
|
||||
['left-[-96rem]', '-left-384'], // Within the limit
|
||||
['left-[calc(96rem+1px)]', 'left-[calc(96rem+1px)]'], // Out of the positive limit
|
||||
['left-[calc(-96rem-1px)]', 'left-[calc(-96rem-1px)]'], // Out of the negative limit
|
||||
['z-[9999999]', 'z-9999999'], // `--spacing` multiplier is not used
|
||||
|
||||
// Not a length-unit, can't safely constant fold
|
||||
['[margin:0%]', 'm-[0%]'],
|
||||
|
||||
|
|
|
|||
|
|
@ -1074,6 +1074,35 @@ function printUnprefixedCandidate(designSystem: DesignSystem, candidate: Candida
|
|||
// ----
|
||||
|
||||
const SPACING_KEY = Symbol()
|
||||
|
||||
// We prefer bare values over arbitrary values such as `left-[6px]` →
|
||||
// `left-1.5`.
|
||||
//
|
||||
// There are situations where this doesn't always make sense. E.g.:
|
||||
// `left-[99999px]` → `left-24999.75`.
|
||||
//
|
||||
// We could try and analyze the incoming value and bail out if we detect certain
|
||||
// patterns. But a first step would be to limit the bare values for large values
|
||||
// and keep them low. The largest rem value we have is for the 2xl breakpoint,
|
||||
// which is 96rem, which is 1536px.
|
||||
const MAX_BARE_VALUE_IN_PX = 1536
|
||||
const MAX_BARE_VALUE_IN_REM = MAX_BARE_VALUE_IN_PX / 16
|
||||
|
||||
function isReasonableBareValue(value: number, designSystem: DesignSystem, rem: number | null) {
|
||||
let spacingMultiplier = designSystem.resolveThemeValue('--spacing')
|
||||
if (spacingMultiplier === undefined) return false
|
||||
|
||||
let parsed = dimensions.get(constantFoldDeclaration(spacingMultiplier, rem))
|
||||
if (parsed === null) return false
|
||||
|
||||
let [spacingValue, spacingUnit] = parsed
|
||||
let bareValueInPixels = value * spacingValue
|
||||
|
||||
if (spacingUnit === 'px') return bareValueInPixels <= MAX_BARE_VALUE_IN_PX
|
||||
if (spacingUnit === 'rem') return bareValueInPixels <= MAX_BARE_VALUE_IN_REM
|
||||
return false
|
||||
}
|
||||
|
||||
function createSpacingCache(
|
||||
designSystem: DesignSystem,
|
||||
options?: CanonicalizeOptions,
|
||||
|
|
@ -1220,7 +1249,10 @@ function arbitraryUtilities(candidate: Candidate, options: InternalCanonicalizeO
|
|||
) {
|
||||
let bareValue = designSystem.storage[SPACING_KEY]?.get(value) ?? null
|
||||
if (bareValue !== null) {
|
||||
if (isValidSpacingMultiplier(bareValue)) {
|
||||
if (
|
||||
isValidSpacingMultiplier(bareValue) &&
|
||||
isReasonableBareValue(bareValue, designSystem, options.signatureOptions.rem)
|
||||
) {
|
||||
yield Object.assign({}, candidate, {
|
||||
value: { kind: 'named', value: bareValue, fraction: null },
|
||||
})
|
||||
|
|
@ -1261,7 +1293,11 @@ function arbitraryUtilities(candidate: Candidate, options: InternalCanonicalizeO
|
|||
// Try bare value based on the `--spacing` value. E.g.:
|
||||
//
|
||||
// - `w-[64rem]` → `w-256`
|
||||
if (spacingMultiplier !== null) {
|
||||
if (
|
||||
spacingMultiplier !== null &&
|
||||
isValidSpacingMultiplier(spacingMultiplier) &&
|
||||
isReasonableBareValue(spacingMultiplier, designSystem, options.signatureOptions.rem)
|
||||
) {
|
||||
for (let replacementCandidate of parseCandidate(
|
||||
designSystem,
|
||||
`${root}-${spacingMultiplier}`,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue