[next] Suggestion: split SSR configuration apart from theme configuration. Use react-jss

[jcheroske]

I'm in the process of getting MUI@next to work with next.js, and I just wanted to pass on my experience so far. Even though MUI uses JSS, it feels like MUI is getting involved in a lot of areas that are already handled by react-jss MUI now has its own SSR configuration, separate from jss and react-jss, and this, to me, feels like a possible mistake. Personally, I wish MUI just deferred SSR configuration to the JSS suite. If it did that, then a there would be a single point of SSR configuration for all use of JSS within an app, rather than separate MUI and non-MUI configurations. Also, react-jss now supports the the new JSS function properties, automatically generating both a static and dynamic stylesheet for the wrapped react component, and passing in the component props to all of the function properties. It seems like a decision was made to not use that library, and instead have a combination of jss-theme-reactor and MuiThemeProvider take over those responsibilities. This means that, as react-jss evolves, MUI will have to duplicate features to stay current.

I realize that a shortcoming with react-jss has been that it can only take in an object when injecting a stylesheet, when what we really want is for it to take in a function like theme => stylesheet, and this is probably the main perk of theme reactor. Now, with function properties, we can do:

injectSheet({
  backgroundColor: ({theme}) => theme.backgroundColor,
  color: ({theme}) => theme.color
})

It's more verbose, but it works. All that would be necessary would be to wrap the react-jss injectSheet HOC like:

export const withStyles = styleSheet => compose(
  withTheme,
  injectSheet(styleSheet)
)

If all of the MUI components then used that HOC, SSR config could be done just with the react-jss StyleRegistryProvider

If using function properties for theme-related styles is too much overhead, then perhaps the withStyles HOC could be reworked to both take in a theme => stylesheet function, populate an object with the theme data, and then delegate to injectSheet for the actual stylesheet injection. That way we would have the best of all worlds: a choice between static or dynamic theme-based styles.

Just my two cents. Thanks for the amazing library.

Jay

[oliviertassinari]

Hey, thanks for sharing those though. If I understand correctly this issue is deeply around making jss-theme-reactor integrate as much as possible with the existing react-jss project so users like you get a simplified API and maintainers like me get more synergy with the ecosystem.

I'm not I'm the best one to answer that issue, @kof and @nathanmarks are 👼 . But I'm gonna give it a shot.

I see three things that jss-theme-reactor is proving over react-jss and the function properties.

1. Hard-coded specificity, we have a fine grain control over the injection order of the stylesheets in the DOM.
2. The overrides property, it's documented here, it's really powerful!
3. The styles of the stylesheets is computed once, not at every render like it would be with function properties. I don't have any benchmark but I would expect it to be faster. How much faster? Does it make a different? I have no idea.

[kof]

I am using mui next with react-jss together, though I don't have SSR.

I realize that a shortcoming with react-jss has been that it can only take in an object when injecting a stylesheet, when what we really want is for it to take in a function like theme => stylesheet,

Yeah, we will have that in react-jss soon as well.

I don't think you have to worry in general about jss-theme-reactor, its an internal solution for mui to make the best possible fit the requirements.

For SSR though I agree we should have only one interface even if they are both used.

[kof]

Btw. function values for static theming is not an ideal fit, they are a better fit for situations where you would normally use inline styles, for dynamic theming or dynamic overrides.

[jcheroske]

@oliviertassinari @kof I've dug around in the code a bit more, and had a chance to sleep on it. My understanding of the situation has clarified somewhat.

I now think the primary issue is simply that MUI has its own SSR config, and that it's not easily comprehensible. Handing over SSR to react-jss would accomplish two important things:

• Centralize SSR config
• Get MuiThemeProvider out of the business of SSR

The rest of it then becomes merely a matter of preference. If I want function properties, I can use react-jss. If I want the MUI theme, I can use createStyleSheet. It will all just work.

There is one lurking issue though, and that's the jss-preset configuration. Currently, MuiThemeProvider initializes jss using jss-preset-default with no way to override. To allow for more explicit jss configuration, and to aid comprehension, perhaps another component needs to be introduced. Something called StyleConfigurator or JssConfigurator that handles the jss config and makes it distinct from MuiThemeProvider's job providing the theme. If that component was missing, then MUI could fall back to defaults. This, to me, would read nicely:

import jssPresetDefault from 'jss-preset-default'
import {MuiThemeProvider, JssConfigurator} from 'material-ui/styles'
import myJssPlugin from 'somewhere'
import theme from 'somewhereElse'

const jssPlugins = jssPresetDefault();
jssPlugins.push( myJssPlugin() );

export default () => (
  <JssConfigurator plugins={jssPlugins} >
    <MuiThemeProvider theme={theme} >
      <Root />
    </MuiThemeProvider>
  </JssConfigurator>
)

[oliviertassinari]

MuiThemeProvider initializes jss using jss-preset-default with no way to override.

Yes, you can. Simply provide your own styleManager to this component. It's not directly documented here but we could improve it.

[jcheroske]

I wanted to give some more feedback on SSR pain with MUI@next, this time using Apollo with next.js.

Over the last two weeks I've managed to get SSR working with the following stack: next.js, redux, Apollo, JSS, MUI 0.18. I've uncovered a JSS bug, and need a pull request to land on next.js, but it works without error otherwise. Transfer of styles and dehydration/rehydration of redux state, which includes the Apollo query cache, are working perfectly.

The major problem is that, when I tell next.js to do a production build, it blows up with bizarre errors. If I remove MUI from the project, the build executes without issue. So I thought I'd try another attempt at getting MUI@next working, in hopes that the more recent version wouldn't have the same build issue. This has been a total failure for completely different reasons.

Again, the crux of the issue is that MUI@next has conflated SSR config, theme config, and just the ability to successfully render, all within the <MuiThemeProvider>. Compare it to JSS:

• SheetsRegistryProvider component that exists just for SSR
• If above component is missing on the client, system still functions properly.

Whereas MUI:

• Explodes when MuiThemeProvider is missing.
• Forces configuration of theming and SSR concerns in the same place, even though those concerns have nothing to do with one another. In next.js, you configure SSR in _document, which is a server-only file. MUI@next requires duplicate and very brittle config to function properly on both the server and client.

JSS already has an SSR style registry, available at context.jssSheetsRegistry. If MUI were to use that, I think all of these problems would be solved.

[oliviertassinari]

@jcheroske Thanks a lot for that feedback! I was considering using the same stack than you on a new project, I'm glad to hear it's working out for you.

Explodes when MuiThemeProvider is missing.

That might go away with the @nathanmarks effort on improving the theme handling, also there is a new theme effort at the jss level that we could take advantage of. Right now, removing that requirement, introduce too much boilerplate in the implementation. We though it was simpler that way. Still, we could migrate all the code base to use a Higher-order Component with that concern. Actually, I want to make all the components takes a classes property so we could extend the concern with that. Additional, I might need a style-less version of the components for this new project.

Forces configuration of theming and SSR concerns in the same place

How is that? They can be defined independently.

Thanks for the tips regarding improving the situation

[jcheroske]

How is that? They can be defined independently.

At the point the StyleManager is created, the theme needs to be passed in.

I gave this another go, and I have succeeded in getting it up and running. But it required too much cleverness, and I stand by my earlier statements that the sheet registry should not be tightly coupled to the theme.

In order to preserve as much isomorphic code, and to minimize duplication, I resorted to child->parent communication. The SSR config component passes a callback function through the context that takes the style manager. The child component tasked with configuring MUI creates the style manager, and then calls the callback. Simple, but not standard react thinking.

The SSR config for both JSS and MUI@next are below.

_document.js

import {StyleManagerRetriever} from 'material-ui-extras'
import Document, { Head, Main, NextScript } from 'next/document'
import {SheetsRegistry, SheetsRegistryProvider} from 'react-jss'

export default class EnhancedDocument extends Document {
  static async getInitialProps ({renderPage}) {
    const sheets = new SheetsRegistry({resetClassNameCounter: true})

    let styleManager
    const setStyleManager = sm => (styleManager = sm)

    const decoratePage = Page => props => {
      console.log('[_document] Page decorator')

      return (
        <SheetsRegistryProvider registry={sheets}>
          <StyleManagerRetriever setStyleManager={setStyleManager}>
            <Page {...this.props} />
          </StyleManagerRetriever>
        </SheetsRegistryProvider>
      )
    }

    // !!! Passing an HOC to renderPage won't work until a PR drops.
    // SSR config with next.js is severely limited until that happens.
    // Multiple competing PRs are trying to fix the issue.
    const renderedPage = renderPage(decoratePage)

    const styles = (
      <style id='jss-ssr-styles' type='text/css'>
        {sheets.toString() + '\n'}
        {styleManager.sheetsToString()}
      </style>
    )
    const props = {
      ...renderedPage,
      styles
    }
    return props
  }

  render () {
    return (
      <html>
        <Head>
          <link rel='stylesheet' href='https://fonts.googleapis.com/css?family=Roboto:300,400,500' />
        </Head>
        <body>
          <Main />
          <NextScript />
        </body>
      </html>
    )
  }
}

StyleManagerRetriever.js

import PropTypes from 'prop-types'
import {Component} from 'react'

class StyleManagerRetriever extends Component {
  static propTypes = {
    setStyleManager: PropTypes.func.isRequired,
    children: PropTypes.element.isRequired
  }

  static childContextTypes = {
    setStyleManager: PropTypes.func.isRequired
  }

  getChildContext () {
    return {
      setStyleManager: this.props.setStyleManager
    }
  }

  render () {
    return this.props.children
  }
}

export default StyleManagerRetriever

material-ui.js (configuration HOC)

import {MuiThemeProvider} from 'material-ui/styles'
import {defaultGetInitialProps} from 'next-extras'
import PropTypes from 'prop-types'
import {Component} from 'react'
import {createEagerFactory} from 'recompose'
import theme from 'theme'

export default () => BaseComponent => {
  const factory = createEagerFactory(BaseComponent)

  class MuiProvider extends Component {
    static contextTypes = {
      setStyleManager: PropTypes.func
    }

    static getInitialProps = defaultGetInitialProps(BaseComponent)

    render () {
      const {styleManager} = MuiThemeProvider.createDefaultContext({theme})

      const {setStyleManager} = this.context
      if (setStyleManager) {
        setStyleManager(styleManager)
      }

      return (
        <MuiThemeProvider styleManager={styleManager} theme={theme}>
          {factory(this.props)}
        </MuiThemeProvider>
      )
    }
  }

  return MuiProvider
}

[oliviertassinari]

We have updated the styling solution, we have reduced the gap with react-jss. For SSR, we rely on the JssProvider of react-jss. I have updated the documentation, it's not live yet, only in the source code.