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

Upgrade react-hot-loader to 4.6 #10455

Merged
merged 9 commits into from Dec 17, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/gatsby/cache-dir/app.js
@@ -1,7 +1,7 @@
import React from "react"
import ReactDOM from "react-dom"
import domReady from "domready"
import { hot, setConfig } from "react-hot-loader"
import { setConfig } from "react-hot-loader"

import socketIo from "./socketIo"
import emitter from "./emitter"
Expand Down Expand Up @@ -57,7 +57,7 @@ apiRunnerAsync(`onClientEntry`).then(() => {
loader.addDevRequires(syncRequires)

loader.getResourcesForPathname(window.location.pathname).then(() => {
let Root = hot(module)(preferDefault(require(`./root`)))
let Root = preferDefault(require(`./root`))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New /root/hot API will not stand asynchronously between the first part and the second.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that hot should stay in this module? i.e.

let Root = hot(preferDefault(require(`./root`)))

That was the reason for the original hot update was not successful error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hot or import 'react-hot-loader/root are calling module.hot.accept for the current module, thus setting self-acceptance.
It's expected that some action would take a place on module hot update immediately and in a synchronous mode. Using hot inside indirectly called function are breaking some expectations - "module" could not accept it, as long as the "real action" is not yet called.

Here - if you want to make ./root hot - you have to make IT hot. And that's was done.

The second moment is about HMR and an update propagation.
When you change a file webpack bubbles update to parent, unless parent will accept the change, then webpack will update parent and everything below it.
We had WFT-level issues if "parent" is the module with react-dom/render - everything got wiped and regenerated, making reconciliation impossible (Symbols could be updated for example).

This is not quite clear from RHL documentation, sorry. The difference is between self-accept in v4, and child-accept(ie module.hot.accept('./containers/App') in v3.

Here you might want to accept only ./root, but would accept everything.

domReady(() => {
renderer(<Root />, rootElement, () => {
apiRunner(`onInitialClientRender`)
Expand Down
3 changes: 2 additions & 1 deletion packages/gatsby/cache-dir/root.js
@@ -1,6 +1,7 @@
import React, { createElement } from "react"
import { Router } from "@reach/router"
import { ScrollContext } from "gatsby-react-router-scroll"
import { hot } from "react-hot-loader/root"
import {
shouldUpdateScroll,
init as navigationInit,
Expand Down Expand Up @@ -109,4 +110,4 @@ const WrappedRoot = apiRunner(
}
).pop()

export default () => WrappedRoot
export default hot(() => WrappedRoot)
2 changes: 1 addition & 1 deletion packages/gatsby/package.json
Expand Up @@ -102,7 +102,7 @@
"raw-loader": "^0.5.1",
"react-dev-utils": "^4.2.1",
"react-error-overlay": "^3.0.0",
"react-hot-loader": "^4.5.1",
"react-hot-loader": "^4.6.0",
"redux": "^4.0.0",
"relay-compiler": "1.5.0",
"request": "^2.85.0",
Expand Down
19 changes: 19 additions & 0 deletions packages/gatsby/src/utils/webpack-utils.js
Expand Up @@ -252,6 +252,13 @@ module.exports = async ({
}
},

reactHot: options => {
return {
options,
loader: require.resolve(`react-hot-loader/webpack`),
}
},

eslint: (schema = ``) => {
const options = eslintConfig(schema)

Expand Down Expand Up @@ -296,6 +303,18 @@ module.exports = async ({
rules.js = js
}

{
let reactHot = (options = {}) => {
return {
test: /\.jsx?$/,
include: /node_modules/,
use: [loaders.reactHot(options)],
}
}

rules.reactHot = reactHot
}

/**
* mjs loader:
* webpack 4 has issues automatically dealing with
Expand Down
1 change: 1 addition & 0 deletions packages/gatsby/src/utils/webpack.config.js
Expand Up @@ -294,6 +294,7 @@ module.exports = async (
let configRules = [
rules.mjs(),
rules.js(),
rules.reactHot(),
rules.yaml(),
rules.fonts(),
rules.images(),
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Expand Up @@ -15641,10 +15641,10 @@ react-error-overlay@^3.0.0:
resolved "https://registry.yarnpkg.com/react-error-overlay/-/react-error-overlay-3.0.0.tgz#c2bc8f4d91f1375b3dad6d75265d51cd5eeaf655"
integrity sha512-XzgvowFrwDo6TWcpJ/WTiarb9UI6lhA4PMzS7n1joK3sHfBBBOQHUc0U4u57D6DWO9vHv6lVSWx2Q/Ymfyv4hw==

react-hot-loader@^4.5.1:
version "4.5.1"
resolved "https://registry.yarnpkg.com/react-hot-loader/-/react-hot-loader-4.5.1.tgz#4a9cf136542b1db4f948ac74b31fa16a411ceebb"
integrity sha512-aqw8F0ZTCakfTXTj1n3F9vXfEKWWHEpQ3Mq9YmYvrNdHvey5RfkSGv7rz4r9TXjqVLt2mpItqydJQdhboZ/acQ==
react-hot-loader@4.6:
version "4.6.0"
resolved "https://registry.yarnpkg.com/react-hot-loader/-/react-hot-loader-4.6.0.tgz#7a556efea51a3c79e8bffdb094fbe17883a79432"
integrity sha512-ytmtbJB0RlTUqa9HnpVsoZiZ6iRsTzu+O2WovKT++f+tDYOTNZYa7OesVAE+R90e/1w/OJO4G/tw4rNSMYCjFw==
dependencies:
fast-levenshtein "^2.0.6"
global "^4.3.0"
Expand Down