5131237d67
I was testing to upgrade tool on various random projects just to see how
it behaves. Then I noticed an odd migration...
This PR fixes an issue where the upgrade tool accidentally migrated
classes such as `mt-[0px]` to `-mt-[0px]`. The reason for this is
because we are trying to find a replacement, and the computed signature
for both of them are exactly the same.
- `mt-[0px]` translates to:
```css
.x {
margin-top: 0px;
}
```
- `-mt-[0px]` translates to:
```css
.x {
margin-top: calc(0px * -1);
}
```
Which in turn translates to
```css
.x {
margin-top: 0px;
}
```
Notice that this is `0px`, not `-0px`.
Internally we use the roots of functional utilities to find
replacements. For intellisense purposes we typically show negative
versions before positive versions. This then means that we will try
`-mt-*` before `mt-*`. Because of the signature above, the `mt-[0px]`
was translated into `-mt-[0px]`.
We could solve this in a few ways. The first thing we can try is to make
sure that the signature is not the same and that `-mt-[0px]` actually
translates into `-0px` not `0px`.
This would solve our problem of the accidental migration. However, if we
_just_ sort the functional utilities roots such that the positive
versions exist before negative version and rely on the fact that
`-mt-[0px]` has the same signature. Then it also means that by doing
that we can migrate `-mt-[0px]` into `mt-[0px]` which is even better
because it's the same result and shorter.
## Test plan
1. Added a test to verify that `mt-[0px]` does not get migrated to
`-mt-[0px]`.
2. Added a test to verify that `-mt-[0px]` does get migrated to
`mt-[0px]`.
119 lines
4.2 KiB
TypeScript
119 lines
4.2 KiB
TypeScript
import { __unstable__loadDesignSystem } from '@tailwindcss/node'
|
|
import { describe, expect, test, vi } from 'vitest'
|
|
import { DefaultMap } from '../../../../tailwindcss/src/utils/default-map'
|
|
import * as versions from '../../utils/version'
|
|
import { migrateCandidate as migrate } from './migrate'
|
|
vi.spyOn(versions, 'isMajor').mockReturnValue(false)
|
|
|
|
const designSystems = new DefaultMap((base: string) => {
|
|
return new DefaultMap((input: string) => {
|
|
return __unstable__loadDesignSystem(input, { base })
|
|
})
|
|
})
|
|
|
|
const css = String.raw
|
|
|
|
describe.each([['default'], ['with-variant'], ['important'], ['prefix']])('%s', (strategy) => {
|
|
let testName = '%s => %s (%#)'
|
|
if (strategy === 'with-variant') {
|
|
testName = testName.replaceAll('%s', 'focus:%s')
|
|
} else if (strategy === 'important') {
|
|
testName = testName.replaceAll('%s', '%s!')
|
|
} else if (strategy === 'prefix') {
|
|
testName = testName.replaceAll('%s', 'tw:%s')
|
|
}
|
|
|
|
// Basic input with minimal design system to keep the tests fast
|
|
let input = css`
|
|
@import 'tailwindcss' ${strategy === 'prefix' ? 'prefix(tw)' : ''};
|
|
@theme {
|
|
--*: initial;
|
|
--spacing: 0.25rem;
|
|
--color-red-500: red;
|
|
|
|
/* Equivalent of blue-500/50 */
|
|
--color-primary: color-mix(in oklab, oklch(62.3% 0.214 259.815) 50%, transparent);
|
|
}
|
|
`
|
|
|
|
test.each([
|
|
// Arbitrary property to named functional utlity
|
|
['[color:red]', 'text-red-500'],
|
|
|
|
// Promote data types to more specific utility if it exists
|
|
['bg-(position:--my-value)', 'bg-position-(--my-value)'],
|
|
|
|
// Promote inferred data type to more specific utility if it exists
|
|
['bg-[123px]', 'bg-position-[123px]'],
|
|
|
|
// Do not migrate bare values or arbitrary values to named values that are
|
|
// deprecated
|
|
['order-[0]', 'order-0'],
|
|
['order-0', 'order-0'],
|
|
|
|
// Migrate deprecated named values to bare values
|
|
['order-none', 'order-0'],
|
|
|
|
// `-0` should not be migrated to `0`.
|
|
//
|
|
// This used to be a bug that translate `mt-[0px]` into `-mt-[0px]` because
|
|
// `-mt-[0px]` translates to `margin-top: calc(0px * -1);` and therefore we
|
|
// handle the `0px * -1` case which translates to `0px` not `-0px`.
|
|
//
|
|
// This translation is actually fine, because now, we will prefer the
|
|
// non-negative version first so we can replace `-mt-[0px]` with `mt-[0px]`.
|
|
['mt-[0px]', 'mt-[0px]'],
|
|
['-mt-[0px]', 'mt-[0px]'],
|
|
])(testName, async (candidate, result) => {
|
|
if (strategy === 'with-variant') {
|
|
candidate = `focus:${candidate}`
|
|
result = `focus:${result}`
|
|
} else if (strategy === 'important') {
|
|
candidate = `${candidate}!`
|
|
result = `${result}!`
|
|
} else if (strategy === 'prefix') {
|
|
// Not only do we need to prefix the candidate, we also have to make
|
|
// sure that we prefix all CSS variables.
|
|
candidate = `tw:${candidate.replaceAll('var(--', 'var(--tw-')}`
|
|
result = `tw:${result.replaceAll('var(--', 'var(--tw-')}`
|
|
}
|
|
|
|
let designSystem = await designSystems.get(__dirname).get(input)
|
|
let migrated = await migrate(designSystem, {}, candidate)
|
|
expect(migrated).toEqual(result)
|
|
})
|
|
|
|
test.each([
|
|
['order-[0]', 'order-0'],
|
|
['order-0', 'order-0'],
|
|
|
|
// Do not migrate `order-none` if defined as a custom utility as it is then
|
|
// not safe to migrate to `order-0`
|
|
['order-none', 'order-none'],
|
|
])(`${testName} with custom implementations`, async (candidate, result) => {
|
|
if (strategy === 'with-variant') {
|
|
candidate = `focus:${candidate}`
|
|
result = `focus:${result}`
|
|
} else if (strategy === 'important') {
|
|
candidate = `${candidate}!`
|
|
result = `${result}!`
|
|
} else if (strategy === 'prefix') {
|
|
// Not only do we need to prefix the candidate, we also have to make
|
|
// sure that we prefix all CSS variables.
|
|
candidate = `tw:${candidate.replaceAll('var(--', 'var(--tw-')}`
|
|
result = `tw:${result.replaceAll('var(--', 'var(--tw-')}`
|
|
}
|
|
|
|
let localInput = css`
|
|
${input}
|
|
|
|
@utility order-none {
|
|
order: none; /* imagine this exists */
|
|
}
|
|
`
|
|
|
|
let designSystem = await designSystems.get(__dirname).get(localInput)
|
|
let migrated = await migrate(designSystem, {}, candidate)
|
|
expect(migrated).toEqual(result)
|
|
})
|
|
})
|