Skip to content
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

Improve in-* variant migrations #15054

Merged
merged 11 commits into from
Nov 20, 2024
Merged

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Nov 20, 2024

While testing the codemods on some projects, I noticed some issues with the migration to the new in-* variant.

One such example is that we checked for & at the end, instead of & (the whitespace is significant).

This meant that [figure>&]:my-0 was converted to in-[figure>]:my-0 which is wrong. In this case, we want to keep it as [figure>&]:my-0.

Additionally this PR brings back the migration from group-[]:flex to in-[.group]:flex. If you are using a prefix, then group-[]:tw-flex is migrated to tw:in-[.tw\:group]:flex.

Last but not least, this does some internal refactors to group migrations logically together.

This test fails and was converted to `in-[figure>]` which is incorrect.
The reason is because we only checked for `&` and the very end, but it
should be preceded by a combinator space.
It's a little bit silly, but during debugging I noticed that the
`variant` looked all funky. This is because we convert one variant to
another via `Object.assign`.

It does work properly because we compare the `kind` field (which gets
converted), and then we know that all the properties of the new `kind`
are available.

With this change we first clear out all the existing keys, resulting in
an expected shape when inspecting the `variant`.
If you use a prefix, migrate `group-[]:tw-flex` to `tw:in-[.tw\:group]:flex`
Instead of bailing early, let's nest it to keep it consistent with other
migrations in the current file. Every "migration" should be guarded by
some if statements.
If multiple nodes are valid, it could result in different specificity
because the `in-*` variant wraps everything in a `:where(…)`.
@RobinMalfait RobinMalfait requested a review from a team as a code owner November 20, 2024 14:21
@RobinMalfait RobinMalfait enabled auto-merge (squash) November 20, 2024 14:46
@RobinMalfait RobinMalfait force-pushed the fix/improve-in-variant-migrations branch from 0f14708 to 2c3293b Compare November 20, 2024 14:47
@RobinMalfait RobinMalfait merged commit 57be02a into next Nov 20, 2024
1 check passed
@RobinMalfait RobinMalfait deleted the fix/improve-in-variant-migrations branch November 20, 2024 14:47
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