hupe1980 / gatsby-plugin-material-ui

Gatsby plugin for Material-UI with built-in server-side rendering support
MIT License
136 stars 25 forks source link

4.0.3 plugin results in flakey SSR behavior #80

Open klittlepage opened 2 years ago

klittlepage commented 2 years ago

Hi, thanks for your great work on the 4.x branch and support for mui v5 🎉 . I hit a snag when updating from 3.0.1 to 4.0.3. For some reason, head elements inserted via gatsby-plugin-react-helmet are sporadically clobbered during SSR. As a concrete example I'm getting empty elements such as <title data-react-helmet="true"></title> in places.

This behavior is nondeterministic across builds. As a long shot, I experimented with placing this plugin both before and after gatsby-plugin-react-helmet given the use of replaceRenderer in the 4.x line. That didn't resolve the problem.

Any ideas are greatly appreciated!

amalitsky commented 2 years ago

Literally spent the whole day debugging this issue! Happy to come across this.

Posted comment in this thread: https://github.com/gatsbyjs/gatsby/issues/22908#issuecomment-964642891

Probably related to this change: https://github.com/hupe1980/gatsby-plugin-material-ui/commit/e787e20016f775d536153492aa02a1937134281b#diff-7e93431ae23dd281591a7c6f2e6c8e3fb708cd3ba67ef2011d1607db8dd6804e

Added console.log to replaceRendered (copy pasted from this repo) and onRenderBody (used by gatsby-plugin-react-helmet).

I'm getting the following order:

replaceRendered: page A
replaceRendered: page B
onRenderBody: page A
onRenderBody: page B

While if I understand the code correctly expected order should be like this:

replaceRendered: page A
onRenderBody: page A
replaceRendered: page B
onRenderBody: page B

This might be crucial for proper Helmet SSR rendering.

amalitsky commented 2 years ago

Ok, I don't think this is the bug with gatsby-plugin-material-ui since it is using public API replaceRender provided by gatsbyjs. To be honest I'm not sure if redefined replaceRender is causing the issue at all.

Looks like the root cause of my issue (very similar to what was described by the topic starter) is the concurrency of html builds present in the gatsbyjs code itself.

It could be that redefined replaceRender increases the probability of the race condition previously present. In my case one article is pretty lengthy and I assume that also contributes to the increased probability of hitting the race condition between reactDOM.renderToString and Helmet.renderStatic() calls being made by different pages during SSR build.

More on why it matters: react-helmet readme and alternative react-helmet-async. _Note: react-helmet-async doesn't seem to solve our "gatsbyjs specific" problem._

Temporary(?) solution is to introduce your own replaceRenderer in gatsby-ssr.js, importing replaceRenderer from gatsby-plugin-material-ui and supplementing it with onRenderBody imported from gatsby-plugin-react-helmet.

Note that gatsby-plugin-react-helmet should be removed from the list of plugins in jest.config.js because we will be calling it's onRenderBody method manually. gatsby-plugin-material-ui should stay in the list of plugins but will produce Duplicate replaceRenderer found warning during the build.

My gatsbyjs site gatsby-ssr.ts:

import { ReplaceRendererArgs } from 'gatsby';
// @ts-ignore-next-line
import { replaceRenderer as muiReplaceRenderer } from 'gatsby-plugin-material-ui/gatsby-ssr';
// @ts-ignore-next-line
import { onRenderBody as reactHelmetOnRenderBody } from 'gatsby-plugin-react-helmet/gatsby-ssr';

export function replaceRenderer(replaceRendererArgs: ReplaceRendererArgs): void {
  muiReplaceRenderer.call(null, replaceRendererArgs); // calls renderToString inside. Intentionally wo await
  reactHelmetOnRenderBody.call(null, replaceRendererArgs); // calls Helmet.renderStatic()
}

Unrelated to the bug - it would be great to hear from this project maintainers if replaceRenderer is the only way to make this plugin work. And if so - why is that. Just to expand my understanding.

Thanks

stschwark commented 2 years ago

On our site I also saw page metadata from one page ending up on another. This started happening after upgrading the site from version 3.0.1 to version 4.0.3 of this plugin. After a downgrade to version 3.0.1 while keeping all other dependencies the same, the issue no longer occured. Looking at the main timeline of this repo this would suggest this issue is linked to one of the changes in #78.

rroslaniec commented 2 years ago

Solution proposed by @amalitsky works but it's a little bit hacky.

In my personal project I have two branches with Gatsby v4 but one with mui v4 and second with v5. With v4 everything works perfectly and with v5 had same problems as @klittlepage.

After some research in the history there was some similar problems (https://stackoverflow.com/questions/61442468/gatsby-react-helmet-generating-empty-title-on-ssr) with gatsby. I'm not sure but maybe also that is related with CacheProvider?

rroslaniec commented 2 years ago

I resolved all my problems 🎉

Basically a solution was to replace gatsby-plugin-material-ui with gatsby-plugin-emotion. I did not yet found any problems with my site.

stldo commented 2 years ago

I was debugging this issue and ended with a small reproducible example before finding this page — https://github.com/stldo/reproduce-material-ui-react-helmet-issue. I committed the public folder to the repo. In /public/first-post/index.html, the <title> tag — set by React Helmet — shows the value "/second-post", while the <h1> element shows the correct pathname of the page, "/first-post". /public/second-post/index.html has an empty <title> value. The other routes rendered fine. Removing the CSS import from gatsby-browser.js makes everything work without issues.

@rroslaniec solution of using gatsby-plugin-emotion seems to work, but it needs a few adjustments if you are coming from gatsby-theme-material-ui.