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.

Sign up for GitHub

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

Jump to bottom

Add option explicitAmpersand on @emotion/cache to handle :is and :where, etc #3210

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add option explicitAmpersand on @emotion/cache to handle :is and :where, etc #3210

wants to merge 5 commits into from

Conversation

thomaslindstrom
Copy link

@thomaslindstrom thomaslindstrom commented Jul 12, 2024 •
edited
Loading

Based on the following issue: #2836
This adds an opt-in option on @emotion/cache explicitAmpersand (unsure about name) that disables the automatic prefixing of pseudo classes in the CSS.

What:

I identified an issue in my own CSS with newer pseudo classes like :where and :is, where Emotion automatically prefixed those classes with the current class name (with no spaces). This was not the expected behavior, and there was no way for me to opt-out of this behavior.

Consider:

<article
  css={css`
    color: black;

    :is(p, ul, ol) + :is(p, ul, ol) {
      margin-top: 1em;
    }

    :where([data-theme="dark"]) & {
      color: white;
    }
  `}
/>;

where the output with the explicitAmpersand set to false (the current behavior) will output:

.emotion-article:is(p, ul, ol)+:is(p, ul, ol) {margin-top: 1em;}
.emotion-article:where([data-theme="dark"]) & {color:white;}

But with explicitAmpersand set to true (new behavior):

.emotion-article :is(p, ul, ol)+:is(p, ul, ol) {margin-top: 1em;}
:where([data-theme="dark"]) .emotion-article {color: white;}

Why:

The current behavior of automatic prefixing of the parent class name is confusing and breaks the behavior of new psuedo classes.

How:

Added an opt-in option on @emotion/cache explicitAmpersand.

Checklist:

  • Documentation
  • Tests
  • Code complete
  • Changeset

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jul 12, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 9d502cd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@emotion/cache Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@thomaslindstrom thomaslindstrom mentioned this pull request Jul 12, 2024
Open
@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Jul 12, 2024 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@thomaslindstrom thomaslindstrom mentioned this pull request Jul 12, 2024
Closed
@Andarist
Copy link
Member

@emmatown I think we should do something about it. In the next major version, we should drop the compat plugin. So the question is, how do we ease the migration? The compat plugin could nudge people to enable this option when it finds a selector that has to be "patched". It's not exactly ergonomic to enable this using cache options... but it's also the only way to do it it only for the current Emotion instance etc. In general, I'm in favor of this change.

@emmatown
Copy link
Member

Unless I'm misunderstanding something about specificity, there's a very simple workaround to this of using *, right? Like *:where() and :where() are equivalent, right?

I still agree with changing the behaviour though, pretty much just since it's the behaviour in the standard CSS nesting now. I'm not sure about the name of the option though since there are still implicit ampersands, they're just inserted in a different place now.

The compat plugin could nudge people to enable this option when it finds a selector that has to be "patched"

By a selector that needs to be "patched", do you mean selectors with things like :hover or selectors with things like :where or both?

@Andarist
Copy link
Member

By a selector that needs to be "patched", do you mean selectors with things like :hover or selectors with things like :where or both?

Both. Currently we "patch" some selectors by inserting & into them. If we plan to change this (and we bot agree that we should) then for migration purposes we should warn about all of them so people can fix all of them in their code before using the new option

@emmatown
Copy link
Member

Both

Sounds good then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thomaslindstrom @Andarist @emmatown