[styles] Theme Provider is not working correctly when nested themes are used

[naman13924]

Earlier themes were working fine before the 4.0 release but now the nested themes are overriding the themes which are outside its scope
for example in your documentation example https://codesandbox.io/s/material-demo-hdy1n

replace the demo.js code with the attached code in demo.txt file
demo.txt

The theme of components outside the nested themes are getting overridden by the nested one.
I am facing this challenge in my whole project as I have many nested themes and the ui has changed unexpectedly due to this,
earlier it was working fine with v3.9

[oliviertassinari]

@naman13924 You need a top-level theme provider: https://codesandbox.io/s/material-demo-29fxp.

[eps1lon]

So basically multiple theme roots are not allowed on a page. There has to be a single provider root. Wouldn't that make multiple react trees impossible on a page?

[oliviertassinari]

@eps1lon I have demonstrated the simplest workaround in the codesandbox: one ThemeProvider at the top of the tree.
For the multiple React tree pattern, people have to use a different seed if they want to use different themes: https://material-ui.com/styles/api/#creategenerateclassname-options-class-name-generator.
Another workaround is to use the disableGlobal option.

[eps1lon]

@eps1lon I have demonstrated the simplest workaround in the codesandbox: one ThemeProvider at the top of the tree.

Pretty sure that's what I said

So basically multiple theme roots are not allowed on a page. There has to be a single provider root.

For the multiple React tree pattern, people have to use a different seed if they want to use different themes

I would guess this is a pretty basic use case when using multiple react roots. That shouldn't be hidden behind nested API docs.

[oliviertassinari]

@eps1lon Yes, I have repeated it for people that might have the problem and that skip read. It's a really important point. I do agree that there is close to no documentation it about and that we should address the problem. I wasn't sure it would impact many people. I suspect few people have this use case, but I don't have enough data to defend this claim. I will work on it as more people report it (hence gain priority over the other issues)

[Ajaay]

Am I right in thinking that this is what has broken support of our v0.X theme nesting?

[ztoben]

Also wondering that @Ajaay. We have a v0 theme nested in a now v4 theme and are running into a ton of issues that seem to be theme related.

[oliviertassinari]

@ztoben It's still supported (but not encouraged). Do you have a minimale reproduction?

[Ajaay]

