search

gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.27k stars 10.31k forks source link

Material-UI and Gatsby v2 - hot reload does not work for styles #8237

Closed HriBB closed 5 years ago

HriBB commented 6 years ago

This is basically a re-post from the Material-UI issue in hopes that someone can help me.

I'm using Material-UI v3.1.0 with Gatsby v2.0.0 following the official example. Gatsby v2 changed how layout works, specifically the changes after gatsby@2.0.0-beta.59 caused many headaches for me. I managed to get my production build to work perfectly, but development is a different story. Styles work, but hot reload does not, which makes dev story pretty bad. Having to refresh the page after each style change sucks big time.

I don't have time to debug it ATM, going for a short break, but decided to post the issue. Maybe it magically resolves when I come back a week later :)

My use case is a bit different than the example. I use Gatsby's new wrapPageElement to wrap page with my custom Layout. Inside that layout, I use withRoot from the example. This way, JssProvider and MuiThemeProvider are created only once, not each time a different page is rendered. I suspect this might be the reason why hot reload does not work. Then again, it could be related to babel@7 problems with RHL. Note that in the example, hot reload works, but it's not using the wrapPageElement api. Would be cool if there was some example using it.

Furthermore, JssProvider is also created inside replaceRenderer method. So now I have two JSS providers on SSR: one in replaceRenderer and one in withRoot. This is bad, right? I already created an issue on Gatsby's issue tracker, but got nowhere. I guess there's not so many people living on the edge :)

Anyway, here's my code:

(note that gatsby-browser.js is pretty much the same as gatsby-ssr.js without the replaceRenderer)

const React = require('react')
const { renderToString } = require('react-dom/server')
const { JssProvider } = require('react-jss')

const postcss = require('postcss')
const autoprefixer = require('autoprefixer')
const csswring = require('csswring')

const { AdyenProvider } = require('providers/adyen')
const { CartProvider } = require('providers/cart')
const getPageContext = require('utils/getPageContext').default
const Layout = require('components/layout/Layout').default

const prefixer = postcss([autoprefixer])
const minifier = postcss([csswring])

exports.wrapRootElement = ({ element }) => {
  return (
    <AdyenProvider>
      <CartProvider>
        {element}
      </CartProvider>
    </AdyenProvider>
  )
}

exports.wrapPageElement = ({ element, props }) => {
  return (
    <Layout {...props}>{element}</Layout>
  )
}

exports.replaceRenderer = ({ bodyComponent, replaceBodyHTMLString, setHeadComponents }) => {
  const muiPageContext = getPageContext()

  const bodyHTML = renderToString(
    <JssProvider registry={muiPageContext.sheetsRegistry}>
      {bodyComponent}
    </JssProvider>
  )

  replaceBodyHTMLString(bodyHTML)

  // extract and minify styles
  const css = muiPageContext.sheetsRegistry.toString()
  const options = { from: undefined }
  const prefixed = prefixer.process(css, options)
  const minified = minifier.process(prefixed.css, options)

  setHeadComponents([
    <style
      type={'text/css'}
      id={'server-side-jss'}
      key={'server-side-jss'}
      dangerouslySetInnerHTML={{ __html: minified.css }}
    />,
  ])
}
HriBB commented 6 years ago

I managed to get hot reloading to work by using export default hot(module)(PageOrTemplate).

Had to add this to all my pages and templates. Hacky, but at least I can work normally ...

// @flow

import React from 'react'
import { hot } from 'react-hot-loader'
import { Trans } from '@lingui/react'

import Content from 'components/ui/Content'
import Section from 'components/ui/Section'
import Title from 'components/ui/Title'

import HeroImage from 'components/about/HeroImage'

const HomePage = () => {
  return (
    <Content>
      <HeroImage />
      <Section first>
        <Title>
          <Trans>Hot reloading</Trans>
        </Title>
      </Section>
    </Content>
  )
}

export default hot(module)(HomePage)
kakadiadarpan commented 6 years ago

@HriBB is this issue reproducible in the example of material-ui for Gatsby?

Can you also provide relevant environment information by running gatsby info --clipboard?

tmctl commented 6 years ago

@kakadiadarpan I have experienced this issue as well - here's a demo https://github.com/relytmcd/gatsby-issue8237-demo

basically, hot reload was working for me when changing styles in a page, but when I import a component, changing those styles does not hot reload

System: OS: macOS High Sierra 10.13.6 CPU: x64 Intel(R) Core(TM) i7-3615QM CPU @ 2.30GHz Shell: 5.3 - /bin/zsh Binaries: Node: 10.11.0 - /usr/local/bin/node Yarn: 1.10.1 - /usr/local/bin/yarn npm: 6.4.1 - /usr/local/bin/npm Browsers: Chrome: 69.0.3497.100 Firefox: 61.0.2 Safari: 12.0 npmPackages: gatsby: latest => 2.0.12 npmGlobalPackages: gatsby-cli: 2.0.0-rc.6

