Hot reloading CSS broken when component lives in wrapPageElement

[wesbos]

Description

I'm using Styled Components (and the gatsby plugin for it) but I can confirm this is an issue with CSS modules as well.

Hot reloading of CSS works great when you are editing a component on a page.

However if you use the wrapPageElement or wrapRootElement API in gatsby-browser.js and have a styled component that lives in that layout, it's reloads the whole page - losing at state you might have.

export function wrapPageElement({ element, props }) {
  return <Layout {...props}>{element}</Layout>;
}

So for example, if I have a Layout component:

export default function Layout({ children }) {
  return (
    <>
      <div>
          <Nav></Nav>
          {children}
      </div>
    </>
  );
}

And then either in Layout.js directry, or inside of another component - like Nav - I have my styled component:

const NavStyles = styled.nav`
  background: green;
`;

Now if I change green to blue. The whole page reloads.

If I were to move that Nav component out of Layout.js and into a page like pages/index.js, it hot reloads fine without reloading the page.

Environment

This happens in both Firefox and Chrome

System:
OS: macOS 10.15.2
CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 13.9.0 - /usr/local/bin/node
Yarn: 1.22.0 - /usr/local/bin/yarn
npm: 6.13.7 - /usr/local/bin/npm
Languages:
Python: 2.7.15 - /usr/local/bin/python
Browsers:
Chrome: 81.0.4044.122
Firefox: 59.0.1
Safari: 13.0.4
npmGlobalPackages:
gatsby: 2.15.14

[wesbos]

Some more info:

I tried to just manually wrap every page in <Layout> and the issue persists.

I think this might not be related to wrapPageLayout, but nested components?

[pieh]

Are you sure about case of wrapping every page in <Layout> in this not working?

I can for sure reproduce this problem when using wrapPageElement (and wrapRootElement), but it worked fine when manually wrapping pages with layouts.

Just so there is no misunderstandings here - way I understand manually wrapping means to me something like we do in default starter - https://github.com/gatsbyjs/gatsby-starter-default/blob/95a4d12f3341460c638dde2fbf13c5f2c9b04cdc/src/pages/index.js#L9-L18 where each page uses <Layout> to wrap its content - is this what you tried or something else?

[pieh]

Can you also try running GATSBY_HOT_LOADER=fast-refresh gatsby develop and see how hot reloading works there? (that is available from [email protected] onwards)

This will use react-refresh (over default react-hot-loader) and it seems like it handles it better (for wrapPageElement parts etc). It's still considered experimental ( https://www.npmjs.com/package/react-refresh ) and requires more recent version of react than what gatsby@2 have as minimal so we can't really make that default yet, but we hope to switch to it in the future.

[chocobuckle]

@wesbos @pieh I was having the same problem and running GATSBY_HOT_LOADER=fast-refresh gatsby develop solved it, for me anyway. Would be nice to figure out what the cause of the original issue was though! ¯_(ツ)_/¯

[pieh]

Would be nice to figure out what the cause of the original issue was though! ¯_(ツ)_/¯

It appears to me that with default module hot reloading (using react-hot-loader) we wrap only page templates with hot -

gatsby/packages/gatsby/src/bootstrap/requires-writer.js

