Material-ui: Box component classname id appears to not be deterministic, breaks SSR

[pheuter]

When I render the following snippet, it loads just fine on the initial page load from the server, but once the client code loads I see a className did not match error and the correct styling disappears.

import { unstable_Box as Box } from '@material-ui/core/Box';

<Box clone ml={1} fontWeight="bold">
  <Typography variant="subtitle1">Test</Typography>
</Box>

Might be worth noting that I'm also seeing the following TypeScript error:

And this is the className warning:

> Warning: Prop `className` did not match. Server: "MuiTypography-root-1mdmxcj MuiTypography-subtitle1-i5il2s MuiBox-root-131eauo MuiBox-root-1" Client: "MuiTypography-root-1mdmxcj MuiTypography-subtitle1-i5il2s MuiBox-root-1cjflis MuiBox-root-1"

Theory

It looks like the Box classname id generation might not be consistent/deterministic? All of my other @material-ui/core components appear to handle SSR just fine and consistently load the same className ids.

---

Reproduction

https://codesandbox.io/s/6y1wrn8p9z

thanks @weslenng

---

Environment 🌎

"@material-ui/core": "4.0.0-alpha.2",
"@material-ui/icons": "4.0.0-alpha.1",
"@material-ui/styles": "4.0.0-alpha.2",
"react": "16.8.4",
"react-dom": "16.8.4",
"jss": "next"

[oliviertassinari]

@pheuter Do you have a reproduction?

[pheuter]

What’s the best way to make an SSR reproduction?

On Sat, Mar 9, 2019 at 12:19 PM Olivier Tassinari [e-mail]
wrote:

@pheuter https://github.com/pheuter Do you have a reproduction?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mui-org/material-ui/issues/14808#issuecomment-471202470,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFqGtOSZ8OlGtHOR-Q6yUkZ4sD3_lLcks5vU-00gaJpZM4bmygA
.

[oliviertassinari]

@pheuter I would start from https://github.com/mui-org/material-ui/tree/next/examples/nextjs-next.

[pheuter]

Thanks, will give it a shot.

On Sat, Mar 9, 2019 at 12:30 PM Olivier Tassinari [e-mail]
wrote:

@pheuter https://github.com/pheuter I would start from
https://github.com/mui-org/material-ui/tree/next/examples/nextjs-next.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mui-org/material-ui/issues/14808#issuecomment-471203635,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFqGhcLqOcXQk5ajnKMWcesYdFrHiz1ks5vU-_MgaJpZM4bmygA
.

[weslenng]

I am having the same issue,
Here is a reproduction of the error:

https://codesandbox.io/s/6y1wrn8p9z

[oliviertassinari]

Ok, I have figured out the source of the problem. The bundle contains two versions of the styles package:

The ugly workaround that confirm my previous point:

--- a/examples/nextjs-next/pages/_app.js
+++ b/examples/nextjs-next/pages/_app.js
@@ -4,6 +4,8 @@ import React from 'react';
 import App, { Container } from 'next/app';
 import Head from 'next/head';
 import { StylesProvider, ThemeProvider } from '@material-ui/styles';
+import ThemeProvider2 from '@material-ui/styles/ThemeProvider';
+import StylesProvider2 from '@material-ui/styles/StylesProvider';
 import CssBaseline from '@material-ui/core/CssBaseline';
 import getPageContext from '../src/getPageContext';

@@ -33,16 +35,24 @@ class MyApp extends App {
           generateClassName={this.pageContext.generateClassName}
           sheetsRegistry={this.pageContext.sheetsRegistry}
           sheetsManager={this.pageContext.sheetsManager}
+        >
+        <StylesProvider2
+          generateClassName={this.pageContext.generateClassName}
+          sheetsRegistry={this.pageContext.sheetsRegistry}
+          sheetsManager={this.pageContext.sheetsManager}
         >
           {/* ThemeProvider makes the theme available down the React
               tree thanks to React context. */}
           <ThemeProvider theme={this.pageContext.theme}>
+          <ThemeProvider2 theme={this.pageContext.theme}>
             {/* CssBaseline kickstart an elegant, consistent, and simple baseline to build upon. */}
             <CssBaseline />
             {/* Pass pageContext to the _document though the renderPage enhancer
                 to render collected styles on server-side. */}
             <Component pageContext={this.pageContext} {...pageProps} />
+          </ThemeProvider2>
           </ThemeProvider>
+        </StylesProvider2>
         </StylesProvider>
       </Container>
     );

I think that we should do the following change in the core:

--- a/packages/material-ui/src/Box/Box.js
+++ b/packages/material-ui/src/Box/Box.js
@@ -2,7 +2,7 @@

 import borders from '@material-ui/system/borders';
 import compose from '@material-ui/system/compose';
-import styled from '@material-ui/styles/styled';
+import { styled } from '@material-ui/styles';
 import display from '@material-ui/system/display';
 import flexbox from '@material-ui/system/flexbox';
 import palette from '@material-ui/system/palette';

cc @eps1lon I would like to get your point of view on the problem. We should probably forbid any @material-ui/styles/X imports.

[oliviertassinari]

@weslenng Do you want to submit a pull request to fix the problem? :)

[pheuter]

@oliviertassinari It seems like 4.0.0-alpha.3 removes @material-ui/core/styles. Does it address this issue of having multiple styles packages? If so, is there a replacement for import { createMuiTheme } from "@material-ui/core/styles" ?

[oliviertassinari]

@material-ui/core/styles should still be present. What makes you think it's now? No, it won't solve the problem.

[pheuter]

Oh ok, I misread https://github.com/mui-org/material-ui/pull/14767 then. Seemed like it's ripping out a lot of code and the Deprecation and Breaking Changes sections suggested renaming the imports from @material-ui/core/styles to @material-ui/styles

[oliviertassinari]

@pheuter We document the upgrade from v3 to v4 in https://next.material-ui.com/guides/migration-v3/#styles.

[pheuter]

Yeah, I'm currently on 4.0.0-alpha.2 myself, was just confused by alpha.3's "Remove the old styles modules" bit.

[pheuter]

I'd love to start using the Box component but it's still not clear to me what the problem is. You mention there are 2 versions of styles but the way I setup my app is to have a single StylesProvider and ThemeProvider from @material-ui/styles that does the whole install() thing for core components to still work.

And everything does work, even on the SSR the Box component gets a proper style. It's only when the client-side app loads does it generate a different className _just_ for the Box component. Is the change you're proposing to the styled import in material-ui/src/Box/Box.js supposed to fix that issue? I can submit a PR for that if that's what's holding us back.

[oliviertassinari]

Is the change you're proposing to the styled import in material-ui/src/Box/Box.js supposed to fix that issue?

@pheuter Yes, you can try the following implementation locally to verify this affirmation:

import borders from '@material-ui/system/borders';
import compose from '@material-ui/system/compose';
import display from '@material-ui/system/display';
import flexbox from '@material-ui/system/flexbox';
import palette from '@material-ui/system/palette';
import positions from '@material-ui/system/positions';
import shadows from '@material-ui/system/shadows';
import sizing from '@material-ui/system/sizing';
import spacing from '@material-ui/system/spacing';
import typography from '@material-ui/system/typography';
import css from '@material-ui/system/css';
import { styled } from '@material-ui/styles';

export const styleFunction = css(
  compose(
    borders,
    display,
    flexbox,
    positions,
    palette,
    shadows,
    sizing,
    spacing,
    typography,
  ),
);

const Box = styled('div')(styleFunction, { name: 'MuiBox' })

export default Box;

I can submit a PR for that if that's what's holding us back.

We would love that! ❤️