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.

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

[docs] Gatsby order conflict in SSR output (head/JSS vs. body/emotion) #25312

Closed
MartinDawson opened this issue Mar 11, 2021 · 14 comments
Closed
Labels
docs Improvements or additions to the documentation ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@MartinDawson
Copy link

MartinDawson commented Mar 11, 2021

I'm following this example exactly: https://github.com/mui-org/material-ui/tree/next/examples/gatsby

Here's my gatsby-config.js (abbreviated):

module.exports = {
  plugins: [
    "gatsby-plugin-layout",
    {
      resolve: "gatsby-plugin-material-ui",
      options: {},
    },
    "gatsby-plugin-emotion",
  ],
};

Gatsby-browser.js:

export { wrapPageElement, wrapRootElement } from "./sharedGatsby";

Gatsby-ssr.js

export { wrapPageElement, wrapRootElement } from "./sharedGatsby";

SharedGatsby.js

import React from "react";
import theme from "./src/theme";
import { createMuiTheme, ThemeProvider } from "@material-ui/core/styles";

export const wrapRootElement = ({ element }) => {
  return (
        <ThemeProvider theme={createMuiTheme(theme)}>
          {element}
        </ThemeProvider>
  );
};

In pages/index.js:

import {
  Typography,
  makeStyles,
} from "@material-ui/core";

const useStyles = makeStyles({
  typographyHeader: {
    fontWeight: "bold",
    fontSize: ({ isOnMobile }) => (isOnMobile ? 30 : 30),
    color: textColor,
  },
});


<StyledEngineProvider injectFirst>
         <Typography classes={{ root: classes.typographyHeader }} align="center">
          Goodbye, Excel.
          <Box>Hello, automated Discounted Cash Flows.</Box>
        </Typography>
 ...other components
</StyledEngineProvider>

package.json:

    "@material-ui/core": "^5.0.0-alpha.25",
    "@material-ui/icons": "^5.0.0-alpha.24",
    "@material-ui/styles": "^5.0.0-alpha.25",
    "@emotion/react": "^11.1.1",
    "@emotion/styled": "^11.0.0",
    "gatsby": "^2.32.3",
    "gatsby-plugin-material-ui": "^2.1.10",
    "gatsby-plugin-emotion": "^6.0.0",

It works 100% fine on the client side but when I run npm run build and generate a static SSR index.html the styles order is incorrect.

Here's the output:

image

You can see that my custom styles .jss4 and .jss2 are being overwritten by MuiTypography-root.

I imagine I'm doing something wrong but not sure what.

Thanks for any help

@MartinDawson MartinDawson added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 11, 2021
@MartinDawson
Copy link
Author

MartinDawson commented Mar 12, 2021

After further investigation, it seems to be a problem with all 3 styling methods, makeStyles, useStyles and the HOC API.

All 3 methods and my custom styles are overwritten by material ui's styles in SSR.

example:

import { makeStyles } from '@material-ui/core';

const useStyles = makeStyles({
  typographyHeader: {
    fontWeight: 'bold',
    fontSize: () => 30,
    color: '#292929',
  },
});

          <Typography classes={{ root: classes.typographyHeader }} align="center">
            Gatsby v5-alpha example
          </Typography>


running the example with js disabled (SSR):

image

running the example with js enabled (non-SSR):

image

You can see from the above that the styles are in the wrong order in the example gatsby repo.

@MartinDawson

This comment has been minimized.

@MartinDawson
Copy link
Author

@oliviertassinari Is their any chance this could be looked into please? I also posted an SO post here and it seems nobody knows:
https://stackoverflow.com/questions/66638599/material-ui-v5-server-side-rendering-css-order-not-working-with-gatsby

@oliviertassinari
Copy link
Member

@MartinDawson Yes, we will. I didn't yet have the chance. Next.js is growing a lot faster than Gatsby, so it didn't seem to be a crazy important. However, the onboarding experience is critical to the success of our project so it should be painless to start with Gatsby.

@MartinDawson
Copy link
Author

@oliviertassinari Thank you.

I considered next or gatsby. Guess I chose the wrong framework.

@mojimi
Copy link

mojimi commented Mar 27, 2021

It's a shame it's not working anymore, Gatsby is incredibly popular, including the Mui plugins Gatsby has

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 27, 2021

@mojimi What makes you think that it's "not working anymore". It's popular, but not really growing.

@MartinDawson I had a look at your issue. I believe this problem can't be solved with the current API exposed by Gatsby and emotion. What we would need is the capability to extract the CSS output from the HTML page, and inline it in the initial SSR request, before the JSS output.

You have 3 options:

  1. Stop using the makeStyles/withStyles API now. We are about to deprecate them in v5 (and remove in v6).
  2. Migrate to Next.js.
  3. Use styled-components instead of emotion. styled-components doesn't render it's style inside the body, with the order of the Gatsby plugins, you should be able to get the correct injection order.

While looking at the issue. I noticed this simplification opportunity:

diff --git a/examples/gatsby/plugins/gatsby-plugin-top-layout/gatsby-browser.js b/examples/gatsby/plugins/gatsby-plugin-top-layout/gatsby-browser.js
index 63f4104f1a..3f7ade93b0 100644
--- a/examples/gatsby/plugins/gatsby-plugin-top-layout/gatsby-browser.js
+++ b/examples/gatsby/plugins/gatsby-plugin-top-layout/gatsby-browser.js
@@ -1,7 +1,13 @@
 /* eslint-disable import/prefer-default-export */
 import * as React from 'react';
 import TopLayout from './TopLayout';
+import StyledEngineProvider from '@material-ui/core/StyledEngineProvider';

+// TODO v5: remove StyledEngineProvider once migration to emotion is completed
 export const wrapRootElement = ({ element }) => {
-  return <TopLayout>{element}</TopLayout>;
+  return (
+    <StyledEngineProvider injectFirst>
+      <TopLayout>{element}</TopLayout>
+    </StyledEngineProvider>
+  );
 };
diff --git a/examples/gatsby/src/pages/about.js b/examples/gatsby/src/pages/about.js
index eccbd909d9..cd84bae812 100644
--- a/examples/gatsby/src/pages/about.js
+++ b/examples/gatsby/src/pages/about.js
@@ -1,5 +1,4 @@
 import * as React from 'react';
-import StyledEngineProvider from '@material-ui/core/StyledEngineProvider';
 import Container from '@material-ui/core/Container';
 import Typography from '@material-ui/core/Typography';
 import Box from '@material-ui/core/Box';
@@ -9,18 +8,15 @@ import Copyright from '../components/Copyright';

 export default function About() {
   return (
-    // TODO v5: remove once migration to emotion is completed
-    <StyledEngineProvider injectFirst>
-      <Container maxWidth="sm">
-        <Box sx={{ my: 4 }}>
-          <Typography variant="h4" component="h1" gutterBottom>
-            Gatsby v5-alpha example
-          </Typography>
-          <Link to="/">Go to the main page</Link>
-          <ProTip />
-          <Copyright />
-        </Box>
-      </Container>
-    </StyledEngineProvider>
+    <Container maxWidth="sm">
+      <Box sx={{ my: 4 }}>
+        <Typography variant="h4" component="h1" gutterBottom>
+          Gatsby v5-alpha example
+        </Typography>
+        <Link to="/">Go to the main page</Link>
+        <ProTip />
+        <Copyright />
+      </Box>
+    </Container>
   );
 }
diff --git a/examples/gatsby/src/pages/index.js b/examples/gatsby/src/pages/index.js
index e17f067ce5..a050a09b2b 100644
--- a/examples/gatsby/src/pages/index.js
+++ b/examples/gatsby/src/pages/index.js
@@ -1,5 +1,4 @@
 import * as React from 'react';
-import StyledEngineProvider from '@material-ui/core/StyledEngineProvider';
 import Container from '@material-ui/core/Container';
 import Typography from '@material-ui/core/Typography';
 import Box from '@material-ui/core/Box';
@@ -9,20 +8,17 @@ import Copyright from '../components/Copyright';

 export default function Index() {
   return (
-    // TODO v5: remove once migration to emotion is completed
-    <StyledEngineProvider injectFirst>
-      <Container maxWidth="sm">
-        <Box sx={{ my: 4 }}>
-          <Typography variant="h4" component="h1" gutterBottom>
-            Gatsby v5-alpha example
-          </Typography>
-          <Link to="/about" color="secondary">
-            Go to the about page
-          </Link>
-          <ProTip />
-          <Copyright />
-        </Box>
-      </Container>
-    </StyledEngineProvider>
+    <Container maxWidth="sm">
+      <Box sx={{ my: 4 }}>
+        <Typography variant="h4" component="h1" gutterBottom>
+          Gatsby v5-alpha example
+        </Typography>
+        <Link to="/about" color="secondary">
+          Go to the about page
+        </Link>
+        <ProTip />
+        <Copyright />
+      </Box>
+    </Container>
   );
 }

In case somebody want to work on it 👍

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 27, 2021
@oliviertassinari oliviertassinari changed the title Gatsby Next example for MUI v5 not working in SSR [docs] Gatsby CSS injection order conflict in SSR output (head/JSS vs. body/emotion) Mar 27, 2021
@oliviertassinari oliviertassinari changed the title [docs] Gatsby CSS injection order conflict in SSR output (head/JSS vs. body/emotion) [docs] Gatsby order conflict in SSR output (head/JSS vs. body/emotion) Mar 27, 2021
@MartinDawson
Copy link
Author

MartinDawson commented Mar 27, 2021

@oliviertassinari Thanks a lot for the update.

I can't migrate to next.js yet due to time constraints so I'm interested in the other 2 solutions .

Edit: Okay, using the sx prop works great. I have migrated my app to completely using the sx prop anyway so that's fine for me until I switch to nextjs at a later date.

Thanks

@mnajdova mnajdova added OCD21 and removed good first issue Great for first contributions. Enable to learn the contribution process. labels Jun 30, 2021
@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Jul 16, 2021
@oliviertassinari
Copy link
Member

We already fixed the issue in #27357. If needed, developers can prepend: https://github.com/mui-org/material-ui/pull/27357/files#r673042892.

@ffMathy
Copy link

ffMathy commented Oct 22, 2021

It doesn't seem to work for me. The issue still persists. Here's my project:
https://github.com/sponsorkit/sponsorkit.io/tree/main/src/frontend

@ffMathy
Copy link

ffMathy commented Oct 22, 2021

  1. Stop using the makeStyles/withStyles API now. We are about to deprecate them in v5 (and remove in v6).

I tried this without luck. The issue persists.

3. Use styled-components instead of emotion. styled-components doesn't render it's style inside the body, with the order of the Gatsby plugins, you should be able to get the correct injection order.

How do I do this?

@MartinDawson
Copy link
Author

@ffMathy It works if you use the sx prop on components.

You can do everything with emotion/styled-components within sx anyway.

If a component doesn't support sx then just wrap it in the Box component which does support it

@ffMathy
Copy link

ffMathy commented Oct 22, 2021

@MartinDawson ah maybe that's the issue then. I think I am using a Theme from the MUI store that doesn't use sx.

Any workaround?

@ffMathy
Copy link

ffMathy commented Oct 23, 2021

Actually @MartinDawson I just contacted the theme creators. They say they only use sx.

Not sure why it's happening then.

Perhaps because I am also importing styles directly in TypeScript from .module.scss files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

No branches or pull requests

5 participants