fix(canonicalize): handle utilities with empty property maps in collapse#19727
fix(canonicalize): handle utilities with empty property maps in collapse#19727kirkouimet wants to merge 1 commit intotailwindlabs:mainfrom
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
WalkthroughThis change addresses a null reference issue in the collapse canonicalization logic and adds corresponding test coverage. The production code modification updates the 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ 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). 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. Comment |
Problem
canonicalizeCandidatescrashes when called withcollapse: trueand the candidate list includes utilities whose CSS output contains no standard declaration properties (only@propertyrules and CSS custom properties).This is reproducible with vanilla Tailwind CSS and no custom configuration:
All shadow utilities (
shadow-sm,shadow-md,shadow-lg,shadow-xl) crash when combined with any other utility andcollapse: true.This was discovered via
eslint-plugin-better-tailwindcss, which callscanonicalizeCandidateswithcollapse: truefor itsenforce-canonical-classesrule. The crash brings down ESLint entirely.Root cause
In
collapseGroup, theotherUtilitiesarray is built by mapping over each candidate's property values:When a utility like
shadow-smgenerates CSS with@propertyrules and custom property declarations but no standard CSS properties,propertyValues.keys()is empty, the loop never executes, andresultstaysnull. The non-null assertionresult!returnsnullinto the array.Downstream code then crashes when iterating or calling
.has()on the null entry:Fix
Return an empty
Setinstead ofnullwhen 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 in the linking algorithm. It won't cause false collapses or suppress valid collapses of other utilities.
Test plan
collapse does not crash when utilities with no standard properties are presentshadow-sm + border,shadow-md + p-4, andshadow-sm + shadow-mddon't throw