Lines 148 to 170 in fbdb4de

	// TODO: Remove all "hot" references in this `syncRequires` variable when fast-refresh is the default
	const hotImport =
	process.env.GATSBY_HOT_LOADER !== `fast-refresh`
	? `const { hot } = require("react-hot-loader/root")`
	: ``
	const hotMethod =
	process.env.GATSBY_HOT_LOADER !== `fast-refresh` ? `hot` : ``
	// Create file with sync requires of components/json files.
	let syncRequires = `${hotImport}
	// prefer default export if available
	const preferDefault = m => m && m.default || m
	\n\n`
	syncRequires += `exports.components = {\n${components
	.map(
	c =>
	` "${
	c.componentChunkName
	}": ${hotMethod}(preferDefault(require("${joinPath(c.component)}")))`
	)
	.join(`,\n`)}
	}\n\n`

The wrapPageElement and wrapRootElement are above those in component hierarchy, so when making changes to those, there is nothing that can auto-accept changes and then webpack fallback to automatic browser refreshes. If you tick Preserve log in dev tools and apply changes you should see something similar as:

I have tracked when we started applying hot to page templates only to #10455 , (it moved it from wrapping Root of application to wraping page templates). I don't exactly understand why this was done (what was it fixing), so it's bit dangerous to move the hot back to Root without understanding reason that it was moved. I will try it tho and if it will "just work" and I won't immediately find problems will release canary for it and open PR. @theKashey (maintainer of react-hot-loader) made quite a bit of comments about the move there that I need to check on and try to understand.

It seems to work with react-refresh/fast-refresh because this approach doesn't require wrapping with hot

[wesbos]

Great - thanks for the explanation.

I tried once again with just wrapping the page in my <Layout> component, and it still refreshes with both css modules and styled components.

Switching to fast refresh does fix the issue though! SO that is a good solution

[wesbos]

Do you know if I can stick GATSBY_HOT_LOADER=fast-refresh in an .env file? I've tried .env and .env.development, but both are ignored. Not sure how well sticking it in package.json will work for windows users

[gaearon]

Is there a plan to switch to Fast Refresh by default? React Hot Loader is extremely fragile and I hope we can phase it out asap.

[pieh]

Do you know if I can stick GATSBY_HOT_LOADER=fast-refresh in an .env file? I've tried .env and .env.development, but both are ignored. Not sure how well sticking it in package.json will work for windows users

Unfortunately this will likely not work right now. We are aware of this and do plan improvement and streamlining those for next major - #21004 but those will need breaking changes so those are reserver for v3.

Is there a plan to switch to Fast Refresh by default? React Hot Loader is extremely fragile and I hope we can phase it out asap.

Yes! We just can't do it in version 2 of Gatsby, because it still supports react/react-dom@>=16.4.2 and while we were researching Fast Refresh we found in some of comments that it needs at least 16.10 (or so, I don't remember right now). This is planned for next major as well. I personally am also bit weary of disclaimer that currently there is in README for react-refresh:

This is an experimental package for hot reloading.

Its API is not as stable as that of React, React Native, or React DOM, and does not follow the common versioning scheme.

Use it at your own risk.

So we do hope that timing will align and react-refresh will be marked as stable before/during our v3 work.

[wesbos]

Shoot it seems like the new hot reloading works for CSS, but is broken for my react components. It gives me:

Ignored an update to unaccepted module ./src/pages/index.js -> ./.cache/sync-requires.js -> ./.cache/app.js -> 0

[HMR] The following modules couldn't be hot updated: (Full reload needed)
This is usually because the modules which have changed (and their parents) do not know how to hot reload themselves. See https://webpack.js.org/concepts/hot-module-replacement/ for more details.

[HMR]  - ./src/pages/index.js 

And doesn't even refresh the page.

[gaearon]

What does your ./src/pages/index.js look like?

[gaearon]

I personally am also bit weary of disclaimer that currently there is in README for react-refresh

That's just a template we copy paste in all new packages. It's been used in React Native for almost a year now and is stable.

The part that needs work is how it's integrated. I'm assuming you are doing this via the current webpack plugin? That plugin needs more work to be good.

[wesbos]

@gaearon - looks like a restart solved it for now - will keep an eye on what causes if it if happens again

[chocobuckle]

@wesbos Are you using wrapPageElement or gatsby-plugin-layout? With wrapPageElement react-refresh/fash-refresh is working without problems for me, but I haven't tried it on a gatsby-plugin-layout project.

[wesbos]

I'm using wrapPageElement

[gaearon]

Updated react-refresh README. facebook/react@5b89d35

[chocobuckle]

Ah, you sorted it out, good stuff. The ol' plug it-out-and-plug-it-back-in-again trick! :)

[pieh]

There is some bugginess with wrapPageElement still that need to be solved (that is not related even to react-hot-loader). From my checks what I discovered is that we manually call those instead of using React.createElement on them so this might trip things out and it might cause "heisenbugs" mentioned that sometimes it work and sometimes it doesn't. On my first try with react-refresh it didn't work, but then I tried again after trying different approaches and then it worked. I assumed I messed up my initial try, but maybe there is just some weird things happening "randomly" (might be related to calling functions instead of using React.createElement - or might be completely unrelated)

The part that needs work is how it's integrated. I'm assuming you are doing this via the current webpack plugin? That plugin needs more work to be good.

We use @pmmmwh/react-refresh-webpack-plugin right now - you can check #21534 for how it's integrated in Gatsby. Also /cc @blainekasten who did integration as well as some research and fixes in the webpack plugin itself prior to getting support in Gatsby.

[gaearon]

You might want to track pmmmwh/react-refresh-webpack-plugin#75. @Timer has forked the plugin for Next, using a new webpack feature (which was also backported into 4.x) to improve the resilience. We need to get those changes merged back into the react-refresh-webpack-plugin. Then we want to upstream the plugin itself into React repo.

[blainekasten]

@gaearon I'm absolutely tracking all of this :) Here's our rough plan of action:

• take part in stabilizing the refresh plugin ecosystem and keep it incorporated with gatsby
• Gatsby V3 will raise the bar for minimum react version, which allows us to make fast-refresh the default dev experience
• continue to support hot-loader until preact is compatible with fast-refresh

Since fast-refresh should fix these rough edges for us, I don't think it's worth our effort fixing this in hot-loader as it will eventually be fully replaced.

[theKashey]

Oh. I am waiting for this for the half year.

[github-actions]

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!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

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

[wesbos]

Still an issue, can be closed if Gatsby ships fast-refresh as default and this doesn't need fixing

[blainekasten]

Thanks @wesbos! It should be the default in v3