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

gatsby-develop | useState from react hooks doesn't work in development environment #9489

Closed
aamorozov opened this issue Oct 28, 2018 · 37 comments · Fixed by #10259
Closed

gatsby-develop | useState from react hooks doesn't work in development environment #9489

aamorozov opened this issue Oct 28, 2018 · 37 comments · Fixed by #10259
Labels
type: upstream Issues outside of Gatsby's control, caused by dependencies

Comments

@aamorozov
Copy link

aamorozov commented Oct 28, 2018

Description

When running dev server, the state from react hooks is not working without any errors. It works, however, in production after compiling the files.

Steps to reproduce

  1. Pull down https://github.com/aamorozov/gatsby-react-hooks;
  2. Run yarn && gatsby develop;
  3. When dev server is running, click on the button - the state is not getting updated here;
  4. Run gatsby build && gatsby serve
  5. When prod server is running, click on the button - the state is being updated correctly.

Expected result

The state from react-hooks should work in both dev/prod builds.

Actual result

The state from react-hooks is working only in production build.

Environment

System:
    OS: macOS 10.14
    CPU: x64 Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
    Shell: 3.2.57 - /bin/bash
  Binaries:
    Node: 11.0.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: 62.0.3
    Safari: 12.0
  npmPackages:
    gatsby: ^2.0.19 => 2.0.28
    gatsby-plugin-flow: ^1.0.2 => 1.0.2
    gatsby-plugin-jsxstyle: ^0.0.3 => 0.0.3
    gatsby-plugin-manifest: ^2.0.5 => 2.0.5
    gatsby-plugin-offline: ^2.0.5 => 2.0.7
    gatsby-plugin-react-helmet: ^3.0.0 => 3.0.0
  npmGlobalPackages:
    gatsby-cli: 2.4.3
@thorn0
Copy link
Contributor

thorn0 commented Oct 28, 2018

Caused by gaearon/react-hot-loader#1088

@thorn0 thorn0 added the type: upstream Issues outside of Gatsby's control, caused by dependencies label Oct 28, 2018
@ng-hai
Copy link
Contributor

ng-hai commented Oct 29, 2018

Hey, I wrap the function with React.memo() and it works 😆

@theKashey
Copy link
Contributor

theKashey commented Oct 29, 2018

The more proper fix

import { cold } from 'react-hot-loader';
cold(MyComponent); // black list MyComponent. But state would be lost on update. 

@wKovacs64
Copy link
Contributor

wKovacs64 commented Oct 29, 2018

