[system] Counter intuitive behavior when overriding media queries

[rossjs]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

I'm unable to override certain styles on components. In this example, I'm trying to override the minHeight property on a Toolbar component.

https://github.com/mui/material-ui/blob/2a77a330230dfd534225a914b957cd591e7802c3/packages/mui-material/src/styles/createMixins.js#L4-L12

I was eventually able to override that property by adding !important to my style but according to the docs, that doesn't seem like it should be necessary. I would expect any provided styles to be applied last and thus override the default styles.

Expected Behavior 🤔

Based on the documentation, I would expect to be able to...

• use sx to override this directly on the component (<Toolbar sx={{ minHeight: '0px' }}>...)
• OR create a variant in the theme to be able to enable that specific style override
• OR use styled to create a one-off component variant that has those styles applied to it

Steps to Reproduce 🕹

CodeSandbox Example using sx (preferred method), styled, and theme

Context 🔦

I was used to overriding styles via withStyles or the classes prop in v4 but it's unclear to me how to override styles without using !important in my styles.

Your Environment 🌎

`npx @mui/envinfo`

``` System: OS: macOS 11.6 Binaries: Node: 14.16.1 - ~/.nvm/versions/node/v14.16.1/bin/node npm: 7.24.2 - ~/.nvm/versions/node/v14.16.1/bin/npm Browsers: Chrome: 94.0.4606.71 (browser being used) Edge: 94.0.992.38 Firefox: 92.0 Safari: 15.0 npmPackages: @emotion/react: ^11.4.1 => 11.4.1 @emotion/styled: ^11.3.0 => 11.3.0 @mui/core: 5.0.0-alpha.50 @mui/icons-material: ^5.0.3 => 5.0.3 @mui/material: ^5.0.3 => 5.0.3 @mui/private-theming: 5.0.1 @mui/styled-engine: 5.0.1 @mui/styles: ^5.0.1 => 5.0.1 @mui/system: 5.0.3 @mui/types: 7.0.0 @mui/utils: 5.0.1 @types/react: 17.0.14 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 ```

[mnajdova]

Thanks for the report. It has to do with breakpoints and how we are resolving them. See https://codesandbox.io/s/mui-v5-overrides-pain-forked-jjke4?file=/ToolbarsWithSX.jsx:226-282

  sx={{
    minHeight: { xs: "0px", sx: "0px" } // doesn't work
  }}

(if I use breakpoints in the sx prop they are overriding the styles). The styles for the toolbar mixins are adding this CSS property using some media queries this is why it is happening.

[rossjs]

Thanks @mnajdova! Appreciate the workaround!

Is there any way to know which CSS properties might be affected by this for any given component? If not, I guess I'll just keep this in the back of my mind for the time being if I'm having trouble overriding.

[mnajdova]

@rossjs I didn't manage to look into this yesterday, will do it today and report back.

[mnajdova]

I spent some time on this today. It is an interesting issue. It comes from how emotion processes styles (as far as I can understand, but probably @Andarist can correct me if I am wrong).

When we use the styled() utility, all styles are combined together under one classname, for example: css-someHash. If there are breakpoints those are being added in addition using the generated classname, for example @media (min-width: 0px) { .css-someHash: {} }. This is getting complicated when another component is used, or there are some overrides on CSS properties that are already defined in the media queries. Take a look on this codensadbox:https://codesandbox.io/s/funny-browser-lw0iv?file=/src/App.js

import "./styles.css";
import styled from "@emotion/styled";

const Component = styled("div")({
  "@media (min-width: 0px)": {
    background: "blue"
  },
  "@media (min-width: 600px)": {
    background: "red"
  }
});

const OtherComponent = styled(Component)({
  background: "green" // this will never be applied :(
});

export default function App() {
  return (
    <div className="App">
      <Component>Test</Component>
      <OtherComponent>Always green</OtherComponent>
    </div>
  );
}

If we try this using plain CSS it actually depends on the order of which the styles are injected: https://codepen.io/mnajdova/pen/VwzYEJg

---

What should we do?

• We could probably at least document that breakpoints can be overridden only using the same media queries.

not sure if there is there something else we can do at this point to be honest.

[Andarist]

I think the diagnosis here is correct - when you compose styles are "folded" into a single class name and the rest is just CSS behavior.

While this issue is slightly about a different problem I think it might actually shed some additional light on the issue here: https://github.com/emotion-js/emotion/issues/1860 .

[mnajdova]

Thanks for confirming @Andarist. In this case there is nothing much we can do in terms of implementation. I will think about what is the best way to document this.

[rossjs]

Thanks for the help, both of you.

Not to push back too much but is it possible for the library to access a list of CSS properties that are using media queries for each MUI component and then convert something like sx={{ background: 'green' }} to sx={{ background: { xs: 'green' }}}? I assume in practice it would just convert it to the actual media query underneath the hood, but I think you get the gist. I'm not sure it could work for the styled approach but I could see it working for the sx style override approach.

I'm a little wary of a secret list of undocumented CSS properties that can't be overridden without using a media query. Happy to try and put in a PR if you think that's a possibility and can point me to the right place where the sx prop is being resolved. You're intercepting and resolving those media query shortcuts (xs, sm, md, etc) anyway, correct? Since you can define your own custom breakpoints in the theme, I would assume you'd have to be resolving those within the library.

[rossjs]

Also, just to document another workaround for overriding the minHeight property specifically for the Toolbar component, minHeight can be overridden normally with sx if used with variant="dense" since that prevents it from using the theme.mixins that apply the breakpoint-specific media query styling.

https://github.com/mui-org/material-ui/blob/512896973499adbbda057e7f3685d1b23cc02de9/packages/mui-material/src/Toolbar/Toolbar.js#L40-L45

[oliviertassinari]

I didn't anticipate this customization pain coming. Should we categorize it as a bug in emotion? https://codesandbox.io/s/aged-platform-kl7hr?file=/src/App.js

It seems to behave against how CSS, JSS, and styled-components behave. We could also consider it as a regression from v4. Now, it also seems to be a part of the tradeoff to solve the CSS injection order challenge, so we might not be able to do much about it. It's a new behavior in v5, not improving the DX, but it's, what it is.

is it possible for the library to access a list of CSS properties that are using media queries for each MUI component and then convert something like sx={{ background: 'green' }} to sx={{ background: { xs: 'green' }}}?

@rossjs I'm not sure we should explore this path more. The root cause seems to be about emotion merging the style to fix another issue.

On a similar matter, when looking at the style sources, I personally find it confusing not to see the styles of the different styled wrapper, I don't know what style comes from which component when I want to customize them with the theme. Everything is collapsed under a single class name.

http://0.0.0.0:3000/components/bottom-navigation/#bottom-navigation

[oliviertassinari]

I will think about what is the best way to document this.

@mnajdova I wouldn't discount the value of clear GitHub issues. Sometimes, it's all that is needed, assuming it's easy to search and the description clearly describes the symptoms, the issue, and the solution. Now, this is not a slight regression, I imagine many people will hit this.

[rossjs]

is it possible for the library to access a list of CSS properties that are using media queries for each MUI component and then convert something like sx={{ background: 'green' }} to sx={{ background: { xs: 'green' }}}?

@rossjs I'm not sure we should explore this path more. The root cause seems to be about emotion merging the style to fix another issue.

@oliviertassinari That's completely fair since it appears to be related to the order in which emotion is applying styles. I'll take my leave and stop being the squeaky wheel now 😄

Thank you everyone for your hard work on this library though! I've been singing your praises for years.

[Andarist]

It seems to behave against how CSS, JSS, and styled-components behave

I wouldn't really say that this behaves differently than CSS itself: https://codesandbox.io/s/confident-http-3te1h?file=/src/App.js

In a way - this works exactly the same.

On a similar matter, when looking at the style sources, I personally find it confusing not to see the styles of the different styled wrapper, I don't know what style comes from which component when I want to customize them with the theme. Everything is collapsed under a single class name.

I agree that this is not ideal. Only better source maps could help with that (you already have concatenated labels in the class name itself but it's not enough). However, given how this all is heavily dynamic I couldn't find a way to manipulate generated source maps at runtime. The problem is that each style also acts as a "partial" and we can compose them in various, different ways but each partial has a source map of its own and they all point to their respective file names and kinda assume starting at the beginning of the generated ruleset. So the problem is that we'd have to somehow adjust those "offsets" for inner source maps.

This gives me an idea that maybe this could be solved with additional Babel transforms that would inject helpers to manipulate partials coming from the styles objects etc. It could actually know more about the structure of the final styles (how exactly they were compose) and it could track, in better way stuff like:

css`
  ${cls1}
  &:hover {
    ${cls2}
  }
  &:active {
    ${cls3}
  }
  ${cls4}
`

It could actually try to handle this nested structure somewhat well, while the runtime can't do that without getting a major perf hit - for the runtime, this is just a "flat" string.

That being said - the solution would be bound to using a Babel plugin, I'm not entirely sure if it's possible (it's just a new idea that might not work out) and also given its complexity it probably won't happen any time soon.

[mnajdova]

I wouldn't really say that this behaves differently than CSS itself: https://codesandbox.io/s/confident-http-3te1h?file=/src/App.js

@Andarist you need to change the order so that the media query would be first, see https://codesandbox.io/s/nifty-pasteur-i37vc

[Andarist]

Ye, I know 😛 just showcasing how you can end up with this "unwanted" situation with vanilla CSS too (so it's not specific to Emotion)

[mnajdova]

I see what you did there 👀 😄

[siriwatknp]

It looks like the docs are updated. I'm closing this

[oliviertassinari]

@siriwatknp I'm reopening as the docs aren't covering this problem, I think that it's about how a style that is added after a media query (that doesn't add specificity) doesn't win. I think it's a bug/limitation with Emotion. The behavior changed when moving from v4 to v5. I don't think that it's for the better.

import { styled } from "@mui/system";
import { withStyles, makeStyles } from "@mui/styles";

const MUIV4 = withStyles({
  root: {
    background: "yellow",
    "@media (min-width: 0px)": {
      background: "red"
    }
  }
})((props) => {
  return (
    <div {...props} className={`${props.classes.root} ${props.className}`} />
  );
});

const MUIV5 = styled("div")({
  background: "yellow",
  "@media (min-width: 0px)": {
    background: "red"
  }
});

const useStyles = makeStyles({
  root: {
    background: "blue"
  }
});

export default function App() {
  const classes = useStyles();
  return (
    <div style={{ color: "#fff" }}>
      <MUIV4 className={classes.root}>v4: should be blue</MUIV4>
      <MUIV5 sx={{ background: "blue" }}>v5: should be blue</MUIV5>
    </div>
  );
}

https://codesandbox.io/s/confident-cdn-8e6bgc?file=/src/App.js

If we can't fix it, this issue could serve as documentation for it.

[Andarist]

@oliviertassinari if this is about this:

css`
  @media (max-width: 1250px) {
    color: red;
  }
  color: hotpink;
`

producing such CSS:

.css-h4sh { color: hotpink; }
@media (max-width: 1250px) { .css-h4sh { color: red; } }

Then yeah - it's a "limitation" of Emotion. This is exactly how SCSS mixins work too. It's because final styles are not truly tied to components, it's just a "flat" string.

I'm not super intimate with Styled-Component's implementation but from what I can see they are separating styles for each "wrapper". So they are hashing each "partial" separately and putting multiple class names on a component. On top of that - they add the component's "identity" to the hashed value. So for the very same styles, they produce different class names and thus they inject the same thing twice.

So while this case works arguably better with SC, it comes with a price - the generated stylesheet is bigger because by design it can contain "duplicates". In Emotion, we try to minimize the CSS/HTML output, and if the styles are the same we reuse the cached class name. A different set of tradeoffs.

To be fair - we can only skip some identical styles appearing on different components (I feel like the likelihood of that isn't that big). So it's not exactly that our stylesheets would be tremendously smaller.

[oliviertassinari]

if this is about this:

@Andarist Yes, I think that this is what this GitHub issue is about 👌.

When all the styles are in your codebase, it's easier to work around this limitation. But when some of the styles are coming from your code, and the rest is from npm, where you can't easily see that it uses media query, then I think that it's a frustration for the developers that aren't deep into how CSS overrides flows (a lot).

---

To some extent, this GitHub issue could also be about how Emotion merges the styles together. See

https://codesandbox.io/s/frosty-vaughan-pwr1gh?file=/src/App.js

Which style is added by who is clearer with JSS than with Emotion. I think that knowing the origin of a CSS property matters a great deal when you need to further customize the components:

• JSS

• Emotion