If it makes any difference I didn’t have problems with v4.0.0. Only when I updated to v4.0.1 did they start Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Olivier Tassinari <notifications@github.com> Sent: Wednesday, May 29, 2019 2:44:15 PM To: mui-org/material-ui Cc: Ajaay; Mention Subject: Re: [mui-org/material-ui] Theme Provider is not working correctly when nested themes are used (#15914) @ztoben<https://github.com/ztoben> It's still supported. Do you have a minimale reproduction? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#15914>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABO3LP66YW7LHLRA6I4BG3TPX2CC7ANCNFSM4HQCI2HQ>.

[ztoben]

@oliviertassinari, it's in a private repo that's pretty complex. We're working on getting a reproduction case set up.

[nareshbhatia]

@oliviertassinari, @eps1lon - can you please clarify what you mean by "Multiple theme roots are not allowed on a page. There has to be a single provider root"? In my sandbox example, I have a single provider root in index.tsx and a nested provider in App.tsx. However the nested theme is not working. The intent is to achieve a dark theme in the left sidebar whereas the rest of the app should be light themed. What am I missing?

[eps1lon]

With root I mean a provider which has no parent provider:

<React.Fragment>
	<ThemeProvider /> /* root */
	<ThemeProvider /> /* root */
	<ThemeProvider /> /* root */
</React.Fragment>

<React.Fragment>
	<ThemeProvider> /* root */
		<ThemeProvider /> /* child */
		<ThemeProvider /> /* child */
	</ThemeProvider>
</React.Fragment>

Edit:
The issue with dark mode in nested themes is that something like a dark mode requires global styles. You should only have one theme changing the dark/light mode per <body />.

[oliviertassinari]

The intent is to achieve a dark theme in the left sidebar whereas the rest of the app should be light themed.

@eps1lon is right, in order to achieve this, you need a light theme provider that wraps your whole tree, a dark theme provider that wraps the side nav and a <Typography component="div"> that applies the right color property (as we rely on color inheritance when possible).

[nareshbhatia]

Thanks for your responses @eps1lon & @oliviertassinari. Now I am clear that in my example, I have one light theme provider at the root of the page and one dark theme provider as a child that wraps the sidebar - which is essentially a List. So far so good.

The issue is, as @eps1lon mentioned, the dark theme relies on global styles. The List component does not even attempt to paint its background dark. It relies on the body background which unfortunately happens to be white. Similarly the text in the list is black because body color is black and ButtonBase sets the color to inherit.

Given this approach, I don't think nested themes will automatically switch between light and dark themes. However, I came up with a simple trick based on @oliviertassinari's hint that seems to work. I have introduced a <Paper> instance right below the nested theme. I also changed palette.background.paper to #303030 to get the correct dark background. This seems to work - the List background is dark and the text color is white. Here's the result. Please let me know if you find any holes with this approach.

Thanks again for your help.

[oliviertassinari]

Looks good to me. If your side nav stays as simple, I would try not to nest two themes.

[nareshbhatia]

@oliviertassinari, the side nav will stay that simple. I was relying on <Paper> to pick up the right colors from the nested theme. How do you propose doing it without a nested theme? Perhaps a simple <div> that changes background-color and color? Will that do the trick? Or did you have something else in mind?

[oliviertassinari]

You could apply the dark background color and color present in the theme. So yes, with a simple div.

[nareshbhatia]

Just tried a simple div with dark background and white color - it does not work nicely. Here's an example. While the list text works because it uses color="inherit", the icons and toggle buttons don't because they pickup their color from the parent theme. Based on this, I think the simplest solution is to have a nested theme. Thoughts?

[oliviertassinari]

Makes sense.

[pkhodaveissi]

@pkhodaveissi I think that your concern is different and outside of the scope of this issue. We didn't optimize for micro-frontend, nor do we aim to, but are still interested in finding improvement opportunities.

Sure I understand, but guess my question boils down to a solution to reduce the number of <style> tags introduced to the document while you're being forced to have multiple siblings of ThemeProvider element around(cuz of the architecture).

[oliviertassinari]

An update, this issue is being resolved in v5 thanks to #22342. So far, we have migrated the Slider in the lab, where this can be tested. It no longer generates global class names, class names are hashed. However, the components have global class names that can be targeted.

[Zache]

I'm affected by this issue, we have multiple react roots on the page and because of this the classnames that are generated aren't deterministic, causing components to re-render unnecessarily.

[ahayes91]

@eps1lon I have demonstrated the simplest workaround in the codesandbox: one ThemeProvider at the top of the tree.
For the multiple React tree pattern, people have to use a different seed if they want to use different themes: https://material-ui.com/styles/api/#creategenerateclassname-options-class-name-generator.
Another workaround is to use the disableGlobal option.

Ditto on this issue - I'm working on a single-spa app as @pkhodaveissi mentioned, and having multiple React apps on the page that each use a MuiThemeProvider component with the same theme in their tree are causing conflicting stylesheets and unexpected behaviour in our components.

(Note: This is only happening on pages with separately webpack-aged apps, if you know what I mean.)

I confirmed that upgrading to MUI 5 would indeed resolve the issue for us, but seeing as MUI 5 isn't ready yet and we have quite a lot of migration tasks to do from 4 -> 5, I'm looking for an interim solution in the meantime.
@oliviertassinari what solution would you recommend in this case, something like the following in each app on the page?

const generateClassName = createGenerateClassName({
  disableGlobal: true, // or seed: 'something_unique' ?
});
...
const App = () => {
...
return (
    <StylesProvider generateClassName={generateClassName}>
        <MuiThemeProvider theme={theme}>
        ....
        </MuiThemeProvider>
    </StylesProvider>
);
};

I couldn't quite figure out if StylesProvider is meant to replace or supplement MuiThemeProvider from the documentation.
Thanks!

[oliviertassinari]

@ahayes91 If you have multiple react roots, disableGlobal isn't enough. You need to have a unique seed for each class name generators.

I'm closing as this is fixed in v5.

[Johnrobmiller]

To be brutally honest, details about the ThemeProvider's technology should not even matter here, not even a little bit, or at least from the user's point of view. Concerns about a feature's design are almost always of greater significance than the technology that enables said design. And with this said, regarding the fact that multiple ThemeProviders must have a parent ThemeProvider element, there is an unasked yet obvious question that warrants asking: where is the user expected to learn about this somewhat esoteric yet potentially code-breaking ThemeProvider fact? In the issues section of the mui Github? No. tldr: this should not be a design feature to begin with, but since it is and hasn't been changed yet, the docs should be extremely clear about it, which they aren't.

[oliviertassinari]

@Johnrobmiller It's a v4 only issue, a version we don't actively support anymore.

This issue is gone in v5. We gave up on styling with global class names. This structural change harms the DX of customizations but yields other interesting features that make it a significant net positive.

[sag1v]

@oliviertassinari Sorry to bring this up again but we are in a middle of a migration from v4 to v5 and i would like to minimize the amount of research and surprises i'm going to face.
the seed solution for nested Themes in v4 wasn't 100% complete. The issue we had with this is that the seed wasn't chained to "nested MUI classes"

For example, given this style:

root: {
  '& .MuiBadge-badge': {
    // something
  }
}

The seed won't be chained to the MuiBadge-badge class name, therefore this selector will impact badges that are not part of this seed (scope) instead of targeting the actual badge we want.
We had to create a wrapper over makeStyles that goes through all keys (recursively) and chain the seed to all MUI-* classes.

The reason i'm bringing this is up, is to ask if the solution that was introduced in v5 bypass or address this issue in any way?
Otherwise, we will need to either fix this or at least have a way to workaround it in user-land (as we managed to do in v4).