classes styles don't override default when using @emotion/react

[garronej]

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

Best,

[majelbstoat]

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.

[mnajdova]

In order for emotion to consider how the classes should be appended in the head, it requires you to use the cx in order to merge the classes. Now this comes from the context and it is emotion specific API (it doesn't exist in the context of styled-components) so I am not sure that it could work in a generic manner.

I've played a bit with the sandbox and I saw that the className prop work as expected - https://stackblitz.com/edit/tss-react-vnqand?file=index.tsx (probably emotion handles this by default).

Anyway, I don't think we can do much with how the classes are appended (note that they can be generated in a different manner - maybe with static css or with any other css in jss solution), so it is up to the developers to ensure that those will be injected after the default styles coming from Material-UI.

We can correctly predict how the styles will be merged if some of the internal APIs are used (sx or styled).

---

Anyway, I will spend some more time on this tomorrow to see if we can have some quick win maybe. I am open for ideas of how we can improve this.

[garronej]

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.

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

[mnajdova]

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.

[mnajdova]

I've spent some more time on this. I don't think it's worth solving. The reasons are the following:

• we need to read a new context, that is only emotion specific
• we need to add new API (cx) which is again only emotion specific
• seem like too much of a corner case, and there is already an alternative of how it can be solved (using className)

I will leave the issue open, to see if other developers will have the problem.

[garronej]

Hi @mnajdova,
I did some thinking and I think I owe to @janus-reith who spent some time contributing to tss-react to better represent the argument in favor of solving this.

CSS injection priority is not that easy to configure especially when SSR is involved. In the current state of things, to get everything working as expected, users have to:

• Tell MUI to use a custom cache with "prepend": true
• Use another cache without "prepend": true for their custom style.
• If SSR is required, inject the styles bound to the mui cache first and then inject the styles bound to the cache used for custom style.
  It would be much easier for users to just use the same emotion cache for their custom style than the one used internally by mui and let you guys handle the priorities. It would also be much easier to document how to configure SSR (see, it is less straightforward than it used to).

That said the three points you made are valid as well. It's a tough call, I'm glad I am not the one having to make it. 😁

[garronej]

Also for user that would like to use @emotion/react for their custom style, there is no way for them to use a dedicated cache.

• If they use <CacheProvider /> from @emotion/react, mui will pick up the cache provided.
• If they use the <StyledEngineProvider injectFirst> their custom style will pickup the cache mui internally uses.

I think, if you chose not to address this issue, you'll have to state in the documentation that it isn't possible to use @emotion/react for custom styles because there is currently no way for users to make it work.

[mnajdova]

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.

[garronej]

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.

Forgive me🙏, I am quite embarrassed, this damage my credibility quite a bit, as I was preparing the sandbox I realised that your are in-fact right. It's something that was reported to me by one user, I assumed it was true. I should have tested it before submitting it here.

The other points I made still stands though.

[mnajdova]

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 :)

[mnajdova]

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

[garronej]

@mnajdova, that's fine with me. I updated the issue so it can be easily found by people that would be affected.

Best regards

[garronej]

Hi @mnajdova ,
Allow me to up this issue.
Two points that I want to stress:

1. As detailed in #31825 this prevent from fully supporting @emotion/react as a solution for writing custom style.
2. I did find a workaround for TSS but it involves a lot of voodoo and requires users to explicitly provide an emotion cache to be able to use TSS. I hope there was a way for me to make TSS work out of the box with MUI, this issue is what's blocking it.
   It's a real problem for libraries that uses tss-react internally like mui-datatable. They have to instruct their users to explicitly provide an emotion cache as well.
   Example of a confused user.

I understand that this is a very hard to address issue but I also think it is pretty critical.

Best regards and congratulations for being able to hire again. MUI is a true success story.

[mnajdova]

Thanks for the bump @garronej I am adding it to my list for next week.

[garronej]

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,

[mnajdova]

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.

[garronej]

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?

[mnajdova]

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

[garronej]

Addressed in #32067