Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SSR, makeStyles, overrides not same on each page reload #28994

Closed
1 of 2 tasks
borm opened this issue Oct 11, 2021 · 6 comments
Closed
1 of 2 tasks

SSR, makeStyles, overrides not same on each page reload #28994

borm opened this issue Oct 11, 2021 · 6 comments
Assignees
@mnajdova
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. status: waiting for author Issue with insufficient information

Comments

@borm
Copy link

borm commented Oct 11, 2021 •
edited
Loading 

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>
  );
}
  • 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 😯

image

image

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:

  1. Open sandbox
  2. open one more bash
  3. type :/sandbox$ webpack -w if webpack will not start on dev mode authomatically,
  4. look at classNames on server and client console, server have increment overrides classes
@borm borm added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 11, 2021
@tiagooliveira08
Copy link

same problem :(

@mnajdova
Copy link
Member

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.

@mnajdova mnajdova added package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 12, 2021
@borm
Copy link
Author

borm commented Oct 12, 2021 •
edited
Loading

@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
Copy link
Member

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
Copy link
Author

borm commented Oct 12, 2021 •
edited
Loading

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

@borm borm closed this as completed Oct 12, 2021
@mnajdova
Copy link
Member

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 #22342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mnajdova mnajdova

Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. status: waiting for author Issue with insufficient information
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@borm @mnajdova @tiagooliveira08