From 5131237d676226ab07d6f0407972622670160b58 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 26 May 2025 16:29:22 +0200 Subject: [PATCH] Fix migrating `mt-[0px]` to `-mt-[0px]` instead of the other way around (#18154) 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]`. --- CHANGELOG.md | 1 + .../codemods/template/migrate-arbitrary-utilities.ts | 7 ++++++- .../src/codemods/template/migrate.test.ts | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc106a20e..dc1469521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Upgrade: Migrate deprecated `order-none` to `order-0` ([#18126](https://github.com/tailwindlabs/tailwindcss/pull/18126)) - Support Leptos `class:` attributes when extracting classes ([#18093](https://github.com/tailwindlabs/tailwindcss/pull/18093)) - Fix "Cannot read properties of undefined" crash on malformed arbitrary value ([#18133](https://github.com/tailwindlabs/tailwindcss/pull/18133)) +- Upgrade: Migrate `-mt-[0px]` to `mt-[0px]` instead of the other way around ([#18154](https://github.com/tailwindlabs/tailwindcss/pull/18154)) ## [4.1.7] - 2025-05-15 diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-utilities.ts b/packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-utilities.ts index c03100cdd..63aa9396d 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-utilities.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/migrate-arbitrary-utilities.ts @@ -161,7 +161,12 @@ export function migrateArbitraryUtilities( let spacingMultiplier = spacing.get(designSystem)?.get(value) - for (let root of designSystem.utilities.keys('functional')) { + for (let root of Array.from(designSystem.utilities.keys('functional')).sort( + // Sort negative roots after positive roots so that we can try + // `mt-*` before `-mt-*`. This is especially useful in situations where + // `-mt-[0px]` can be translated to `mt-[0px]`. + (a, z) => Number(a[0] === '-') - Number(z[0] === '-'), + )) { // Try as bare value for (let replacementCandidate of parseCandidate(designSystem, `${root}-${value}`)) { yield replacementCandidate diff --git a/packages/@tailwindcss-upgrade/src/codemods/template/migrate.test.ts b/packages/@tailwindcss-upgrade/src/codemods/template/migrate.test.ts index 5c91f3d07..216be87e3 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/template/migrate.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/template/migrate.test.ts @@ -53,6 +53,17 @@ describe.each([['default'], ['with-variant'], ['important'], ['prefix']])('%s', // 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}`