emotion-js / emotion

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

[Global] `styles` prop type is probably too wide #2355

Open mnajdova opened 3 years ago

mnajdova commented 3 years ago

Current behavior:

The following are accepted:

<Global styles={true} /> <Global styles={false} />

Expected behavior:

It's unclear what this is supposed to do. There isn't anything about these values in the documentation.

Environment information:


First reported in https://github.com/mui-org/material-ui/issues/25208

Andarist commented 3 years ago

We can be a little bit more strict here and create GlobalStyles which would use Interpolation<Theme> in it.

Things like <Global styles={[false]} /> should still be supported though as we need to support this: <Global styles={[bool && stylesA, stylesB]} />. So the type change would only have to affect the very "root" of the type - but the same type should be used within it.

This doesn't have to be supported: <Global styles={bool && style} /> since it's better to do this: bool && <Global styles={style} />. Although, I wonder if the complication is worth the complication here as the gains seem to be rather minor.

mnajdova commented 3 years ago

This doesn't have to be supported: <Global styles={bool && style} /> since it's better to do this: bool && . Although, I wonder if the complication is worth the complication here as the gains seem to be rather minor.

It's not critical, but more of a nice to have I would say. Not sure how big of a complication it would be. Whatever you think it's best. If not we will try to restrict them on our end if possible.