Skip to content

fix(canonicalize): handle utilities with empty property maps in collapse#19727

Open
kirkouimet wants to merge 1 commit intotailwindlabs:mainfrom
kirkouimet:fix/canonicalize-null-safety
Open

fix(canonicalize): handle utilities with empty property maps in collapse#19727
kirkouimet wants to merge 1 commit intotailwindlabs:mainfrom
kirkouimet:fix/canonicalize-null-safety

Conversation

@kirkouimet
Copy link
Contributor

Problem

canonicalizeCandidates crashes when called with collapse: true and the candidate list includes utilities whose CSS output contains no standard declaration properties (only @property rules and CSS custom properties).

This is reproducible with vanilla Tailwind CSS and no custom configuration:

designSystem.canonicalizeCandidates(['shadow-sm', 'border'], { collapse: true })
// TypeError: X is not iterable
designSystem.canonicalizeCandidates(['shadow-sm', 'border'], { collapse: true })
// TypeError: Cannot read properties of null (reading 'has')

All shadow utilities (shadow-sm, shadow-md, shadow-lg, shadow-xl) crash when combined with any other utility and collapse: true.

This was discovered via eslint-plugin-better-tailwindcss, which calls canonicalizeCandidates with collapse: true for its enforce-canonical-classes rule. The crash brings down ESLint entirely.

Root cause

In collapseGroup, the otherUtilities array is built by mapping over each candidate's property values:

let otherUtilities = candidatePropertiesValues.map((propertyValues) => {
  let result: Set<string> | null = null
  for (let property of propertyValues.keys()) {
    // ... builds result ...
  }
  return result! // returns null if propertyValues has no keys
})

When a utility like shadow-sm generates CSS with @property rules and custom property declarations but no standard CSS properties, propertyValues.keys() is empty, the loop never executes, and result stays null. The non-null assertion result! returns null into the array.

Downstream code then crashes when iterating or calling .has() on the null entry:

for (let i = 0; i < otherUtilities.length; i++) {
  let current = otherUtilities[i]     // null
  for (let property of current) {     // "X is not iterable"
    if (other.has(property)) {        // "Cannot read properties of null"

Fix

Return an empty Set instead of null when a utility has no property keys:

return result ?? new Set<string>()

This is semantically correct: a utility with no standard properties cannot be linked to or collapsed with any other utility, which is exactly what an empty Set represents in the linking algorithm. It won't cause false collapses or suppress valid collapses of other utilities.

Test plan

  • Added test: collapse does not crash when utilities with no standard properties are present
  • Verifies shadow-sm + border, shadow-md + p-4, and shadow-sm + shadow-md don't throw
  • Verifies the candidates are returned uncollapsed (correct behavior)
  • All 1218 existing tests continue to pass

When `canonicalizeCandidates` is called with `collapse: true`, the
`collapseGroup` function builds an `otherUtilities` array by mapping
each candidate's property values. If a utility generates CSS but has
no standard declaration properties (e.g. shadow utilities that use
`@property` rules and CSS custom properties), the inner loop over
`propertyValues.keys()` never executes, leaving `result` as `null`.

The non-null assertion `result!` then returns `null` into the array,
causing downstream code to crash with "X is not iterable" or "Cannot
read properties of null (reading 'has')" when iterating or calling
methods on the null entry.

Fix: return an empty Set instead of null when a utility has no
property keys. This is semantically correct -- a utility with no
standard properties cannot be linked to or collapsed with any other
utility, which is exactly what an empty Set represents.

Reproduction: `canonicalizeCandidates(['shadow-sm', 'border'], { collapse: true })`
crashes on vanilla Tailwind CSS with no custom configuration.
@kirkouimet kirkouimet requested a review from a team as a code owner February 26, 2026 00:23
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ded4a2 and d6698ec.

📒 Files selected for processing (2)
  • packages/tailwindcss/src/canonicalize-candidates.test.ts
  • packages/tailwindcss/src/canonicalize-candidates.ts

Walkthrough

This change addresses a null reference issue in the collapse canonicalization logic and adds corresponding test coverage. The production code modification updates the collapseGroup function to return an empty Set instead of a potentially null value, ensuring the map computation for property groups always produces non-null results. A new test case is introduced to verify that the collapse path handles edge cases correctly, specifically utilities without standard properties like shadow-related utilities, without throwing exceptions and while preserving expected output.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: handling utilities with empty property maps in the collapse operation, which directly addresses the core issue in the changeset.
Description check ✅ Passed The description comprehensively explains the problem, root cause, fix, and test plan with code examples, directly relating to the changeset modifications.

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

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

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.

1 participant