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

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

Open
HriBB opened this Issue Sep 17, 2018 · 40 comments

Comments

Projects
None yet
@HriBB
Copy link

HriBB commented Sep 17, 2018

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

This comment has been minimized.

Copy link
Author

HriBB commented Sep 18, 2018

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

This comment has been minimized.

Copy link
Contributor

kakadiadarpan commented Sep 18, 2018

@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?

@relytmcd

This comment has been minimized.

Copy link
Contributor

relytmcd commented Sep 28, 2018

@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

This comment has been minimized.

Copy link

oliviertassinari commented Sep 29, 2018

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

This comment has been minimized.

Copy link

oliviertassinari commented Sep 29, 2018

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
  • wrapRootElement is called twice that sounds very wrong.
  • There are many React lifecycles happening with the previous stylesheet, what's going on?
@pfftdammitchris

This comment has been minimized.

Copy link

pfftdammitchris commented Oct 15, 2018

This seriously should be solved.

@HriBB

This comment has been minimized.

Copy link
Author

HriBB commented Oct 15, 2018

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

This comment has been minimized.

Copy link

oliviertassinari commented Oct 31, 2018

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

@hennessyevan hennessyevan referenced this issue Oct 31, 2018

Closed

Use it on GatsbyJS? #10

4 of 4 tasks complete
@axe312ger

This comment has been minimized.

Copy link
Member

axe312ger commented Nov 3, 2018

I fiddled around, thats what I found:

  • wrapRootElement and the MuiThemeProvider seem to be unrelated to hot module reloading not working. Plain withStyles() without theme do not trigger a HMR as well.
  • DOM wise:
  • I tried gatsby-plugin-jss and it's example in the gatsby repo. Here HMR is broken as well, but the behavior is mirrored: New class names are set for the components, the new styles are not injected in the DOM.
  • It seems like, after the HMR, the old sheetManager is still used to get the class names for the component. A new sheetManager exists in sheetsManager with the new styles applied, but that one is not used. Thats why nothing changes, the css is there but the class names are not updated. Probably this line: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/styles/withStyles.js#L195
  • Thewhich results in stylesCreatorSaved staying the same value all the time
  • componentDidUpdate is not called at all, the HoC is actually reconstructed and remounted every time the styles get updated. I guess here lies the problem.
  • Maybe this is related or could help: mui-org/material-ui#13465

I hope this helps @HriBB :)

@cpboyd

This comment has been minimized.

Copy link
Contributor

cpboyd commented Nov 14, 2018

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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Nov 15, 2018

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

@relytmcd

This comment has been minimized.

Copy link
Contributor

relytmcd commented Dec 20, 2018

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.

export const replaceHydrateFunction = () => {
  return (element, container, callback) => {
    ReactDOM.render(element, container, callback)
  }
}

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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Jan 3, 2019

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

This comment has been minimized.

Copy link

walleXD commented Jan 7, 2019

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

This comment has been minimized.

Copy link
Member

axe312ger commented Jan 9, 2019

@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 🤷‍♂️)

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Jan 11, 2019

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

This comment has been minimized.

Copy link
Contributor

bluepeter commented Jan 19, 2019

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

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Jan 20, 2019

@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

This comment has been minimized.

Copy link
Contributor

bluepeter commented Jan 20, 2019

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).

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Jan 21, 2019

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

This comment has been minimized.

Copy link
Contributor

bluepeter commented Jan 21, 2019

@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.

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Jan 21, 2019

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

This comment has been minimized.

Copy link

akoel commented Jan 24, 2019

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

This comment has been minimized.

Copy link
Contributor

bluepeter commented Jan 25, 2019

@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

This comment has been minimized.

Copy link
Contributor

akadop commented Jan 25, 2019

@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

This comment has been minimized.

Copy link
Contributor

bluepeter commented Jan 25, 2019

Oops, sorry @akadop ... hey @akoel please see #8237 (comment)

@kamranayub

This comment has been minimized.

Copy link

kamranayub commented Feb 3, 2019

@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

This comment has been minimized.

Copy link

oliviertassinari commented Feb 19, 2019

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

This comment has been minimized.

Copy link

oliviertassinari commented Feb 20, 2019

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

This comment has been minimized.

Copy link

kamranayub commented Feb 20, 2019

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

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Feb 21, 2019

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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Feb 23, 2019

@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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Feb 23, 2019

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

This comment has been minimized.

Copy link

oliviertassinari commented Feb 24, 2019

@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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Feb 25, 2019

@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

This comment has been minimized.

Copy link
Contributor

cpboyd commented Feb 25, 2019

@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).

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Feb 26, 2019

@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

@deltaskelta

This comment has been minimized.

Copy link
Contributor

deltaskelta commented Feb 26, 2019

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

@gatsbot

This comment has been minimized.

Copy link

gatsbot bot commented Mar 19, 2019

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 gatsbot bot added the stale? label Mar 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.