Hmm, cold(MyComponent) seems to get around potential Hooks can only be called inside the body of a function component errors, but calling the setter function returned from useState is not triggering a re-render when it should. The same component re-renders as expected when wrapped in React.memo instead (understood that it's not the fix, just noting the differences).

Edit: here's a reproduction - maybe I'm just doing something dumb?

https://codesandbox.io/s/1zvrvq5k5j

@jlengstorf
Copy link
Contributor

Potential automated fix? gaearon/react-hot-loader#1088 (comment)

@wKovacs64
Copy link
Contributor

The setConfig approach yields the same result as far as I can tell (not re-rendering when it should). 🤷‍♂️

@wKovacs64
Copy link
Contributor

wKovacs64 commented Oct 31, 2018

This works, pending approval from @theKashey (approved):

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

setConfig({ pureSFC: true });

Credit: gaearon/react-hot-loader#1088 (comment)

@jlengstorf
Copy link
Contributor

Yeah! Just confirmed it by modifying your original Codesandbox: https://codesandbox.io/s/rjqy5y6q3n

@theKashey
Copy link
Contributor

Enabling pureSFC is a good option. It affects the ability of RHL to forceUpdate result, so I could not guarantee that after HMR everything should be ok.
Plus RHL internal will still fail to manage hooks, but this would not a fatal error.

jlengstorf added a commit to jlengstorf/gatsby-with-hooks that referenced this issue Nov 2, 2018
Add the `setConfig` fix from gatsbyjs/gatsby#9489 (comment) to get Hooks working in develop mode using hot module reloading. This may cause issues where the data gets out of sync, but it’s the best we can hope for right now.
@ooloth
Copy link
Contributor

ooloth commented Nov 19, 2018

The pureSFC solution is great except that it causes react-spring to throw the following error:

Function components cannot be given refs. Attempts to access this ref will fail.

Check the render method of `AnimatedComponent`.

As a result, I've had to switch back to wrapping functions that use hooks with React.memo().

Thought I'd share in case any decisions are being made about how to permanently enable hooks for Gatsby users.

@jlengstorf
Copy link
Contributor

Thanks for the heads up, @ooloth!

This isn’t an Official Statement™ by any means, but I‘d call it a safe bet to assume that Gatsby won’t ship any hacks to get Hooks working.

This issue will end up getting fixed upstream, and once there‘s a stable solution for hot reloading with Hooks — assuming it’s backward-compatible — we’ll bump the dependency and Hooks will Just Work.

In the meantime, the hacks to get Hooks support will need to be implemented on a per-project basis by the devs wanting to use them.

Hope that helps clear things up!

@theKashey
Copy link
Contributor

This thing was broken by RHL, and should be fixed by RHL.
Beta version 4.5.1 - could solve everything, once you will apply RHL's webpack-loader to your node_modules.
https://github.com/gaearon/react-hot-loader/blob/7c0f9a04bce89801d387b277251c2b00da525534/README.md#-hot-labs-

@ooloth
Copy link
Contributor

ooloth commented Dec 4, 2018

Hmm...

So, looks like Gatsby did end up deciding to ship a pureSFC hack that causes react-spring to throw the errors described in my comment above.

I'll check in with react-spring about whether this error is actually a concern in practice, but I just wanted to identify this here in the meantime.

@wKovacs64
Copy link
Contributor

wKovacs64 commented Dec 4, 2018

Is #10259 supposed to be enough? It still doesn't work for me unless I call setConfig myself.

gatsby@2.0.62
react-hot-loader@4.5.1

@DSchau
Copy link
Contributor

DSchau commented Dec 4, 2018

@wKovacs64 yeah - you can check out my repo which seems to work just fine for me.

Does it not work for you?

https://github.com/dschau/gatsby-react-hooks

@DSchau
Copy link
Contributor

DSchau commented Dec 4, 2018

@ooloth whoops; sorry about that! I suppose a positive thing (for now) is that the error can be safely ignored as it's not impacting functionality.

It does sound like an issue with react-spring (or perhaps RHL), unfortunately, so let us know how we can help resolve that!

@wKovacs64
Copy link
Contributor

wKovacs64 commented Dec 4, 2018

@wKovacs64 yeah - you can check out my repo which seems to work just fine for me.

Does it not work for you?

https://github.com/dschau/gatsby-react-hooks

OK so it works alone, but not in my app. Sigh. 🙄 Will continue to narrow it down.

@ooloth
Copy link
Contributor

ooloth commented Dec 4, 2018

@DSchau Yeah, it's probably fine!

Always mildly alarming to see the console turn red lol, so just wanted to check. 😳

@wKovacs64
Copy link
Contributor

wKovacs64 commented Dec 4, 2018

@DSchau It appears hooks aren't working in pages/*, as odd as that sounds. Check out my fork of your example repo where I track the counter state directly in page-2.js (edit: fork deleted now that it's been resolved).

@wKovacs64
Copy link
Contributor

@DSchau I know nothing about the cache-dir portion of Gatsby internals, so I don't know if this is the right place for it or not, but if I move it from app.js to gatsby-browser-entry.js (and make Anton's suggested change while doing so), it seems to fix things.

gatsby-browser-entry.js:

setConfig({
  pureSFC: true,
  pureRender: true,
})

@DSchau
Copy link
Contributor

DSchau commented Dec 5, 2018

@wKovacs64 you're right--that does fix it! We won't want to go that way as we want this to be a develop only feature. I'm looking into this now!

