Skip to content

fix(canonicalize): collapse arbitrary values into shorthand utilities#19837

Merged
RobinMalfait merged 5 commits intotailwindlabs:mainfrom
aptinio:fix/canonicalize-collapse-arbitrary-values
Mar 23, 2026
Merged

fix(canonicalize): collapse arbitrary values into shorthand utilities#19837
RobinMalfait merged 5 commits intotailwindlabs:mainfrom
aptinio:fix/canonicalize-collapse-arbitrary-values

Conversation

@aptinio
Copy link
Contributor

@aptinio aptinio commented Mar 22, 2026

The guard on dynamicUtilities restricted root-swapping to named values, so arbitrary values like px-[1.2rem] py-[1.2rem] were never collapsed into p-[1.2rem].

This is what caused #19835 — in --stream mode, the collapse only happened if an earlier line caused the shorthand to be registered in STATIC_UTILITIES_KEY as a side effect, making the output non-deterministic. The underlying issue is that arbitrary value collapse wasn't supported at all.

The fix relaxes the guard from parsedCandidate.value?.kind !== 'named' to parsedCandidate.value === null. cloneCandidate and printCandidate already handle arbitrary values, so the root-swapping machinery works without other changes.

The iteration in dynamicUtilities is over designSystem.utilities.keys('functional') — a fixed set of roots, not input-proportional — so the performance cost of including arbitrary values should be negligible.

Fixes #19835.

@aptinio aptinio requested a review from a team as a code owner March 22, 2026 00:35
@aptinio aptinio force-pushed the fix/canonicalize-collapse-arbitrary-values branch from 36e6216 to ef598fd Compare March 22, 2026 00:37
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9068925c-72fd-4972-a6c0-570aa7dcd4ea

📥 Commits

Reviewing files that changed from the base of the PR and between e3bba73 and fe3e48a.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • packages/tailwindcss/src/canonicalize-candidates.test.ts
✅ Files skipped from review due to trivial changes (2)
  • packages/tailwindcss/src/canonicalize-candidates.test.ts
  • CHANGELOG.md

Walkthrough

The PR relaxes a filter in canonicalizeCandidates so functional parsed candidates are included when parsedCandidate.value is not null (removing the previous value.kind === 'named' requirement). It adds a Vitest test.each that loads a design system via __unstable__loadDesignSystem, runs canonicalizeCandidates with { collapse: true, logicalToPhysical: true, rem: 16 }, and asserts that matching arbitrary px-[…] and py-[…] candidates collapse into p-[…] (including collapsing to canonical spacing tokens). CHANGELOG.md was updated with an Unreleased Fixed entry.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: enabling collapse of arbitrary values into shorthand utilities, which is the core fix for the pull request.
Description check ✅ Passed The description is directly related to the changeset, explaining the root cause (guard preventing arbitrary value collapse), the fix (relaxing the guard condition), and the referenced issue.
Linked Issues check ✅ Passed The PR directly addresses issue #19835 by enabling arbitrary value collapse in canonicalization. The fix relaxes the guard to allow root-swapping for arbitrary values, ensuring deterministic output regardless of stream state.
Out of Scope Changes check ✅ Passed All changes are in scope: the core fix in canonicalize-candidates.ts, a test case for issue #19835, and a changelog entry documenting the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/tailwindcss/src/canonicalize-candidates.test.ts (1)

1314-1328: Add a stream-state regression assertion for this arbitrary-value case.

This test validates collapse, but it doesn’t yet verify the reported stream-order behavior (same input before/after prior calls). Adding that check here would lock in the #19835 fix more directly.

Suggested test strengthening
 test('collapse canonicalization works for arbitrary values', { timeout }, async () => {
   let designSystem = await designSystems.get(__dirname).get(css`
     `@import` 'tailwindcss';
   `)

   let options: CanonicalizeOptions = {
     collapse: true,
     logicalToPhysical: true,
     rem: 16,
   }

-  expect(
-    designSystem.canonicalizeCandidates(['px-[1.2rem]', 'py-[1.2rem]', 'text-left'], options),
-  ).toEqual(['text-left', 'p-[1.2rem]'])
+  let target = ['px-[1.2rem]', 'py-[1.2rem]', 'text-left']
+
+  // First encounter should already collapse
+  expect(designSystem.canonicalizeCandidates(target, options)).toEqual(['text-left', 'p-[1.2rem]'])
+
+  // Prior calls in the same process should not affect the result
+  designSystem.canonicalizeCandidates(['p-[1.2rem]'], options)
+  designSystem.canonicalizeCandidates(['mb-4', 'text-sm'], options)
+
+  expect(designSystem.canonicalizeCandidates(target, options)).toEqual(['text-left', 'p-[1.2rem]'])
 })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/tailwindcss/src/canonicalize-candidates.test.ts` around lines 1314 -
1328, Add a regression assertion that the stream/state is stable by calling
designSystem.canonicalizeCandidates a second time with the same inputs and
options and asserting the result is identical; specifically, after the existing
expect(...) for designSystem.canonicalizeCandidates(['px-[1.2rem]',
'py-[1.2rem]', 'text-left'], options), call
designSystem.canonicalizeCandidates(...) again and assert it returns the same
array (['text-left','p-[1.2rem]']), using the same designSystem (from
designSystems.get) and options to lock in the stream-order fix for
canonicalizeCandidates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/tailwindcss/src/canonicalize-candidates.test.ts`:
- Around line 1314-1328: Add a regression assertion that the stream/state is
stable by calling designSystem.canonicalizeCandidates a second time with the
same inputs and options and asserting the result is identical; specifically,
after the existing expect(...) for
designSystem.canonicalizeCandidates(['px-[1.2rem]', 'py-[1.2rem]', 'text-left'],
options), call designSystem.canonicalizeCandidates(...) again and assert it
returns the same array (['text-left','p-[1.2rem]']), using the same designSystem
(from designSystems.get) and options to lock in the stream-order fix for
canonicalizeCandidates.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ccb1b508-393e-4c79-86d1-294cb62daffb

📥 Commits

Reviewing files that changed from the base of the PR and between 36e6216 and ef598fd.

📒 Files selected for processing (2)
  • packages/tailwindcss/src/canonicalize-candidates.test.ts
  • packages/tailwindcss/src/canonicalize-candidates.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/tailwindcss/src/canonicalize-candidates.ts

`dynamicUtilities` discovers collapse targets by swapping utility
roots (e.g., `px` → `p`), but the guard at line 326 restricted this
to named values. This was likely intentional for the initial
implementation, but `cloneCandidate` and `printCandidate` both handle
arbitrary values correctly, so the restriction can be safely relaxed.

Without this, arbitrary values like `px-[1.2rem] py-[1.2rem]` were
never collapsed into `p-[1.2rem]` — which is what caused the
non-deterministic `--stream` output reported in tailwindlabs#19835.

Fixes tailwindlabs#19835
@RobinMalfait RobinMalfait enabled auto-merge (squash) March 23, 2026 10:32
Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful, thanks!

@RobinMalfait RobinMalfait merged commit b55d960 into tailwindlabs:main Mar 23, 2026
9 checks passed
@aptinio aptinio deleted the fix/canonicalize-collapse-arbitrary-values branch March 23, 2026 10:34
@aptinio
Copy link
Contributor Author

aptinio commented Mar 23, 2026

Thanks for improving and merging! 🙏

RobinMalfait added a commit that referenced this pull request Mar 23, 2026
This PR adds more declaration expansions such that we can collapse more
utilities.

While testing #19837 I noticed that in my tests some utilities weren't
canonicalized correctly. As part of that PR, we check for
`parsedCandidate.value === null`, which means that a functional utility
without a value is skipped. We do have utilities like that such as
`border` (which is equivalent to `border-1`). But while testing, I
noticed that `border-x border-y` should collapse to `border` but they
didn't. This PR fixes that.

By expanding these properties to their long-form physical properties
(instead of the shorter logical properties) we make the signatures of
utilities a bit bigger, but also more correct such that we can collapse
the physical form into logical utilities.

To make this more concrete, this PR allows for the following
canonicalizations now:

| Input | Output |
| --- | --- |
| `border-t-123 border-r-123 border-b-123 border-l-123` | `border-123` |
| `border-t-1 border-r-1 border-b-1 border-l-1` | `border` |
| `border-t-123 border-b-123` | `border-y-123` |
| `border-l-123 border-r-123` | `border-x-123` |
| `border-t-red-500 border-r-red-500 border-b-red-500 border-l-red-500`
| `border-red-500` |
| `border-t-red-500 border-b-red-500` | `border-y-red-500` |
| `border-l-red-500 border-r-red-500` | `border-x-red-500` |
| `scroll-mt-123 scroll-mr-123 scroll-mb-123 scroll-ml-123` |
`scroll-m-123` |
| `scroll-mt-123 scroll-mb-123` | `scroll-my-123` |
| `scroll-ml-123 scroll-mr-123` | `scroll-mx-123` |
| `scroll-pt-123 scroll-pr-123 scroll-pb-123 scroll-pl-123` |
`scroll-p-123` |
| `scroll-pt-123 scroll-pb-123` | `scroll-py-123` |
| `scroll-pl-123 scroll-pr-123` | `scroll-px-123` |
| `overflow-x-hidden overflow-y-hidden` | `overflow-hidden` |
| `overscroll-x-contain overscroll-y-contain` | `overscroll-contain` |

## Test plan

1. Existing tests pass
2. Added a few more tests to verify that these canonicalizations work
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

canonicalize --stream is stateful: output for a class string changes based on previously processed inputs

2 participants