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

Object syntax - Lenient property formatting. #2270

Open
jordanjlatimer opened this issue Feb 25, 2021 · 3 comments
Open

Object syntax - Lenient property formatting. #2270

jordanjlatimer opened this issue Feb 25, 2021 · 3 comments

Comments

@jordanjlatimer
Copy link

jordanjlatimer commented Feb 25, 2021

Description:
Version: 11.1.5

I've discovered that when using object syntax, you can be more relaxed when specifying child elements or pseudo elements than the documentation seems to let on.

The doc's examples do this:

// pseudos
{
  "&:hover": {...}
}

//children
{
  "& div": {...}
}

But I've found the following also work:

//pseudos
{
  ":hover": {...}
}

//children
{
  " div": {...},
  "div": {...},
  div: {...},
}

Is this behavior intentional? Should one avoid using the additional formats I showed?

BTW, when using IDE autocompletion it suggests ":hover" without the ampersand.

Documentation links:
https://emotion.sh/docs/object-styles

@Andarist
Copy link
Member

The object styles are serialized to a string so in a case like { " div": {} } the whitespace doesn't matter - in the very same way it doesn't matter in a string style.

As to &:hover vs :hover - I would encourage you to use the former because :hover should actually be equivalent to & :hover, we are only patching to be equivalent to &:hover. And the patching requires some additional, small, runtime cost. This comes from the fact that we were previously using Stylis 3 which has chosen incorrect behavior for this case if we consider what has been in Sass, Less, and others the correct one. This has been fixed in Stylis 4 but to maintain compatibility on our side we are patching things at runtime so people can migrate more easily.

@oliviertassinari
Copy link

oliviertassinari commented Jan 1, 2023

This has been fixed in Stylis 4 but to maintain compatibility on our side we are patching things at runtime so people can migrate more easily.

I think that it could be great to eventually remove the patch for :hover in emotion. I see issues with this:

  1. It can create confusion: [discussion] Preparing v6 mui/material-ui#30660 (comment) with the other selectors.
  2. It breaks what people learn with SASS and LESS. We can compare the output of
.id {
  color: hotpink;

  :hover {
    color: red;
  }
}
.id {
  color: hotpink;
}
.id :hover {
  color: red;
}
.id {
  color: hotpink;
}
.id :hover {
  color: red;
}
.id {
  color: hotpink;
}
.id:hover {
  color:  red;
}

As a side note, the spec for https://www.w3.org/TR/css-nesting-1/ seems to require either & or :nest to be used, so maybe it could be interesting to not support :hover at all.

@Andarist
Copy link
Member

Andarist commented Jan 2, 2023

@oliviertassinari yeah, I plan to remove this code in the next major version. It was only introduced to ease the transition from Emotion 10 to Emotion 11.

For the time being, we probably gonna stick to LESS/SASS semantics. They are still vastly popular and this is what people got used to. This doesn't conflict with the CSS nesting proposal - it's just an additional syntax that would be allowed.

If you find this compat plugin to be problematic with some of the things that you are doing in MUI - let me know, I could try to figure out what can be done about it in Emotion 11 without breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants