mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.43k stars 31.84k forks source link

Class name mismatch when using material-ui components in styled-components #13355

Closed geiszla closed 5 years ago

geiszla commented 5 years ago

Using styled material-ui components results in a class name mismatch when rendered on the client.

Expected Behavior

During server-side rendering, the generated class names should be the same as the generated class names on the client even when using styled material-ui components.

Current Behavior

When using a material-ui component wrapped in styled HOC, different rules are passed to the class name generator on server than on client resulting in class name mismatch.

Steps to Reproduce

  1. Use server-side rendering for material-ui (server-side rendering for styled-components doesn't affect the issue)
  2. Add styled components to your app. At this point, it generates the class names correctly.
  3. Add material-ui components wrapped in styled HOC to your app, e.g.
    
    const StyledCard = styled(Card)`
    max-width: 382px;
    `;

render() {

...

}


At this point there are more class names generated by the generator on client side and they are in the wrong order.

E.g.
client:

MuiCard-root-1 MuiPaper-root-2 MuiPaper-rounded-3 MuiPaper-elevation0-4 MuiPaper-elevation1-5 MuiPaper-elevation2-6 MuiPaper-elevation3-7 MuiPaper-elevation4-8 ... MuiTouchRipple-childLeaving-149 MuiTouchRipple-childPulsate-150


server:

MuiCardContent-root-1 MuiTypography-root-2 MuiTypography-display4-3 MuiTypography-display3-4 MuiTypography-display2-5 MuiTypography-display1-6 MuiTypography-headline-7 MuiTypography-title-8 ... MuiPaper-elevation23-142 MuiPaper-elevation24-143



This causes the warning when rendering on client:
> Warning: Prop `className` did not match. Server: "MuiTypography-root-2 MuiTypography-body2-10" Client: "MuiTypography-root-30 MuiTypography-body2-38"

This project can be used to reproduce the issue: https://github.com/geiszla/react-website-starter/tree/styled-components

Build with `yarn dev`, start with `yarn start` and navigate to `localhost:8080` to view app.
SSR is in `server/express.jsx`, client is in `src/index.jsx`, wrapped material-ui component is in `src/components/Login.jsx`.

Edit: The app will look broken, as I removed some styles for testing. The indicator of the problem should be the warning in the console.

## Context
<!---
    What are you trying to accomplish? How has this issue affected you?
    Providing context helps us come up with a solution that is most useful in the real world.
-->

## Your Environment
<!---
    Include as many relevant details about the environment with which you experienced the bug.
    If you encounter issues with typescript please include version and tsconfig.
-->

| Tech         | Version |
|--------------|---------|
| Material-UI  | v3.3.0  |
| React        | v16.5.2   |
| Browser      |   Chromium 71.0.3575.0 (Developer Build) (64-bit)   |
| Styled Components |     v4.0.2    |
oliviertassinari commented 5 years ago

@geiszla Thank you for the reproduction repository, I will look into to. Also, keep in mind that you can use JSS with the styled-components API with very few lines of code:

geiszla commented 5 years ago

@oliviertassinari Can you use this to create styled components from basic HTML tags (e.g. div, h1, etc.)?

geiszla commented 5 years ago

Nevermind, I found that it can be done by passing the tag name as a string.

One more question: is it possible for the styled components to have proper JSS class names (i.e. Component-StyleName-#)?

eps1lon commented 5 years ago

@geiszla I can't reproduce it with your repro.

$ yarn
$ yarn dev
$ yarn start # in other terminal
server is started at http://localhost:8080

----------------[ PAGE LOAD: / ]----------------------
Warning: Failed prop type: The prop `username` is marked as required in `Home`, but its value is `undefined`.
    in Home

Edit: Didn't see the branch. Will look again.

oliviertassinari commented 5 years ago

I'm looking at it.

oliviertassinari commented 5 years ago

@geiszla This is an issue with loadable-components, unfortunately, they don't accept issues… I could fix the problem with the following changes:

diff --git a/server/express.jsx b/server/express.jsx
index da61711..8e9f2c2 100644
--- a/server/express.jsx
+++ b/server/express.jsx
@@ -94,7 +94,7 @@ async function renderPage(reactApp, client, sheetsRegistry) {
   // Generate styles
   const sheet = new ServerStyleSheet();

-  const loadableState = await getLoadableState(reactApp);
+  // const loadableState = await getLoadableState(reactApp);
   const content = await renderToStringWithData(reactApp);

   // console.log(loadableState);
@@ -116,7 +116,6 @@ async function renderPage(reactApp, client, sheetsRegistry) {

         <script>window.__APOLLO_STATE__ = ${JSON.stringify(client.extract())};</script>

-        ${loadableState.getScriptTag()}
         <script src="${SCRIPTS_URL}/scripts/main.bundle.js" defer></script>

         <link rel="stylesheet" type="text/css" href="styles.css">
diff --git a/src/index.jsx b/src/index.jsx
index 730ebc3..51039b1 100644
--- a/src/index.jsx
+++ b/src/index.jsx
@@ -5,7 +5,7 @@ import 'whatwg-fetch';

 import { InMemoryCache } from 'apollo-cache-inmemory';
 import { create } from 'jss';
-import { loadComponents } from 'loadable-components';
+// import { loadComponents } from 'loadable-components';
 import React from 'react';
 import { ApolloProvider } from 'react-apollo';
 import ReactDOM from 'react-dom';
@@ -30,7 +30,7 @@ if (process.env.NODE_ENV !== 'production') {
 }

 export default async function hydrateApp(hotModuleReplacement = module.hot) {
-  await loadComponents();
+  // await loadComponents();

   const inMemoryCache = new InMemoryCache().restore(window.__APOLLO_STATE__ || {});
   const client = createApolloClient(false, undefined, inMemoryCache);

oct -28-2018 12-57-25

geiszla commented 5 years ago

@oliviertassinari Thanks for the research. Do you have any details on what's going wrong with loadable-components? They just released a new v3 of the package, so it would make sense to test the issue again, and if it has the same problem, report it at https://github.com/smooth-code/loadable-components/issues.

oliviertassinari commented 5 years ago

I'm confused, I thought that the dependency was coming from @jamiebuilds but it seems to be coming from @neoziro.

gregberge commented 5 years ago

I think it was a problem in the tree traversing, the new v3/v4 doesn't do it. It should not cause any problem with Material UI, just upgrade 🙂.

oliviertassinari commented 5 years ago

❤️