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

useImperativeHandle behaves as if inputs are [] by default #14782

Closed
AhmedElywa opened this issue Feb 7, 2019 · 15 comments
Closed

useImperativeHandle behaves as if inputs are [] by default #14782

AhmedElywa opened this issue Feb 7, 2019 · 15 comments

Comments

@AhmedElywa
Copy link

useImperativeHandle have bug in 16.8.1 if you want to access any state will get the initial state not current state but in 16.8.0-alpha.1 it work good

You can see here https://codesandbox.io/s/xjl8znwpz open console when you change input value will get the currently value in console but if you clicked button get value from ref will get initial value (e)

If you try same here https://codesandbox.io/s/qxkll6po0j with 16.8.0-alpha.1 when clicked button get currently value

@danielkcz
Copy link

danielkcz commented Feb 7, 2019

That's a pretty common "bug" caused by Javascript closures. React apparently is not recreating handle on every render so it will always see only the value from the first render.

You can work around that by using ref: https://codesandbox.io/s/8nr4x36zy9

Personally, I would recommend you to avoid useImperativeHandle. It should be used sparsely for a reason. There are definitely better ways for a data flow.

@danielkcz
Copy link

I think this kind of problems should get its own section in the docs. Many people will get burned by this (me included) because it's just not that apparent without knowing this advanced JS behavior.

@AhmedElywa
Copy link
Author

But why work in alpha version?

@AhmedElywa
Copy link
Author

@FredyC I’m not use it to transfer value this just example
I use here https://github.com/oahtech/rnsc/blob/master/src/components/Accordion/index.js

To control the state of expanded or collapsed

@danielkcz
Copy link

danielkcz commented Feb 7, 2019

I think this is a mindset issue, possibly coming from how jQuery does the things. You shouldn't be controlling components by calling methods on them. Instead, you should pass props that will change that state. I don't have a capacity to update that example of yours right now. Just try to fiddle with it and I am sure you will figure a way without imperative access.

@MicahChalmer
Copy link

MicahChalmer commented Feb 7, 2019

There's a third parameter to useImperativeHandle called inputs which is intended to be used in the same way as similar parameters to useEffect and useMemo--if you pass an array to it, it will re-create the handle whenever any of the inputs have changed since the last call. I find that if I pass in what the closure I'm passing depends on, it works as expected.

The bug here (and I do think it's a bug) is this: unlike the other hooks that have an inputs parameter (and unlike the previous behavior from the alphas, which I still think is the correct behavior,) if you don't pass inputs it defaults to ignoring the new closure you passed in. It defaults to behaving as if you had passed [] to inputs. The others default to always calling it, which I believe is the right thing to do, for precisely the reason that it avoids this gotcha around values saved from previous renders. The current behavior is "memoized by default" and that's wrong for the same reasons explained in other contexts.

This does mean there's an easy workaround for @AhmedElywa or anyone else hitting this issue: just pass something to inputs that always changes--[Symbol()] would do the trick. Or look at what you're passing and list what the real inputs are, like you would to optimize a useMemo call.

@MicahChalmer
Copy link

Here's the first example as updated with workaround: https://codesandbox.io/s/nnrlpxx1qm - note line 13 of Test.js. With that it shows the current value as expected.

@hamlim
Copy link
Contributor

hamlim commented Feb 7, 2019

The third argument to useImperativeHandle is a dependency array much like useEffect, useMemo and useCallback's second argument. In this case you need to provide [value] as the third argument to have the second argument (i.e. ref assignment function) be called again when the value changes:

useImperativeHandle(
  ref,
  () => ({
    getValue: () => {
      console.log(value);
    }
  }),
  [value]
);

Working sandbox here: https://codesandbox.io/s/zl8zy0l6jx

@AhmedElywa
Copy link
Author

@MicahChalmer and @hamlim thanks for explain

@danielkcz
Copy link

@hamlim I think that @MicahChalmer was pointing out correctly, that unlike the useEffect which is executed on each render when no inputs are passed, the useImperativeHandle on the contrary does not update without inputs passed it. It's a slight inconsistency in the API and would worth explaining in the docs at least.

@hamlim
Copy link
Contributor

hamlim commented Feb 8, 2019

Oh wow I must have completely skipped over @MicahChalmer's comments above mine 🤦‍♂️ sorry about that!

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

It defaults to behaving as if you had passed [] to inputs.

Sounds like a bug.

@gaearon gaearon changed the title useImperativeHandle not access the current state in 16.8.1 useImperativeHandle behaves as if inputs are [] by default Feb 8, 2019
@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

#14801

@gaearon
Copy link
Collaborator

gaearon commented Feb 13, 2019

Canary 0.0.0-0e4135e8c should have the fix. This will make it into 16.8.2.

@gaearon
Copy link
Collaborator

gaearon commented Feb 14, 2019

Fixed in 16.8.2.
https://codesandbox.io/s/7k7x17rkpj

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

5 participants