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

Migrate aria-*, data-* and supports-* variants from arbitrary values to bare values #14644

Merged
merged 5 commits into from
Oct 11, 2024

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented Oct 10, 2024

This PR adds a new codemod that can migrate data-* and aria-* variants using arbitrary values to bare values.

In Tailwind CSS v3, if you want to conditionally apply a class using data attributes, then you can write data-[selected]:flex. This requires the DOM element to have a data-selected="" attribute. In Tailwind CSS v4 we can simplify this, by dropping the brackets and by using data-selected:flex directly.

This migration operates on the internal AST, which means that this also just works for compound variants such as group-has-data-[selected]:flex (which turns into group-has-data-selected:flex).

Additionally, this codemod is also applicable to aria-* variants. The biggest difference is that in v4 aria-selected maps to an attribute of aria-selected="true". This means that we can only migrate aria=[selected="true"]:flex to aria-selected:flex.

Last but not least, we also migrate supports-[gap] to supports-gap if the passed in value looks like a property. If not, e.g.: supports-[display:grid] then it stays as-is.

@thecrypticace
Copy link
Contributor

thecrypticace commented Oct 10, 2024

This brings me such joy 🥳

I think we can migrate supports-* too! (if the arbitrary value matches /^[a-z0-9-]+$/ at least)

variant.kind === 'functional' &&
variant.root === 'aria' &&
variant.value?.kind === 'arbitrary' &&
variant.value.value.endsWith('="true"')
Copy link
Member

Choose a reason for hiding this comment

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

Is it valid to pass all CSS attribute selectors here? e.g. :!= or *= etc. This would match more often than we want it in that case

Copy link
Contributor

Choose a reason for hiding this comment

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

The goal here is to match what the user would've typed in the variant in v3 that is equivalent to the variant in v4 so anything other than ="true" would be wrong because ="true" is what v4 uses.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah but I see what @philipp-spiess means. If we have aria-[foo*="true"] then this could will match, but it should not match because we can't safely migrate that. Will adjust and add a test.

Copy link
Member

Choose a reason for hiding this comment

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

Haha yeah that's what I meant! This will then migrate to aria-foo* which seems unexpected

Copy link
Contributor

Choose a reason for hiding this comment

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

ooohhhh I see! yep that's def an issue

Base automatically changed from feat/migrate-legacy-classes to next October 11, 2024 12:22
@RobinMalfait RobinMalfait force-pushed the feat/migrate-data-and-aria-arbitrary-values branch from a99b4b8 to 85f1a07 Compare October 11, 2024 13:32
@RobinMalfait RobinMalfait changed the title Migrate data-* and aria-* variants from arbitrary values to bare values Migrate aria-*, data-* and supports-* variants from arbitrary values to bare values Oct 11, 2024
Comment on lines +35 to +50
(variant.value.value.endsWith('=true') ||
variant.value.value.endsWith('="true"') ||
variant.value.value.endsWith("='true'"))
) {
let [key, _value] = segment(variant.value.value, '=')
if (
// aria-[foo~="true"]
key[key.length - 1] === '~' ||
// aria-[foo|="true"]
key[key.length - 1] === '|' ||
// aria-[foo^="true"]
key[key.length - 1] === '^' ||
// aria-[foo$="true"]
key[key.length - 1] === '$' ||
// aria-[foo*="true"]
key[key.length - 1] === '*'
Copy link
Member Author

Choose a reason for hiding this comment

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

I had a regex that covered all of these cases but it was horrible so made it more explicit 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

lol its almost all special regex chars 😂 — I don't think any of them need to be escaped in a character class tho right? (unless ^ was listed first iirc). I think its this right /[~|^$*]$/.test(key)?

Copy link
Contributor

Choose a reason for hiding this comment

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

explicit list of cases is fine though. It definitely reads better.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the one I used originally /[^~|^$*]=(['"]?)true\1/

Copy link
Contributor

Choose a reason for hiding this comment

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

ahhh yeah that is kinda awful lol

@RobinMalfait RobinMalfait force-pushed the feat/migrate-data-and-aria-arbitrary-values branch 2 times, most recently from 0ce28b0 to 180b5aa Compare October 11, 2024 14:02
@RobinMalfait RobinMalfait force-pushed the feat/migrate-data-and-aria-arbitrary-values branch from 180b5aa to 524cdbc Compare October 11, 2024 14:02
@adamwathan adamwathan merged commit e6d3fa0 into next Oct 11, 2024
1 check passed
@adamwathan adamwathan deleted the feat/migrate-data-and-aria-arbitrary-values branch October 11, 2024 14:41
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.

4 participants