material-ui: `classes` styles don't override default when using @emotion/react

Abstract

Who it’s affecting

This issues only affect users that uses @emotion/react to write their custom styles.

What is the issue

The styles provided to the classes property of MUI components don’t take priority over the internal styles and there is no way around it.

Original issue

Hi,

Following up with 27945 I think there is (arguably) a unexpected behaviour in the way the classes prop is handled when the user is using emotion and the same cache that material-ui is using.

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

Current Behavior 😯

The default style have the priority hover style provided in classes. (in a specific context described below)

Expected Behavior 🤔

The styles explicitely provided by users in classes should always overwrite the defaults.

Steps to Reproduce 🕹

sandbox

Steps:

  1. Remove the !important line 21
  2. Reload and witness that the text is no longer pink.

Context

This issue is not related to SSR and it is not specific to tss-react.

My investigation

mui_shared_cache_issue

Best,

About this issue

  • Original URL
  • State: closed
  • Created 3 years ago
  • Reactions: 5
  • Comments: 19 (18 by maintainers)

Most upvoted comments

Hi @mnajdova,
Let me clarify because I think there is a misunderstanding about what the issue is:
I assert that in the very specific case where:

  • Material-ui uses emotion as style engine.
  • The user creates custom styles with emotion using the same cache as Material-ui.

Then, and only in this very specific configuration, you guys should be able, using cx internally, to ensure that the styles provided to mui components using className or classes take priority over the default mui styles.

<del>Now, you can consider this configuration as being to much of an edge case to bother addressing it.
In this case I’d suggest mentioning in the documentation that this configuration isn’t supported.</del> I kinda disagree with my own statement now since it’s always the case when users uses @emotion/react for custom styles and there is no way around it.

PS: This is not something that I need fixed for tss-react. I just thought you might find this report useful.

If you are OK, we can follow the discussion on the basis of a PR?

Yes, let’s start with a PR and see what the final API would look like

Hi @mnajdova, thanks for your answer,

We would need to export an cx like utility from the @mui/styled-engine-sc package that will simply concat classes, as styled-components does not have a cx util.

Precisely!

Regarding emotion, can we do this without requiring the apps to be wrapper with a cache provider?

Yes, I can use unstable_useEmotionCache in tss-react to pickup whatever cache is used.

As far as I saw, having the unstable_useEmotionCache should allow us to do that (we can have default cache, and we can add a similar hook in the @mui/styled-engine-sc package.

The great thing is that we won’t need to change anything beside exposing cx. Event if no cache is explicitly provided by the user unstable_useEmotionCache never returns null in the browser so tss-react always have a way to pickup whatever cache mui is using.

If you are interested to give this a go, feel free to, I can help wtih the @mui/styled-engine-sc package. The idea is this package and the @mui/styled-engine package should expose the same API, so that they can be interchangeable.

Yes! I am very interested, @mui/styled-engine-sc would export clsx as classNames and @mui/styled-engine would export  emotion’s cx as classNames.
Emotion’s cx is an implementation of the classNames API that detects emotion generated class names ensuring styles are overwritten in the correct order. (I am sure you know that already but I document it for others).

If you are OK, we can follow the discussion on the basis of a PR?

We would need to export an cx like utility from the @mui/styled-engine-sc package that will simply concat classes, as styled-components does not have a cx util. Regarding emotion, can we do this without requiring the apps to be wrapper with a cache provider? As far as I saw, having the unstable_useEmotionCache should allow us to do that (we can have default cache, and we can add a similar hook in the @mui/styled-engine-sc package.

If you are interested to give this a go, feel free to, I can help wtih the @mui/styled-engine-sc package. The idea is this package and the @mui/styled-engine package should expose the same API, so that they can be interchangeable.

That’s great to hear.
I am willing to submit a PR if you give me the green light.
For recall, what needs to be done is to internally use cx to make sure the styles provided by users in classes overwrite the internal.

I am very motivated because there is something else I forgot to mention. Right now, I need specific instructions to instruct users how to setup SSR.
It’s not a big deal for first-hand TSS users but it is one for users relying on libraries that use TSS internally. mui-datatable for example.
I’ve gone as far as getting a new API into emotion to enable me to pick up the cash but there is still this issue in the way.

Best regards,

I won’t do any changes at this moment and wait to see if there will be more activity on the issue.

The other points I made still stands though.

Ok, glad we are on the same page 😃 Will report back tomorrow after I spend some more time on this 😃

Some projects built with mui v4 relies on the fact that className used to take priority over classes.root when using makeStyle. It is no longer the case and there is no way for users to emulate this behavior in v5.

I am a but confused with this statement. In the codesandbox that you shared, when using classes.root the overrides did not work, but they did work when you used className. This is the opposite of what you’ve just said. Maybe an example to illustrate would help.

In v4 as far as I know it doesn’t matter if you provide the classes generated from makeStyles to the className or classes, it works the same way, as which one will take precedence is based on when the useStyles hook is invoked.

Anyway, more detailed example of v4 vs v5 that proves this would be helpful, as I really don’t think using classes generated by makeStyles take presedence based on whether they are passed to className vs classes.root.

Thanks a lot for the clarification @garronej I will anyway spend some more time tomorrow, my biggest worry is that cx is emotion only API, so I would need to export it as a custom function for the other styled engine in order to make sure that the same interface is exported. Will report back on the issue what I’ve come up with.

I have observed this same behaviour. Initially I couldn’t even see where my styles were being applied, but eventually I noticed them merged into one giant rule. I have observed similar behaviour with className too. This is a departure from v4, I believe.

In the sandbox example above, the combined style .foo-bar-XXXXXX-MuiButtonBase-root-MuiButton-root has three color properties on it. Given that the last one wins, if this approach to merging is the expected new norm, the custom styles should be appended.

I will add, it also makes it harder to debug custom styles, because you can’t easily tell what’s coming from where.