also tried after updating gatsby-cli to 2.4.2

oliviertassinari commented 6 years ago

I have been digging into this issue, spent an hour or so. I have no clue what's going on. The React lifecycle calls seem to be all wrong. Next.js has recently removed the react-hot-loader package. Overheal, this change was a great improvement both for update time performance and for predictability. There is one thing that looks very wrong, the wrapRootElement() method as well as the children hooks are called twice. Shouldn't happen only once?

oliviertassinari commented 6 years ago

Here is the behavior with the simplest test case possible, I have added some log to the key events:

first render

withStyles.js:95 withStyles()
[HMR] connected
[HMR] bundle has 1 warnings
gatsby-browser.js:24 wrapRootElement()
withStyles.js:266 attach refs 0 uuid 0.444594722974339
withStyles.js:285 add into DOM {root: "Index-root-1"}
index.js:81 render {root: "Index-root-1"}

hot reloading

[HMR] bundle rebuilding
[HMR] bundle rebuilt in 270ms
[HMR] Checking for updates on the server...
withStyles.js:95 withStyles()
withStyles.js:266 attach refs 0 uuid 0.27185351167434657
withStyles.js:285 add into DOM {root: "Index-root-2"}
withStyles.js:266 attach refs 1 uuid 0.444594722974339
[HMR] Updated modules:
[HMR]  - ./src/pages/index.js
[HMR]  - ./.cache/sync-requires.js
[HMR]  - ./.cache/app.js
[HMR]  - ./.cache/root.js
[HMR] App is up to date.
gatsby-browser.js:24 wrapRootElement()
withStyles.js:266 attach refs 2 uuid 0.444594722974339
src/pages/index.js:81 render {root: "Index-root-1"}
withStyles.js:307 detach refs 3 uuid 0.444594722974339
withStyles.js:266 attach refs 2 uuid 0.444594722974339
src/pages/index.js:81 render {root: "Index-root-1"}
withStyles.js:307 detach refs 3 uuid 0.444594722974339
oliviertassinari commented 6 years ago

https://github.com/oliviertassinari/material-ui/tree/gatsby-bug-reproduction

pfftdammitchris commented 6 years ago

This seriously should be solved.

HriBB commented 6 years ago

@pfftdammitchris you are welcome to help ;) I'm pretty busy ATM, so I cannot focus on stuff that does not pay the bills.

From what I understand, it's not yet clear where the problem is: Gatsby, MaterialUI, React Hot Loader or a combination. If someone can figure out where and what needs to be fixed, that would be a big step towards finding the solution.

Might be related to https://github.com/gatsbyjs/gatsby/issues/8018. In Gatsby v2, it's impossible to pass props to bodyComponent, therefore two instances of JssProvider need to be used, which is not ideal.

oliviertassinari commented 6 years ago

For anyone wondering, the workaround is not to use the wrapRootElement API, React lifecycle calls number and order are wrong.

axe312ger commented 6 years ago

I fiddled around, thats what I found:

I hope this helps @HriBB :)

cpboyd commented 5 years ago

FWIW, I don't remember this being an issue when I filed #7716 and #7568 (there was another issue, related to not being able to track styles when building more than one page).

I'm sure @oliviertassinari's method works, but you then lose out on the ability to have smooth transitions between pages with shared layouts...

Which is what wrapPageElement and wrapRootElement were intended to resolve.

Looking through the current open issues, it seems that HMR is responsible for quite a few. Maybe it's worth looking at how Next.js migrated away from react-hot-loader, since it seems like it has caused on-going issues.

I noticed in the changelog that it used to have issues with stateless components, so I tried converting all my stateless components to React.Component but that didn't resolve this issue.

My biggest complaint with Gatsby is that I can have something looking amazing in the develop mode, but it always has countless issues to debug when I try to build.

cpboyd commented 5 years ago

Hmm. So, after looking into it further, it seems that gatsby-plugin-offline was leading to my style issues as mentioned in #5734.

tmctl commented 5 years ago

this is linked from https://github.com/mui-org/material-ui/blob/master/examples/gatsby/gatsby-ssr.js so I thought I'd jump back in, though I originally commented on hot reloading, i've progressed on the project and got to production issues. Here's what worked for me.

The last bullet is the new suggestion on the thread. Without it, the initial page would look as expected from html and then when the js loaded the styles got weird.

cpboyd commented 5 years ago

Note per https://www.gatsbyjs.org/packages/gatsby-plugin-offline/#remove:

 plugins: [
-  `gatsby-plugin-offline`,
+  `gatsby-plugin-remove-serviceworker`,
 ]

It's now suggested to add the gatsby-plugin-remove-serviceworker plugin to ensure that the service worker is removed completely. (Otherwise the service worker still tries to take over and jumbles all the styles.) I previously had to force-clear the site's app data in Chrome, which I obviously couldn't expect users of a public site to do.

walleXD commented 5 years ago

Is SSR with Gatsby and MUI currently broken? I am not able to get things working and the message in the example says it's not ready yet

Is there any workaround?

@oliviertassinari Any ideas?

axe312ger commented 5 years ago

@walleXD my SSR works fine. See https://github.com/axe312ger/gatsby-starter-collaborative-app

I did not yet test if gatsby-offline is the issue. (Would be a easy fix, just disable it in dev 🤷‍♂️)

jeffwillette commented 5 years ago

The solution presented by @relytmcd worked for me for now. I still get a quick flashing of some styles on reload so it might not be ideal, but it works a lot better than having styles permanently messed up.

bluepeter commented 5 years ago

For those following along, I am working on a Gatsby Material UI starter with the techniques discussed in this thread. https://github.com/bluepeter/gatsby-material-ui-business-starter

jeffwillette commented 5 years ago

@bluepeter I tried to implement your example but I am still getting the same symptom that @relytmcd mentioned where the style initially looks ok and then freaks out a fraction of a second later. I had to add the hydrator. I don't know what is going on yet but I'll try to take a closer look

bluepeter commented 5 years ago

Thanks @deltaskelta ... do you have anything you can you PR to my repo? I plan to do some more work on this over the next several days (at the Oregon coast right now for 3 day weekend, so limited time).

jeffwillette commented 5 years ago

Well yours seems to work as is. Are there any problems you know of? I'm trying to figure out why yours works and mine doesn't, so if I can figure it out I'll at least post it here

bluepeter commented 5 years ago

@deltaskelta cool! I was experiencing problems prior to finding this issue (and following some of the steps within)... I still need to do a once over the code, but I am not able to reproduce the issues I was experiencing w/ the fixes outlined above and included in the repo.

jeffwillette commented 5 years ago

I still can't get it. IDK what is going on.

  1. The style is ok on initial paint and then goes away
  2. if I look at dev tools and hit refresh then I see something is changing in the head, but all the jss classes in my body stay the same
  3. If I click on an internal link on my site then everything goes back to normal. It is only on the initial load (or clicking refresh) that messes it up

the gatsby browser block above makes it render but its not ideal because there is a quick layout change that occurs.

If anyone cares to take a stab at looking at whats going on my repo is here. https://github.com/deltaskelta/deltaskelta.github.io.src

akoel commented 5 years ago

having the exact same issue as @deltaskelta described above. Not sure why: https://github.com/Cruzercru/Queensland-Native-Seeds-Website https://github.com/bluepeter/gatsby-material-ui-business-starter would work in a production build and my code doesn't!

Update: So it seems that the Gatsby-material-ui-business-starter seems to avoid withStyles altogether. Probably the reason why it works as expected on build.

bluepeter commented 5 years ago

@deadcoder0904 @akadop I have updated our repo https://github.com/bluepeter/gatsby-material-ui-business-starter to use withStyles. You can see this in index.js here, and in the demo you can see how we use withStyles to override the color of the home page button. As described above, this required a few odds and ends to get working properly. Let me know if you spot any other issues.

akadop commented 5 years ago

@deadcoder0904 @akadop I have updated our repo https://github.com/bluepeter/gatsby-material-ui-business-starter to use withStyles. You can see this in index.js here, and in the demo you can see how we use withStyles to override the color of the home page button. As described above, this required a few odds and ends to get working properly. Let me know if you spot any other issues.

@bluepeter i think you tagged the wrong person 👍

bluepeter commented 5 years ago

Oops, sorry @akadop ... hey @akoel please see https://github.com/gatsbyjs/gatsby/issues/8237#issuecomment-457748298

kamranayub commented 5 years ago

@relytmcd's list worked for me, finally. The key was the last bullet. That replaceHydrateFunction fixed the flash of styled content, followed by permanent messed up styles.

I kind of want Gatsby to have a "SPA" build mode since I am trying to make a PWA and this might be a dealbreaker 😢 I might have to go back to CRA but I really like Gatsby's workflow and helpers for pages/data sources.

edit Actually, offline plugin is working great for me in local prod build. I'll have to test it on my domain myself before pushing it live but that's promising!

oliviertassinari commented 5 years ago

I have started working on the problem. You can expect a first-class support of Material-UI v4 with Gatsby. We gonna use https://github.com/hupe1980/gatsby-plugin-material-ui for that.

oliviertassinari commented 5 years ago

I think that we can close the issue. react-hot-loader is supported with the higher-order and hook APIs:

feb-20-2019 12-19-39

This is using Material-UI v4.0.0-alpha.1 and this example: https://github.com/mui-org/material-ui/tree/next/examples/gatsby-next.

kamranayub commented 5 years ago

Ahhhh! I can try this out asap tonight I think. Do I just use the latest plugin version?

jeffwillette commented 5 years ago

I won't have time to try it out until maybe tomorrow, but did anyone try with with a production build? I was getting flashing styles on production builds with the previous methods. I will try to check myself as soon as I can

I ask because it looks like the gif above is the dev server only

cpboyd commented 5 years ago

@oliviertassinari @deltaskelta Using the new plugin works for me in develop but not in production builds.

May also want to note in the docs about order of Gatsby plugins... Specifically that something like gatsby-plugin-layout needs to be listed AFTER gatsby-plugin-material-ui if you want to make use of the theme in styles.

cpboyd commented 5 years ago

Using the old components still works for production builds... gatsby-browser:

export const wrapRootElement = ({ element }) => {
  return (
    <MuiThemeProvider
      theme={theme}
      sheetsManager={new Map()}
    >
      {element}
    </MuiThemeProvider>
  );
}

gatsby-ssr:

import { JssProvider } from 'react-jss';
import { SheetsRegistry } from 'jss';
import { createGenerateClassName, MuiThemeProvider } from '@material-ui/core/styles';

export const wrapRootElement = ({ element, pathname }) => {
  const sheetsRegistry = new SheetsRegistry()
  globalLeak.set(pathname, sheetsRegistry)

  return (
    <JssProvider registry={sheetsRegistry}
      generateClassName={createGenerateClassName()}
    >
      <MuiThemeProvider
        theme={theme}
        sheetsManager={new Map()}
      >
        {element}
      </MuiThemeProvider>
    </JssProvider>
  );
}

So it seems like the issue might be related to the new <StylesProvider> or <ThemeProvider> component.

oliviertassinari commented 5 years ago

@cpboyd I'm not sure what new plugin you are referring to, it's not completed yet. It's hosted on mateial-ui/examples/gatsby-next, we are migrating it to https://github.com/hupe1980/gatsby-plugin-material-ui.

cpboyd commented 5 years ago

@oliviertassinari I copied the plugins directory from mateial-ui/examples/gatsby-next into my project and tried using it with the latest alpha releases of @material-ui

As noted, I had rendering issues with the production build of my site, when <MuiThemeProvider> didn't have any issues.

cpboyd commented 5 years ago

@oliviertassinari @deltaskelta I found my issue: I was importing createMuiTheme from @material-ui/core rather than from @material-ui/core/styles

After adding the /styles to the import, everything seems to work consistently (with old methods and new methods, as well as develop and build).

jeffwillette commented 5 years ago

@oliviertassinari I am confused. @material-ui 4.0.0-alpha.1 seems to have moved everything from @material-ui/styles to @material-ui/core/styles but your plugin is still importing from /styles I have not been able to get it to work on my own project because of some complications related to this I believe

nvm, this was a dumb mistake, I somehow didn't have the /styles package installed, maybe it moved in 4 alpha, but installing it explicitly got me compiling without error. I now have some styles that seem to be misapplied when using withStyles but I think I have something missing in my implementation. I'll try to track it down and report back about production builds

EDIT: @cpboyd I had the same problem, I was importing createMuiTheme from @material-ui/styles instead of @material-ui/core/styles. I am left a little bit confused as to when to use core/styles and when to use /styles as there were no breaking changes listed in the 4.0 alpha release

jeffwillette commented 5 years ago

Just tested a production build and it worked successfully! no more first paint issues

gatsbot[bot] commented 5 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

Thanks for being a part of the Gatsby community! 💪💜

gatsbot[bot] commented 5 years ago

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.

Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.

Thanks again for being part of the Gatsby community!

zjaml commented 5 years ago

This issue is still there and can be reproduced by the latest material example of gatsby. I can provide a codesandbox url if you need, since it requires some work.

ThomasJanUta commented 5 years ago

After updating to MUI 4.1.0 I have this issuee. Using the official mui plugin with gatsby as adviseed in the MUI gatsby example. Did not change injection order or anything as it was working fine in MUI 3.x.

oliviertassinari commented 5 years ago

It's a recent regression with an update of react-hot-loader.

ThomasJanUta commented 5 years ago

Temporal workaround from react-hot-loader issue:

If you use the gatsby example from the MUI project: You need to paste 

import { setConfig } from 'react-hot-loader'

setConfig({
  reloadHooks: false
})

on top of the function definition of your root layout i.e. the top layout of the plugin.

zjaml commented 5 years ago

The workaround works like a charm. Thanks @mernerp !

oliviertassinari commented 5 years ago

It's fixed in https://github.com/mui-org/material-ui/pull/16195 and will be soon released in Material-UI v4.1.1.

zjaml commented 5 years ago

You guys rock @oliviertassinari