[Themes] Drop support for multiple themes per component tree

[alitaheri]

Having more than one theme object on the context was implicitly supported, now we wish to explicitly not support it. That means, if we do that providing multiple themes in a single component tree will break things.

We had long discussions about how we can improve our current styling approach. And after chatting a lot with @nathanmarks we came to the conclusion that we should have only one theme object anywhere on the context.

Why?

Good question, with the styling approach that we are going to implement for our components (to improve performance, readable class names, css features in js-styles, etc...) we cannot have more than one theme object because if we do, the code, the class names, and a lot of things will get way too complex, which will again hurt performance, maintainability and readability of the code. So we decided to enforce this rule to simplify the code, optimize it even more and support SSR out of box as we do already.

Besides... that's bad UX and the specs don't mention it anywhere.

How will my app be affected?

If you have any kind of component tree that somehow looks like this:

<MuiThemeProvider muiTheme={theme1}>
  <App>
    // somewhere down the tree
    <MuiThemeProvider muiTheme={theme2}>
      <MoreStuff/>
    </MuiThemeProvider>
  </App>
</MuiThemeProvider>

You'll need to work around it as this change will break the expected behavior.

Does this mean I can't change theme after the initial render?

Not at all! You can still recalculate the theme and pass down a new theme as you could previously. Nothing's changing there, but you can't have a subtree deep down using another theme object.

More technical insight?

Sure, we plan on using jss to convert our js-styles into css in runtime. This will give us all the benefits of css ( media queries, fast :hover, sibling selectors, etc) and will make overriding styles using css a LOT easier ( not dropping support for inline-styles though, ever! ). However, to make it understand our themes and fully support SSR we decided to write a small styleManager ( @nathanmarks came up with that, it's a brilliant idea ) that manages our <style> tags and outputted css. To output deterministic css, we can't have more than one theme. if we do, we should track all the themes, not saying it's impossible, but it's very complex, any solution is complex in it's own way. And worst of all, that complexity, will again, hurt performance of each render ( or mount ). But if we drop support for multiple theme objects in one component tree, everything will become extremely simplified, extremely fast and very easy to maintain.

Why open an issue?

We've decided to open this issue to see how the community will react, it's very rare that any application would do this. But we'd rather get some community feedback on this before moving forward. this issue will probably be opened for a couple of days before we move forward with it.

[nathanmarks]

@alitaheri thanks for writing this out!

With the updated take on theming/styling, only having 1 theme active at a given time would significantly reduce the complexity of the implementation meaning less maintenance, easier performance optimization and a much lower chance of bugs.

In the vast majority of real world use cases, theme changing happens (if at all) at a high level and not a mix-n-match in different sections of a single page -- that's for css/inline styles/js styles.

I think this is a beneficial move considering the benefits 👍

[estaub]

Consider figuring out some means of support for the ubiquitous desktop inverse-video sidebar and header, where a "theme" is used that is deliberately opposed to the main theme. They often have quite a bit of complexity, so being able to sub-theme them would be a big win. With CSS, one would make contextual class declarations to handle the color reversal, but AFAICT this is difficult to accomplish without a lot of hand-tooling using Material-UI. For instance, <ListItem> refers directly to the theme for its coloring, and must be wrapped (or repeatedly styled) for this kind of usage.

I'm only suggesting that there needs to be a good, documented answer for this kind of case, not that it necessarily has to be nested <MuiProvider>s - though I must say that it's the obvious way to handle it.

[estaub]

To output deterministic css, we can't have more than one theme.

Sorry, I don't follow.

Consider using a solution similar to Radium's Style component, whereby each theme would bind to some set of contexts using a scopeSelector.

[amannn]

I have the same use case as @estaub where I've got a dark sidebar and a light main content.

Would it be acceptable that the provider components are siblings instead of nested?

Currently I have something like this:

<div>
  <MuiThemeProvider muiTheme={theme.dark}>
    <Drawer open>
      {sidebar || <NavSidebar />}
    </Drawer>
  </MuiThemeProvider>

  <MuiThemeProvider muiTheme={theme.light}>
    <div>
      {main}
    </div>
  </MuiThemeProvider>
</div>

This works well with the current version.

[oliviertassinari]

I'm closing as we have indeed dropped the support for multiple themes per component tree. However, we now have an issue to bring it back to the v1-alpha branch as that feature has proven to be useful #6129.