fix(canonicalize): prevent collapse cache pollution across calls

[camc314]

Summary

fixes schoero/eslint-plugin-better-tailwindcss#321

This PR fixes an order-sensitive canonicalization bug. This bug caused issues when running eslint-plugin-better-tailwindcss as the order in which files are linted in is not consistent. This caused, in some scenarios, canonicalizeCandidates(..., { collapse: true, logicalToPhysical: true, rem: 16 }) to stop collapsing valid combinations (for example h-4 + w-4 -> size-4) after unrelated prior calls.

To reproduce this issue:

# checkout this branch
$ git checkout c/fix-canonicalizeCandidates
# Revert the fix to the current `main` branch
$ git checkout main ./packages/tailwindcss/src/canonicalize-candidates.ts
# Run the tests
$ pnpm run test

This should produce a failure like so:

 FAIL   tailwindcss  src/canonicalize-candidates.test.ts > regressions > collapse canonicalization is not affected by previous calls
AssertionError: expected [ 'underline', 'h-4', 'w-4' ] to deeply equal [ 'underline', 'size-4' ]

- Expected
+ Received

  [
    "underline",
-   "size-4",
+   "h-4",
+   "w-4",
  ]

 ❯ src/canonicalize-candidates.test.ts:1167:66
    1165|     designSystem.canonicalizeCandidates(['underline', 'mb-4'], options)
    1166| 
    1167|     expect(designSystem.canonicalizeCandidates(target, options)).toEqual(['underline', 'size-4'])
       |                                                                  ^
    1168|   })
    1169| })

# reset all changes on this branch
git reset --hard
# run the tests again (they should now pass)
pnpm run test

The cause of this bug is that the canonicalization caches used DefaultMap in places where lookups were expected to be read-only. DefaultMap.get inserts missing entried, which mutated shared cache state during intermediate lookups and made later canonicalization results depend on prior calls.

By replacing the use of DefaultMap with a plain Map, it avoids inserting into the map on lookup paths. I've polyfilled Map#getOrInsert as it is not widely available yet, and used that where appropriate.

Test plan

I wrote a test that fails on main branch, I then fixed the issue, and validated that the test now passes.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

The PR adds two unit tests for canonicalization: one verifying canonical output for a subset of classes and another exercising repeated collapse canonicalization to ensure idempotence. It updates collapseGroup to short-circuit per-candidate processing when a candidate’s property-values mapping lacks line-height or font-size, skipping further work for those candidates. No public API or exported signatures were changed.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(canonicalize): prevent collapse cache pollution across calls' directly and clearly describes the main change - fixing a cache pollution bug in canonicalization.
Description check	✅ Passed	The description is comprehensive and relates directly to the changeset, explaining the bug, its cause, the fix, and providing a test plan.
Linked Issues check	✅ Passed	The PR addresses the core requirement from issue #321 by making canonicalizeCandidates order-independent through fixing DefaultMap usage in caches, preventing cache pollution across multiple calls.
Out of Scope Changes check	✅ Passed	All changes are focused on fixing the cache pollution bug: modified canonicalization logic, added regression tests, and updated changelog - all directly related to the linked issue requirements.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

Fixes an order-sensitive bug in canonicalizeCandidates where collapse-related caches could be unintentionally mutated during read-only lookups, causing later canonicalization results to depend on earlier calls (important for non-deterministic ESLint lint order).

Changes:

• Replaced DefaultMap-backed collapse caches with Map to avoid mutation on lookup paths.
• Added a Map#getOrInsert polyfill and refactored cache access to use explicit “insert-on-miss” behavior.
• Added a regression test ensuring collapse canonicalization is stable across repeated calls.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
packages/tailwindcss/src/utils/map-get-or-insert.ts	Introduces a Map#getOrInsert polyfill used for explicit insert-on-miss cache behavior.
packages/tailwindcss/src/canonicalize-candidates.ts	Refactors static-utility and utility-property caches from DefaultMap to Map, and updates collapse logic to use explicit cache population.
packages/tailwindcss/src/canonicalize-candidates.test.ts	Adds regression coverage for the order-sensitivity/cross-call cache pollution bug.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This module both augments the global Map TypeScript interface and patches Map.prototype at runtime. Since tailwindcss is a published library, this leaks a non-standard API into consumers’ global types/runtime and can lead to conflicts with other tooling or future platform implementations. Prefer a local helper (e.g. getOrInsert(map, key, createValue)), or a small wrapper class used only for these caches, to avoid global/prototype mutation.

Suggested change

	export {}
	declare global {
	interface Map<K, V> {
	getOrInsert(key: K, defaultValue: V): V
	}
	}
	if (typeof Map.prototype.getOrInsert !== 'function') {
	Object.defineProperty(Map.prototype, 'getOrInsert', {
	configurable: true,
	enumerable: false,
	writable: true,
	value: function <K, V>(this: Map<K, V>, key: K, defaultValue: V): V {
	if (!this.has(key)) {
	this.set(key, defaultValue)
	}
	return this.get(key)!
	},
	})
	export function getOrInsert<K, V>(map: Map<K, V>, key: K, defaultValue: V): V {
	if (!map.has(key)) {
	map.set(key, defaultValue)
	}
	return map.get(key)!

[camc314]

this is a fair point, I didn't think about the fact this was a library vs runtime code.

I'll leave this up to a maintainer to decide if this should be fixed or not - please let me know and i'll change it

[coderabbitai]

🧹 Nitpick comments (1)

packages/tailwindcss/src/canonicalize-candidates.ts (1)

2604-2618: Pre-existing bug in the intersection polyfill — returns a copy of a instead of the true intersection on Node.js < 22.

The loop iterates over b and calls result.delete(item) only when !result.has(item) — but if item is not in result (a copy of a), the delete is a no-op. Items in a that are absent from b are never removed. The function effectively returns new Set(a).

This does not affect correctness — the signature comparison at line 399 gates every potential replacement, so only genuinely valid candidates pass through. It does, however, degrade performance in polyfill environments: potentialReplacements is the full otherUtilities[0] set rather than the narrower intersection, so the inner for…of loop iterates over more candidates than necessary.

♻️ Corrected polyfill

  let result = new Set<T>(a)
- for (let item of b) {
-   if (!result.has(item)) {
-     result.delete(item)
-   }
- }
+ for (let item of result) {
+   if (!b.has(item)) {
+     result.delete(item)
+   }
+ }
  return result

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/tailwindcss/src/canonicalize-candidates.ts` around lines 2604 -
2618, The polyfill in function intersection<T>(a: Set<T>, b: Set<T>) is wrong:
it creates result = new Set(a) then iterates b but deletes only when
!result.has(item), which never removes elements of a that are not in b; fix by
computing the true intersection — either create an empty Set and add items from
a that are present in b, or iterate over result (a's copy) and delete any item
not present in b — ensuring the polyfill returns only items common to both a and
b for Node.js <22 environments.

🤖 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.ts`:
- Around line 2604-2618: The polyfill in function intersection<T>(a: Set<T>, b:
Set<T>) is wrong: it creates result = new Set(a) then iterates b but deletes
only when !result.has(item), which never removes elements of a that are not in
b; fix by computing the true intersection — either create an empty Set and add
items from a that are present in b, or iterate over result (a's copy) and delete
any item not present in b — ensuring the polyfill returns only items common to
both a and b for Node.js <22 environments.

[RobinMalfait]

Hey, thanks for the PR!

We track some caches on a per designSystem instance to reduce the amount of computation that we have to do. You are correct that subsequent calls influence the caches.

While looking at the tests and the implementation, I noticed something where your current test has:

designSystem.canonicalizeCandidates(['underline', 'h-4', 'w-4'], options)

Which correctly becomes ['underline', 'size-4']. However, in a subsequent call we added text-sm and then the size-4 was not produced. This made me think about the simpler case:

designSystem.canonicalizeCandidates(['underline', 'h-4', 'w-4', 'text-sm'], options)

Turns out that this on its own also doesn't work.

I added an additional test for this test case, and turns out while doing some debugging fixing this also means that your original test passes. We do collect properties/values so we can use some intersections later. However, we did do something silly where we added some properties of some text utilities to the property/value cache of unrelated candidates (classes). If we only do that for the relevant candidates then we should be good.

[camc314]

I thought there would be a simpler solution!

Thanks for getting to this and maintaining Tailwind!