@ooloth
Copy link
Contributor

ooloth commented Dec 6, 2018

Apparently, the forwardRef error I reported above was a bug in react-hot-loader which has been fixed (along with a few other issues) in react-hot-loader@4.5.2.

@wKovacs64
Copy link
Contributor

@wKovacs64 you're right--that does fix it! We won't want to go that way as we want this to be a develop only feature. I'm looking into this now!

Any word on this, @DSchau?

@theKashey
Copy link
Contributor

React-Hot-Loader 4.6.0 should work with hooks out of the box.
https://medium.com/@antonkorzunov/react-hot-loader-4-6-41f3ce76fb08

@DSchau
Copy link
Contributor

DSchau commented Dec 13, 2018

@theKashey that's excellent! I'll check this out later this afternoon unless someone wants to jump on this :)

@jgierer12
Copy link
Contributor

@DSchau Upgrading doesn't look too complicated. I will open a PR soon if you don't mind 😃

@wKovacs64
Copy link
Contributor

@jgierer12 I was just about to push it, but you can if you'd rather. :)

@DSchau
Copy link
Contributor

DSchau commented Dec 13, 2018

@jgierer12 not only do I not mind, that would be great :) Please do!

@wKovacs64 @jgierer12 either of you feel free to open, and just reference this issue where we can all take a look!

@wKovacs64
Copy link
Contributor

Go for it, @jgierer12! Get you some Gatsby swag. 😉

@jgierer12
Copy link
Contributor

jgierer12 commented Dec 13, 2018

#10455 opened

@wKovacs64 I already got some - but thanks anyways 💜

DSchau pushed a commit that referenced this issue Dec 17, 2018
Fixes #9489 

~~The docs also [recommend](https://github.com/gaearon/react-hot-loader#react--dom) (but don't require) using a patched `react-dom`, but as I understand it that would override the user's `react-dom` version we have as a peer dependency so I left it out.~~ ➡️ 3894f0a

/cc @wKovacs64
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this issue Jan 22, 2019
Fixes gatsbyjs#9489 

~~The docs also [recommend](https://github.com/gaearon/react-hot-loader#react--dom) (but don't require) using a patched `react-dom`, but as I understand it that would override the user's `react-dom` version we have as a peer dependency so I left it out.~~ ➡️ gatsbyjs@3894f0a

/cc @wKovacs64
@m4rrc0
Copy link
Contributor

m4rrc0 commented Feb 12, 2019

Which version of Gatsby is this fix applied to?
I still see Hooks can only be called inside the body of a function component with Gatsby@^2.0.118

@jgierer12
Copy link
Contributor

@MarcCoet this was published in gatsby@2.0.69. If you are still experiencing issues with hooks I recommend you open a new issue and provide reproduction steps and the output of gatsby info --clipboard.

@m4rrc0
Copy link
Contributor

m4rrc0 commented Feb 12, 2019

Thanks @jgierer12 . That is what I thought. I will dig a bit more.
That is probably just me doing silly stuff.

@jlengstorf
Copy link
Contributor

@MarcCoet are you using yarn link by chance? I ran into that issue and it only happens to me when I linked packages.

@jlengstorf
Copy link
Contributor

Apparently this is a known and expected bug? facebook/react#14257

@m4rrc0
Copy link
Contributor

m4rrc0 commented Feb 13, 2019

@jlengstorf nope, not on that project but that is really good to know, thanks !!
I don't think anyone is likely to make the same mistake I did but just in case... I was messing around with architecture and data transformation and I happened to transform a React Function Component into a simple function. Obviously enough, Hooks are only meant to be used in React Components and not normal functions but the error was a bit misleading and hooks are so small and ...well, functional... that I forgot it was there. ;)
So everything is fine for me. Thanks again.

@Psycarlo
Copy link

I was using useState and getting this problem.
I used useMemo instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: upstream Issues outside of Gatsby's control, caused by dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.