Migrate arbitrary values to built-in values #14841
Conversation
cf849fb to
1500ca7
Compare
| ast.nodes[0].nodes[2].type === 'attribute' | ||
| ) { | ||
| ast.nodes[0].nodes = [ast.nodes[0].nodes[2]] | ||
| addChildVariant = true |
There was a problem hiding this comment.
Only thing I can think of is that *:data-visible:{utility} is technically something like :where(& > [data-visible]) but it might be fine to ignore that detail.
If we're cool with that this all looks good.
There was a problem hiding this comment.
Thinking about this, I think the new value is more correct because with just [&>[data-visible]] you run into more issues: https://play.tailwindcss.com/aPcIgi2nc5
There was a problem hiding this comment.
I'm fine with making the change. Maybe should output a small note when we find it in case the old behavior is being relied on?
There was a problem hiding this comment.
I'd also be fine with doing that in a follow up PR honestly.
There was a problem hiding this comment.
That's an option. We technically already mention to verify the changes. What are your thoughts on this /cc @philipp-spiess @adamwathan
There was a problem hiding this comment.
Yeah that's fair. Maybe that's fine.
There was a problem hiding this comment.
Tricky one since the specificity is different, not sure. We probably can change it but trying to think of cases where it would matter. Should test in Catalyst which does a lot of this sort of thing.
There was a problem hiding this comment.
There are indeed cases in Catalyst where it would break if we do this conversion sadly. We could go through Catalyst and make sure that this isn't an issue, but that doesn't solve it for our users.
I'll revert this particular change.
There was a problem hiding this comment.
Revert this back because we made * use :is(…) instead of :where(…).
5ce36a4 to
c23cdcb
Compare
60b5b25 to
ddf193d
Compare
3d1df7f to
bffd222
Compare
|
I was working on a separate PR for additional conversions similar to this. Since this is not yet merged, I think it's better to just push this one commit here as well. |
|
E.g.: from Catalyst 
|
c092898 to
b846470
Compare
|
Re-introduced the |
224cdc7 to
65922e3
Compare
A bunch of changes happened since this review, so I think it's best to re-review.
65922e3 to
9919924
Compare
E.g.: `has-[[data-visible]]` can be converted `has-data-visible`
Co-authored-by: Adam Wathan <4323180+adamwathan@users.noreply.github.com>
And add support for: - nth - nth-last - nth-of-type - nth-of-last-type
This reverts commit 14b1854.
9919924 to
642fa95
Compare
philipp-spiess
left a comment
There was a problem hiding this comment.
Amazing what we can do with the codemods! I did find two of the examples in the test plan slightly confusing, however I don't think they make anything worse than it is right now so LGTM
| ['[[data-url*="example"]]:flex', 'data-[url*="example"]:flex'], | ||
| ['[[data-url$=".com"_i]]:flex', 'data-[url$=".com"_i]:flex'], | ||
| ['[[data-url$=.com_i]]:flex', 'data-[url$=.com_i]:flex'], |
There was a problem hiding this comment.
None of these seem to work in v3, but it is valid in v4 alpha. However I think the codemods currently are not intended to run on the alpha b/c of the variant reordering so may be able to rm support for this
There was a problem hiding this comment.
Ah true, in v3 you need the &, but the main thing I was testing here is the flag placement and strings vs no strings.
E.g.: [&[data-url*="example"]]:flex
|
|
||
| // Compound arbitrary variants | ||
| ['has-[[data-visible]]:flex', 'has-data-visible:flex'], | ||
| ['has-[&:is([data-visible])]:flex', 'has-data-visible:flex'], |
There was a problem hiding this comment.
has-[&:is([data-visible])]:flex has a specificity of 0, 3, 0 in v3 but [has-[&:is([data-visible])]:flex](has-data-visible:flex) is 0, 2, 0 in v4.
HOWEVER has-[&:is([data-visible])]:flex is also 0, 2, 0 in v4 so keeping this wouldn't fix it either.
There was a problem hiding this comment.
Yeah exactly, it's a side effect of the implementation in v4 unrelated to this change. If you want to be 100% the same, we should switch to arbitrary variants instead but that makes it worse.
This PR adds a migration where we detect arbitrary variants and try to upgrade them to built-in variants.
For example, if you are using
[[data-visible]]:flex, we can convert it todata-visible:flex. We can also upgrade more advanced examples such ashas-[[data-visible]]:flexto a compound variant which becomeshas-data-visible:flex.A table of example migrations:
[[data-visible]]:flexdata-visible:flex[&[data-visible]]:flexdata-visible:flex[[data-visible]&]:flexdata-visible:flex[[data-url*="example"]]:flexdata-[url*="example"]:flex[[data-url$=".com"_i]]:flexdata-[url$=".com"_i]:flex[[data-url$=.com_i]]:flexdata-[url$=.com_i]:flex[&:is([data-visible])]:flexdata-visible:flexhas-[[data-visible]]:flexhas-data-visible:flexhas-[&:is([data-visible])]:flexhas-data-visible:flexhas-[[data-slot=description]]:flexhas-data-[slot=description]:flexhas-[&:is([data-slot=description])]:flexhas-data-[slot=description]:flexhas-[[aria-visible="true"]]:flexhas-aria-visible:flexhas-[[aria-visible]]:flexhas-aria-[visible]:flexWe can also convert combinators from
[&>[data-visible]]:flexto just*:data-visible:flexand[&_[data-visible]]:flexto**:data-visible:flex.[&>[data-visible]]:flex*:data-visible:flex[&_>_[data-visible]]:flex*:data-visible:flex[&_[data-visible]]:flex**:data-visible:flexAdditionally, if you have complex selectors with
:not(), we can convert this to a compoundnot-*variant in some cases as well:[&:nth-child(2)]:flexnth-2:flex[&:not(:nth-child(2))]:flexnot-nth-2:flexIf some of the values in
nth-child(…)are a bit too complex, then we still try to convert them but to arbitrary values instead.[&:nth-child(-n+3)]:flexnth-[-n+3]:flex[&:not(:nth-child(-n+3))]:flexnot-nth-[-n+3]:flexThis also implements some optimizations around
evenandodd:[&:nth-child(odd)]:flexodd:flex[&:not(:nth-child(odd))]:flexeven:flex[&:nth-child(even)]:flexeven:flex[&:not(:nth-child(even))]:flexodd:flexSome examples that stay as-is:
has-[&>[data-visible]]:flexwe can't upgrade this one becausehas-*is not a valid variant.has-[[data-visible][data-dark]]:flexwe can't upgrade this one because[data-visible][data-dark]has to be on the same element. If we convert this tohas-data-visible:has-data-dark:flexthen this condition will be true if an element exists withdata-visibleand another element exists withdata-darkbut we don't guarantee that they are the same element.Running this on the Catalyst codebase results in some updates that look like this: