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

Don't prefix arbitrary classes in peer/group variants #11454

Merged
merged 5 commits into from Jun 28, 2023

Conversation

thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Jun 19, 2023

Fixes #11384

This PR stops us from prefixing selectors in the group and peer variants.

For example, given the following HTML:

<div class="tw-group foo">
  <div class="group-[&.foo]:tw-bg-blue-500">
    hello world
  </div>
</div>

The generated selector would contain .tw-foo instead of .foo as the user intended:

.tw-group.tw-foo .group-\[\&\.foo\]\:tw-bg-blue-500

Whereas this should be the selector that is generated

.tw-group.foo .group-\[\&\.foo\]\:tw-bg-blue-500

The reason this happens is because of the design of the addVariant and matchVariant APIs. We build up a one or more selectors that are used to "modify" the resulting utility selector in your CSS. After we've build up the final list of "changes" to a given utility, taking into account all it's variants, we then add prefixes to all classes in the selector (for example, group becomes tw-group, dark becomes tw-dark, underline becomes tw-underline, etc…). The problem with this approach is, at this point, there's no information about what data was derived from arbitrary values and thus no mechanism by which we can say "prefix class .a but not .b" or "prefix .a but not in this one instance".

How is it that we don't know? After all, aren't we matching against the utility?

Take for example this very simplified version of the group variant:

matchVariant("group", (value) => `.group${value} &`)

When matching the utility group-[.bar]:flex and building its rule we then process the foo variant and generate a selector. We place the value .bar in for the value parameter and the function returns .group.bar &.

This is almost as if we had written this:

addVariant("group-[.bar]", `.group.bar &`)

You'll notice here there are no prefixes present yet — this is because, in general, this is handled later in the pipeline for variants.

Now, we explicitly call prefix function for the group class and disable prefixing later in the pipeline for this variant only. This effectively treats the variant as if it were defined like so:

addVariant("group-[.bar]", `.tw-group.bar &`, { respectPrefix: false })

In which case no prefixing is done after building the variant selector because it includes prefixes already.

Now, given that existing user plugins may be defining variants AND those variants have their classes prefixed we can't just disable it wholesale and expect it to be handled in the variant. Additionally, since there's no information about what data was derived from arbitrary values (AND don't even have to be selectors!), to preserve backwards compat while also fixing this we've updated the group and peer variants specifically such that:

  1. The relevant group, peer, group/name-here, peer/name-here, etc… classes are prefixed inside the variant function that generates the selector.
  2. Exposed an internal-for-now flag for variants called respectPrefix — much like the respectPrefix flag present in addUtilities, matchUtilities, addComponents, etc…
  3. Used this flag such that the auto-prefixing that happens later in the pipeline is disabled for this specific variant (like we already do for arbitrary variants — it is infact using the same code path in the end)

This flag is left as internal for now as we want to be sure of the API before we expose it to user-land, work on documentation, etc…

Then add the `NoPrefix` feature to the variant itself, which will skip
prefixing any other class in the generated selector (because we already
took care of prefixing `.group` and `.peer`).

We are using an internal symbol such that:

- We can keep it as a private API
- We don't introduce a breaking change
We will still use a symbol as an internal/private marker, but the data
itself will be a simple object for now.

If we want to refactor this (and more) in the future using bitflags then
we can refactor that in a separate PR.
@thecrypticace thecrypticace merged commit 615c157 into master Jun 28, 2023
10 checks passed
@thecrypticace thecrypticace deleted the fix/no-prefix-group-peer branch June 28, 2023 16:37
thecrypticace added a commit that referenced this pull request Jul 13, 2023
* Refactor

* Don’t prefix classes in arbitrary values for group and peer

* use `foo` instead of `lol`

* handle the prefix inside the group/peer variants

Then add the `NoPrefix` feature to the variant itself, which will skip
prefixing any other class in the generated selector (because we already
took care of prefixing `.group` and `.peer`).

We are using an internal symbol such that:

- We can keep it as a private API
- We don't introduce a breaking change

* refactor to simple object instead

We will still use a symbol as an internal/private marker, but the data
itself will be a simple object for now.

If we want to refactor this (and more) in the future using bitflags then
we can refactor that in a separate PR.

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
thecrypticace added a commit that referenced this pull request Jul 13, 2023
* Refactor

* Don’t prefix classes in arbitrary values for group and peer

* use `foo` instead of `lol`

* handle the prefix inside the group/peer variants

Then add the `NoPrefix` feature to the variant itself, which will skip
prefixing any other class in the generated selector (because we already
took care of prefixing `.group` and `.peer`).

We are using an internal symbol such that:

- We can keep it as a private API
- We don't introduce a breaking change

* refactor to simple object instead

We will still use a symbol as an internal/private marker, but the data
itself will be a simple object for now.

If we want to refactor this (and more) in the future using bitflags then
we can refactor that in a separate PR.

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
thecrypticace added a commit that referenced this pull request Jul 13, 2023
* Refactor

* Don’t prefix classes in arbitrary values for group and peer

* use `foo` instead of `lol`

* handle the prefix inside the group/peer variants

Then add the `NoPrefix` feature to the variant itself, which will skip
prefixing any other class in the generated selector (because we already
took care of prefixing `.group` and `.peer`).

We are using an internal symbol such that:

- We can keep it as a private API
- We don't introduce a breaking change

* refactor to simple object instead

We will still use a symbol as an internal/private marker, but the data
itself will be a simple object for now.

If we want to refactor this (and more) in the future using bitflags then
we can refactor that in a separate PR.

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
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.

Group Class with prefix config doesn't work
2 participants