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

[React Hooks] Incompatibility with react-hot-loader #1088

Closed
bkniffler opened this issue Oct 26, 2018 · 76 comments
Closed

[React Hooks] Incompatibility with react-hot-loader #1088

bkniffler opened this issue Oct 26, 2018 · 76 comments
Assignees

Comments

@bkniffler
Copy link

Description

The new react hooks API does not work with react-hot-loader. Using the example in my app with export default hot(module)(App); at the bottom of app.js, the following error occurs:

Expected behavior

Should work.

Actual behavior

Uncaught Error: Hooks can only be called inside the body of a function component.

Reproducible Demo

Dunno of a demo is needed, just ping me if you need one.

@bkniffler
Copy link
Author

Partially related
facebook/react#13972

@simmo
Copy link

simmo commented Oct 26, 2018

Yes I've just seen the same. I tried removing my hot(module)(App) equivalent but also required removing babel plugin to get hooks to work.

@theKashey
Copy link
Collaborator

Yes. RHL is 100% not compatible with hooks. There is just a few reasons behind it:

  • SFC are being converted to Class components. There is reason - to be able to forceUpdate on HMR, as long there is no "update" method on SFC. I am looking for other way of forcing the update (like this. So RHL is killing SFC.

  • "hotReplacementRender". RHL is trying to do React's job, and render the old and the new app, to merge them. So, obviously, that's broken now.

I am going to draft a PR, to mitigate both problems. It will work, but not today.

wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Oct 28, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
@harisvsulaiman
Copy link

This could be a workaround gatsbyjs/gatsby#9489 (comment)

@theKashey
Copy link
Collaborator

Next version of RHL, with 16.6 support will add memo and lazy support, and thus break this "fix".

Right now memo-ed components are invisible to RHL, as long they are not functions, but objects.

@theKashey
Copy link
Collaborator

There is a more proper fix, which would work my better and ever after - cold API

You may disable RHL for any custom type.

import { cold } from 'react-hot-loader';
cold(MyComponent);

or just search for "useState/useEffect" inside component source code, and ❄️it.

import { setConfig, cold } from 'react-hot-loader'
setConfig({
  onComponentRegister: (type, name, file) =>
    (String(type).indexOf('useState') > 0 ||  String(type).indexOf('useEffect') > 0) && cold(type),
})

@theKashey
Copy link
Collaborator

Meanwhile, everything depends on this - reactjs/rfcs#74

wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Oct 30, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
@pedronauck
Copy link

pedronauck commented Oct 31, 2018

I just set pureSFC and everything works fine, this can be a problem for other pieces @theKashey?

setConfig({ pureSFC: true })

@painedpineapple
Copy link

Using setConfig({ pureSFC: true }) does it for me. Even though I get the error below, it still hot-reloads even the components using hooks.

index.js:1452 React-hot-loader: reconciliation failed could not dive into [ ƒ children(props) {
          var child = _children(item, state, i);

          return child ? child(props) : null;
        } ] while some elements are still present in the tree.

@theKashey
Copy link
Collaborator

theKashey commented Nov 1, 2018

Probably pureSFC is a good option. Just

  • don't wrap SFC with hot - top component should be a component to be able to update nested tree on HMR.
  • don't define contextTypes on it (not a problem for hooks)
  • it would probably fail to reconcile updates, and will generate some error in "hot-render", and then continue from the next renderable component.

So - just don't use "hidden" components (ie not exported as a top level variables) - and that would be ok.
Also - there could be a problem with force update.

@theKashey
Copy link
Collaborator

PS: While we found a way to handle hooks, React.memo and lazy is not yet supported
(#1084)

@KyleAMathews
Copy link

Who's gonna write the Babel plugin which auto adds this if it detects a hook import? 😘

@theKashey
Copy link
Collaborator

Probably me. I already was used to introduce per-component settings to support React.memo, and the same would work for hooks.
I'll release a new version tomorrow, with memo and hooks 🤞, but, probably, without React.Lazy support.

@theKashey
Copy link
Collaborator

Styled-components v4 also reported not to work. One more thing to manage.

wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Nov 7, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Nov 7, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Nov 8, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
wKovacs64 added a commit to wKovacs64/drinks-gatsby that referenced this issue Nov 9, 2018
N.B. Does not currently work in Gatsby development mode due to
incompatibilities with react-hot-loader. See RHL issue 1088.

gaearon/react-hot-loader#1088 (comment)
@theKashey
Copy link
Collaborator

v4.5.0(next) solves most of 16.6/16.7 issues and could handle anything if you will got your node_modules processed by our webpack-loader.
Please give a try and report back.
PS: If someone could convert webpack loader to webpack plugin - that would be just great.

@ntucker
Copy link

ntucker commented Feb 3, 2019

import { hot } from 'react-hot-loader/root';
import { setConfig } from 'react-hot-loader';
setConfig({ pureSFC: true });
export default hot(withRouter(App), { errorBoundary: false });

Is this how I'm supposed to do it now? I also have webpack alias for the dom thing, and using the babel plugin.

@ntucker
Copy link

ntucker commented Feb 3, 2019

You might consider adding #1088 (comment) to the docs.

@ntucker
Copy link

ntucker commented Feb 3, 2019

@theKashey well i have hotreloading working perfectly without yarn link, but it is still giving that error even after doing setConfig.

https://github.com/ntucker/anansi/tree/master/examples/typescript is the project. the yarn link is for rest-hooks

@natew
Copy link
Contributor

natew commented Feb 7, 2019

Didn't see this anywhere in the comments here or other issues. I'm getting this when I change hooks in a component:

Error: Rendered fewer hooks than expected. This may be caused by an accidental early return statement

Is this a current known issue or limitation or due to an improper config?

@theKashey
Copy link
Collaborator

@natew - try changing the configuration. RHL is trying to re-render updated application, and that "custom" render is not hooks compatible. It was just failing with alpha versions, but it was not a big deal. I didn't test 16.8, and something may change.

setConfig({
disableHotRenderer: true
})

@yoution
Copy link

yoution commented Mar 20, 2019

I write a babel plugin for react hooks hot load, it wrap react hooks component with react component, https://github.com/yoution/babel-plugin-react-hooks-hot-load

@natew
Copy link
Contributor

natew commented Mar 20, 2019

@yoution so this basically helps hooks HMR without the errors you see if you change them?

@theKashey
Copy link
Collaborator

theKashey commented Mar 20, 2019

@yoution - so you are wrapping every hooked-component with Component, and thus letting RHL to setup error boundaries around it.

  • 👍 would help to isolate error in hooks
  • 👎 is not forwarding refs.

I am thinking about proxy-regeneration - if an error is detected inside hooked-component (could be detected during hot-renderer, if it is enabled) - react-hot-loader will fail the comparison between the old and the new component, causing remount.

"remount", and "reset" are proven fixtures :trollface:

@yoution
Copy link

yoution commented Mar 20, 2019

@theKashey in my way, I use babel-plugin-react-transform other then react-hot-loader for hmr, but babel-plugin-react-transform does not support babel7, so I use metro-babel7-plugin-react-transform which is used by react-native hrm;as for forwarding refs, if you use refs in hook component, that may make no difference , if you want to use hooks from parent components, maybe I should add React.forwardRef in component which wrapping the hooks

@yoution
Copy link

yoution commented Mar 20, 2019

@natew in my project , it works well, what do you mean?

@theKashey
Copy link
Collaborator

React.forwardRef exists cos there is no other way to forward a ref. You will have to wrap your class with a forwardRef, to pass ref and another prop, to pass it down to the "real" component as a ref again. That would work, but does not sound good.

@h0jeZvgoxFepBQ2C
Copy link

Any updates on this?

@theKashey
Copy link
Collaborator

@h0jeZvgoxFepBQ2C:

  • Hooks are expected to work
  • RHL is in the deprecation mode
  • Please try React Fast Refresh in case of any 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests