SSR, makeStyles, overrides not same on each page reload

borm commented 3 years ago

import * as React from "react";
import Container from "@mui/material/Container";
import Typography from "@mui/material/Typography";
import { makeStyles } from "@mui/styles";

const useStyles = makeStyles(
  () => ({
    overrides: {
      width: ({ width }) => width
    },
  }),
  {
    name: "App"
  }
);

export default function App() {
  const classes = useStyles({ width: 1200 });
  console.log(classes);
  return (
    <Container className={classes.overrides}>
      <Typography variant="h4" component="h1" gutterBottom>
        Server Rendering v5 example
      </Typography>
    </Container>
  );
}

[x] The issue is present in the latest release.
[ ] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Expected Behavior 🤔

as i remember in v4 that issues was possible to fix by doing createGenerateClassName as a function

export const generateClassName = () => createGenerateClassName();

const sheets = new ServerStyleSheets({
    serverGenerateClassName: generateClassName(),
  });

Steps to Reproduce 🕹

https://codesandbox.io/s/interesting-torvalds-j3lk8?file=/App.js Steps:

Open sandbox
open one more bash
type :/sandbox$ webpack -w if webpack will not start on dev mode authomatically,
look at classNames on server and client console, server have increment overrides classes

tiagooliveira08 commented 3 years ago

same problem :(

mnajdova commented 3 years ago

You haven't configured JSS for SSR. Is it intentional that you are using both emotion and JSS? If yes, take a look at the v4 examples/ssr for more info on how to configure it.

borm commented 3 years ago

@mnajdova, i got example from Plain server-side if u think im doing something wrong please explain me how to fix it correctly in v5 my example and what i got from the mui website is identical, only one change is makeStyles with overrides as i know v4 doenst have overrides in makeStyles

Update, okay, looks like ServerStyleSheets works good

so changes,

function renderFullPage(html, css, emotionCss) {
  return `
    <!DOCTYPE html>
    <html lang="en">
      <head>
        <title>My page</title>
        <style>${css}</style>
        ${emotionCss}
....
}

import { ServerStyleSheets } from '@mui/styles';

const sheets = new ServerStyleSheets();
const html = ReactDOMServer.renderToString(
    sheets.collect(
     <CacheProvider value={cache}>
      <ThemeProvider theme={theme}>
        {/* CssBaseline kickstart an elegant, consistent, and simple baseline to build upon. */}
        <CssBaseline />
        <App />
      </ThemeProvider>
    </CacheProvider>
   )
  );

// Send the rendered page back to the client.
  res.send(renderFullPage(html, sheets.toString(), emotionCss));

@mnajdova, can u approve it? and add to documentation

PS, im only one confused what we needs to use both styles, ServerStyleSheets & emotionCss, it will not have conflicts? because in this case i have duplicates between those styles otherwise please provide a correct example how to combine emotion & jss in v5 and apply it to <head /> on ssr

mnajdova commented 3 years ago

I would just change the order of these two lines:

function renderFullPage(html, css, emotionCss) {
  return `
    <!DOCTYPE html>
    <html lang="en">
      <head>
        <title>My page</title>
-        <style>${css}</style>
-        ${emotionCss}
+        ${emotionCss}
+        <style>${css}</style>

So that the styles coming from jss (makeStyles, withStyles) would override those from emotion (the default styles for the components in v5).

Please add to the documentation

PS, im only one confused what we needs to use both styles, ServerStyleSheets & emotionCss, it will not have conflicts? because in this case i have duplicates between those styles otherwise please provide a correct example how to combine emotion & jss in v5 and apply it to on

@borm in v5 we are using emotion as a default styling engine. We think it's overhead to use JSS for styles overrides, as you would have to bundle two styling packages. This is why we don't document this. We have a section in the migration guide about how you can migrate from JSS - https://mui.com/guides/migration-v4/#migrate-from-jss There are two options: using the built in prop sx or using the styled() utility. If you wish to use a similar syntax as JSS's makeStyles, you can use tss-react which also depends on emotion.

borm commented 3 years ago

hm, is strange but it also works good without emotionCss, if i have removed <CacheProvider value={cache}>

- function renderFullPage(html, css, emotionCss) {
+ function renderFullPage(html, css) {
  return `
    <!DOCTYPE html>
    <html lang="en">
      <head>
        <title>My page</title>
        <style>${css}</style>
-        ${emotionCss}
....
}

import { ServerStyleSheets } from '@mui/styles';

const sheets = new ServerStyleSheets();
const html = ReactDOMServer.renderToString(
    sheets.collect(
-     <CacheProvider value={cache}>
      <ThemeProvider theme={theme}>
        {/* CssBaseline kickstart an elegant, consistent, and simple baseline to build upon. */}
        <CssBaseline />
        <App />
      </ThemeProvider>
-    </CacheProvider>
   )
  );

// Send the rendered page back to the client.
-  res.send(renderFullPage(html, sheets.toString(), emotionCss));
+  res.send(renderFullPage(html, sheets.toString()));

PS, yes, i would like to continue working with jss as well, my vision is styled components was bad idea, and really not clear for understanding how it currently works in material core system, jss was pretty clear going to close this issue now, thanks

mnajdova commented 3 years ago

PS, yes, i would like to continue working with jss as well, my vision is styled components was bad idea, and really not clear for understanding how it currently works in material core system, jss was pretty clear

Lots of the reasoning is already covered in https://github.com/mui-org/material-ui/issues/22342

mui / material-ui