emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.5k stars 1.11k forks source link

Object syntax - Lenient property formatting. #2270

Open jordanjlatimer opened 3 years ago

jordanjlatimer commented 3 years ago

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 commented 3 years ago

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 commented 1 year ago

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: https://github.com/mui/material-ui/issues/30660#issuecomment-1368476632 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 commented 1 year ago

@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.