-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Try to canonicalize any arbitrary utility to a bare value #19379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try to canonicalize any arbitrary utility to a bare value #19379
Conversation
WalkthroughAdds a new canonicalization path for arbitrary utility values in Tailwind CSS: when rem-based canonicalization is enabled and the candidate is a functional arbitrary value, the code resolves the spacing multiplier from the design system, constant-folds it, canonicalizes the candidate value, compares dimensions, and — if units match and the multiplier is non-zero — computes a bare numeric value and emits a named candidate. This path runs before the existing spacing-map lookup. Test coverage was expanded with cases for default and custom spacing scales and mixed units, and some expected outcomes were updated. Pre-merge checks✅ Passed checks (2 passed)
Comment |
To do this, we check if the `rem` option is provided, then we canonicalize the incoming value. We also canonicalize the spacing multiplier so we can deal with both `--spacing: 0.25rem` as `--spacing: 4px` values (different units). We then compute the bare value, and just try it as a replacement.
25b0b5f to
4bd24e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CHANGELOG.md (1)
20-20: Polish wording + add example; consider section placementPrefer Tailwind terminology (“arbitrary values in utilities”) and include an example for clarity. Optionally, this reads more like a behavior change than a bug fix, so consider moving to “Changed” if that better matches intent.
- - Try to canonicalize any arbitrary utility to a bare value ([#19379](https://github.com/tailwindlabs/tailwindcss/pull/19379)) + - Canonicalize arbitrary values in utilities to bare values (e.g., `gap-[116px]` → `gap-29`) ([#19379](https://github.com/tailwindlabs/tailwindcss/pull/19379))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)packages/tailwindcss/src/canonicalize-candidates.test.ts(1 hunks)packages/tailwindcss/src/canonicalize-candidates.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/canonicalize-candidates.ts (2)
packages/tailwindcss/src/constant-fold-declaration.ts (1)
constantFoldDeclaration(8-115)packages/tailwindcss/src/utils/dimensions.ts (1)
dimensions(6-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Linux
- GitHub Check: Linux / upgrade
🔇 Additional comments (2)
packages/tailwindcss/src/canonicalize-candidates.test.ts (2)
533-547: Excellent test coverage for the new canonicalization logic.The new test cases comprehensively verify that arbitrary values can be canonicalized to bare values across different spacing scales and units:
- Default scale (0.25rem/4px):
gap-[116px]→gap-29✓- Custom scale (1px):
gap-[116px]→gap-116✓- Equivalent forms: rem, calc, and px all canonicalize to the same result ✓
The math checks out: 116px ÷ 4px = 29, and the tests verify this works whether the spacing is expressed as
0.25remor4px.
550-550: Correctly updated expectation.The updated expectation
w-[124px]→w-31correctly reflects the new canonicalization behavior (124px ÷ 4px = 31).
Probably will never happen in the real world because a spacing multiplier of 0px (or equivalent) doesn't make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/tailwindcss/src/canonicalize-candidates.ts (1)
993-1025: Spacing canonicalization block looks correct; consider simplifying constantFold usage.The new rem‑aware spacing canonicalization logic is well constrained (only for functional + arbitrary values, unit‑matched, and guarded by signature comparison), and the division‑by‑zero check avoids pathological configs cleanly.
One small nit:
constantFoldDeclarationcurrently returns astring, so thecanonicalizedSpacingMultiplier !== nullcheck is a no‑op. You could drop that check and slightly flatten the nesting by computing both folded values up front and early‑returning whendimensions.get(...)fails, e.g.:- let canonicalizedSpacingMultiplier = constantFoldDeclaration( - spacingMultiplier, - options.signatureOptions.rem, - ) - if (canonicalizedSpacingMultiplier !== null) { - let canonicalizedValue = constantFoldDeclaration(value, options.signatureOptions.rem) - let valueDimension = dimensions.get(canonicalizedValue) - let spacingMultiplierDimension = dimensions.get(canonicalizedSpacingMultiplier) - if ( - valueDimension && - spacingMultiplierDimension && - valueDimension[1] === spacingMultiplierDimension[1] && // Ensure the units match - spacingMultiplierDimension[0] !== 0 - ) { - let bareValue = `${valueDimension[0] / spacingMultiplierDimension[0]}` - if (isValidSpacingMultiplier(bareValue)) { - yield Object.assign({}, candidate, { - value: { kind: 'named', value: bareValue, fraction: null }, - }) - } - } - } + let canonicalizedSpacingMultiplier = constantFoldDeclaration( + spacingMultiplier, + options.signatureOptions.rem, + ) + let canonicalizedValue = constantFoldDeclaration(value, options.signatureOptions.rem) + + let valueDimension = dimensions.get(canonicalizedValue) + let spacingMultiplierDimension = dimensions.get(canonicalizedSpacingMultiplier) + if ( + valueDimension && + spacingMultiplierDimension && + valueDimension[1] === spacingMultiplierDimension[1] && // Ensure the units match + spacingMultiplierDimension[0] !== 0 + ) { + let bareValue = `${valueDimension[0] / spacingMultiplierDimension[0]}` + if (isValidSpacingMultiplier(bareValue)) { + yield Object.assign({}, candidate, { + value: { kind: 'named', value: bareValue, fraction: null }, + }) + } + }Purely a readability/maintenance tweak; behavior stays the same.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/tailwindcss/src/canonicalize-candidates.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/tailwindcss/src/canonicalize-candidates.ts (3)
packages/tailwindcss/src/constant-fold-declaration.ts (1)
constantFoldDeclaration(8-115)packages/tailwindcss/src/utils/dimensions.ts (1)
dimensions(6-20)packages/tailwindcss/src/utils/infer-data-type.ts (1)
isValidSpacingMultiplier(356-358)
This PR adds an improvement to our canonicalization logic when dealing with arbitrary values. When trying to canonicalize utilities, we make use of the intellisense suggestions list where we typically use multiples of the spacing scale.
This means that a value like
gap-[128px]gets properly canonicalized togap-32. However, when you try a value that we typically don't suggest such asgap-[116px]then it doesn't get canonicalized at all.This PR fixes that by trying to use the spacing scale and convert
116px / 4pxand try thegap-29utility instead.This is done by canonicalizing the incoming arbitrary value and the spacing multipliers such that
--spacing: 0.25remand--spacing: 4pxboth work as expected.Test plan
0.25rem(which is the default)4px1pxAlso had to update 1 test that now gets canonicalized properly, e.g.:
w-[124px]→w-31.