Skip to content

Fix migrating mt-[0px] to -mt-[0px] instead of the other way around #18154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 26, 2025

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented May 26, 2025

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:

    .x {
      margin-top: 0px;
    }
  • -mt-[0px] translates to:

    .x {
      margin-top: calc(0px * -1);
    }

    Which in turn translates to

    .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].

@RobinMalfait RobinMalfait requested a review from a team as a code owner May 26, 2025 12:36
Copy link
Contributor

@thecrypticace thecrypticace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and simple 👍

@RobinMalfait RobinMalfait merged commit 5131237 into main May 26, 2025
7 checks passed
@RobinMalfait RobinMalfait deleted the fix/handle-negative-zeroes branch May 26, 2025 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants