styleManager.sheetsToString() returns empty

[brianfeister]

This issue is quite a seriously tangled mess, so I'm opening it here as well as the already open issue over on callemall/material-ui because it seems to trace back to the jss-theme-reactor library.

https://github.com/callemall/material-ui/issues/6740

Given the current material-ui docs here on handling server-rendering, the following code returns a styleManager and theme but calling styleManager.sheetsToString() yields an empty string value

function createStyleManager() {
  return MuiThemeProvider.createDefaultContext({
    theme: createMuiTheme({
      palette: createPalette({
        primary: green,
        accent: red,
        type: 'light',
      }),
    }),
  });
}

I'm going to need some help understanding the intended behavior for this. Looking through the dist/material-ui.js file used via node_modules in my app, I find that the reason it returns empty is because it iterates over an empty styleMap array via the following function, which originates with the nathanmarks/jss-theme-reactor library:

function sheetsToString() {
    return sheetMap.sort(function (a, b) {
      if (a.jssStyleSheet.options.index < b.jssStyleSheet.options.index) {
        return -1;
      }
      if (a.jssStyleSheet.options.index > b.jssStyleSheet.options.index) {
        return 1;
      }
      return 0;
    }).map(function (sheet) {
      return sheet.jssStyleSheet.toString();
    }).join('\n');
  }

Digging through the source, I find that over here, the method createDefaultContext

  static createDefaultContext(props = {}) {
    const theme = props.theme || createMuiTheme();
    const styleManager = props.styleManager || createStyleManager({
      theme,
      jss: create(jssPreset()),
    });

    if (!styleManager.sheetOrder) {
      styleManager.setSheetOrder(MUI_SHEET_ORDER);
    }

    return { theme, styleManager };
  }

looks to createStyleManager, a method imported from a jss-theme-reactor external library?

That seems a strange thing, to pull in an external dependency for managing material themes and palettes, but I'm sure someone else knows the story better than I do.

Anyway, I'm happy to submit a PR, but I can't tell, seems that the issue is in jss-theme-reactor?

In the syleManager constructor a function updateTheme() is called and sets the new theme and then calls rerender(). The problem with rerender is that it calls it depends on sheetMap to determine which sheets should output styles, and we're now at the end of the styleManager constructor phase and we still haven't seen any function calls which would actually set a value to this empty array.

[brianfeister]

/cc @oliviertassinari

[oliviertassinari]

Are you providing the styleManager object to the MuiThemeProvider component? Otherwise, it's gonna create a second styleManager, and the one you have access to will definitely be empty.

Also, I have noticed that we have a naming inconsistency:

function createStyleManager() {
  return MuiThemeProvider.createDefaultContext({

We should normalize createStyleManager vs createDefaultContext.

[nathanmarks]

@brianfeister cc @oliviertassinari

Related... I'm also thinking about this: https://github.com/callemall/material-ui/issues/6129

What do you think to that issue? What's the story for server side rendering gonna look like if there are nested theme providers that we need to pull the style manager out of? The current design is not really fully suited to the use case presented in that issue. With the sort of themes this was designed for (big, global... like material design) I wasn't considering having ThemeProviders scattered about throughout the rendering tree.. more of a single theme provider at the root of your app and a single style manager to manage the sheets. Is the use case a signal that there needs to be another override mechanism for theme props in the rendering tree? Or a better way to create variations outside of the main theme? I'm torn on the best approach here. cc @kof

[oliviertassinari]

@nathanmarks I thought that the point of nesting ThemeProviders was regarding changing the theme not the styleManager. Also, I'm wondering if CSS variables is another technical way to solve the same issue.

[kof]

css variables are not implemented in all browsers, you are unlikely to be able to rely on them soon inside of mui

[brianfeister]

@oliviertassinari I am providing styleManager to the MultiThemeProvider as follows:

class DemoApp extends Component {

  render() {

    function createStyleManager() {
      return MuiThemeProvider.createDefaultContext({
        theme: createMuiTheme({
          palette: createPalette({
            primary: green,
            accent: red,
            type: 'light',
          }),
        }),
      });
    }
    // Create a styleManager instance.
    const { theme, styleManager } = createStyleManager();
    const css = styleManager.sheetsToString()
    console.log('styleManager', JSON.stringify(styleManager.sheetsToString())) // empty

    return (
        <MuiThemeProvider styleManager={styleManager} theme={theme}>
[... etc]

[oliviertassinari]

console.log('styleManager', JSON.stringify(styleManager.sheetsToString())) // empty

That's expected. There is no magic at stake here ✨ . We can't know the styles until the elements tree has been traversed by React.

[brianfeister]

Gotcha, can you explain how I could debug this, because it's a black box to me and it's not working so I'm a bit lost.

[oliviertassinari]

Still, why would you want to get the CSS generated on the client? I'm not sure what's your actual issue is behind.

[brianfeister]

I'm trying to follow the setup instructions documented here, which says to generate the CSS on the client: https://github.com/callemall/material-ui/blob/next/docs/src/pages/getting-started/server-rendering.md#the-client-side

function createStyleManager() {
  return MuiThemeProvider.createDefaultContext({
    theme: createMuiTheme({
      palette: createPalette({
        primary: green,
        accent: red,
        type: 'light',
      }),
    }),
  });
}

class Main extends Component {
  // Remove the server-side injected CSS.
  componentDidMount() {
    const jssStyles = document.getElementById('jss-server-side');
    if (jssStyles && jssStyles.parentNode) {
      jssStyles.parentNode.removeChild(jssStyles);
    }
  }

  render() {
    return <App {...this.props} />
  }
}

// Create a styleManager instance.
const { styleManager, theme } = createStyleManager();

render(
  <MuiThemeProvider styleManager={styleManager} theme={theme}>
    <Main />
  </Provider>,
  document.querySelector('#root'),
);

[brianfeister]

Am I just mis-reading this? Because if the explanation is meant to say that I CHOOSE between rendering CSS client or server side, then that's not what I see when reading it.

[oliviertassinari]

Sorry, I don't understand what's not working out for you :/. What do you mean by choosing? You have the CSS injected in the client side no matter what you do. If you have any issue, I would expect it to be on the server side, not the client one.

[brianfeister]

Let me clarify a bit, I should have explained that I'm trying to reduce boilerplate by including the same component on both the client and server DemoApp per below, in <Helment> I'm injecting a <style> tag which I expect to be empty if it's running client side. I realize this could be the wrong approach, just trying to understand why.

class DemoApp extends Component {

  // Remove the server-side injected CSS.
  componentDidMount() {
    const jssStyles = document.getElementById('jss-server-side');
    if (jssStyles && jssStyles.parentNode) {
      jssStyles.parentNode.removeChild(jssStyles);
    }
  }

  render() {

    function createStyleManager() {
      return MuiThemeProvider.createDefaultContext({
        theme: createMuiTheme({
          palette: createPalette({
            primary: green,
            accent: red,
            type: 'light',
          }),
        }),
      });
    }
    // Create a styleManager instance.
    const { theme, styleManager } = createStyleManager();
    const css = styleManager.sheetsToString()

    return (
        <MuiThemeProvider styleManager={styleManager} theme={theme}>
          <Helmet>
            <style id="jss-server-side">{css}</style> 
          </Helment>
[... etc]

[oliviertassinari]

Oh, I understand now. Sorry, I don't think that there is any better way that what's documented. That section of the helmet documentation is a good example, they have to do the same thing we do with the theme. Maybe you could push the style into their object to reduce a bit the boilerplate

[nathanmarks]

@oliviertassinari style manager is only capable of handling a single theme, that's the problem with that.

[brianfeister]

Talk about "trial by fire", this is my second day using React and this was definitely not easy to figure out.

Leaving the commit here that shows the (non-trivial) changes I made to the react-universally boilerplate to enable server-side rendering in their boilerplate which has alot of great features and alot of moving parts:

https://github.com/brianfeister/react-universally/commit/2e80248a2c46b68fe38fa7c022dad33fd07a84de

[oliviertassinari]

@brianfeister We are glad you figured a solution out 😄 .