fix(canonicalize): prevent collapse cache pollution across calls (#19675)
## Summary fixes https://github.com/schoero/eslint-plugin-better-tailwindcss/issues/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`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/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. --------- Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
This commit is contained in:
parent
d520e1f571
commit
5a4a7eba3a
3 changed files with 48 additions and 0 deletions
|
|
@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
- Improve performance Oxide scanner in bigger projects ([#19632](https://github.com/tailwindlabs/tailwindcss/pull/19632))
|
||||
- Ensure import aliases in Astro v5 work without crashing ([#19677](https://github.com/tailwindlabs/tailwindcss/issues/19677))
|
||||
- Allow escape characters in `@utility` names to improve support with formatters such as Biome ([#19626](https://github.com/tailwindlabs/tailwindcss/pull/19626))
|
||||
- Fix invalid cache during subsequent canonicalization calls ([#19675](https://github.com/tailwindlabs/tailwindcss/pull/19675))
|
||||
|
||||
### Deprecated
|
||||
|
||||
|
|
|
|||
|
|
@ -1163,3 +1163,46 @@ describe('options', () => {
|
|||
expect(designSystem.canonicalizeCandidates(['m-[16px]'])).toEqual(['m-[16px]']) // Ensure options don't influence shared state
|
||||
})
|
||||
})
|
||||
|
||||
// https://github.com/schoero/eslint-plugin-better-tailwindcss/issues/321
|
||||
test('a subset of classes should be canonicalizable', { timeout }, async () => {
|
||||
let designSystem = await designSystems.get(__dirname).get(css`
|
||||
@import 'tailwindcss';
|
||||
`)
|
||||
|
||||
let options: CanonicalizeOptions = {
|
||||
collapse: true,
|
||||
logicalToPhysical: true,
|
||||
rem: 16,
|
||||
}
|
||||
|
||||
expect(
|
||||
designSystem.canonicalizeCandidates(['underline', 'h-4', 'w-4', 'text-sm'], options),
|
||||
).toEqual(['underline', 'text-sm', 'size-4'])
|
||||
})
|
||||
|
||||
test('collapse canonicalization is not affected by previous calls', { timeout }, async () => {
|
||||
let designSystem = await designSystems.get(__dirname).get(css`
|
||||
@import 'tailwindcss';
|
||||
`)
|
||||
|
||||
let options: CanonicalizeOptions = {
|
||||
collapse: true,
|
||||
logicalToPhysical: true,
|
||||
rem: 16,
|
||||
}
|
||||
|
||||
let target = ['underline', 'h-4', 'w-4']
|
||||
|
||||
expect(designSystem.canonicalizeCandidates(target, options)).toEqual(['underline', 'size-4'])
|
||||
|
||||
designSystem.canonicalizeCandidates(['mb-4', 'text-sm'], options)
|
||||
designSystem.canonicalizeCandidates(['underline', 'mb-4'], options)
|
||||
|
||||
expect(designSystem.canonicalizeCandidates(target, options)).toEqual(['underline', 'size-4'])
|
||||
expect(designSystem.canonicalizeCandidates(target.concat('text-sm'), options)).toEqual([
|
||||
'underline',
|
||||
'text-sm',
|
||||
'size-4',
|
||||
])
|
||||
})
|
||||
|
|
|
|||
|
|
@ -267,6 +267,8 @@ function collapseCandidates(options: InternalCanonicalizeOptions, candidates: st
|
|||
let interestingLineHeights = new Set<string | number>()
|
||||
let seenLineHeights = new Set<string>()
|
||||
for (let pairs of candidatePropertiesValues) {
|
||||
if (!pairs.has('line-height')) continue
|
||||
|
||||
for (let lineHeight of pairs.get('line-height')) {
|
||||
if (seenLineHeights.has(lineHeight)) continue
|
||||
seenLineHeights.add(lineHeight)
|
||||
|
|
@ -292,6 +294,8 @@ function collapseCandidates(options: InternalCanonicalizeOptions, candidates: st
|
|||
|
||||
let seenFontSizes = new Set<string>()
|
||||
for (let pairs of candidatePropertiesValues) {
|
||||
if (!pairs.has('font-size')) continue
|
||||
|
||||
for (let fontSize of pairs.get('font-size')) {
|
||||
if (seenFontSizes.has(fontSize)) continue
|
||||
seenFontSizes.add(fontSize)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue