nice-modal-react icon indicating copy to clipboard operation
nice-modal-react copied to clipboard

returned object from useModal hook have "unstable" identity

Open mrudowski opened this issue 2 years ago • 4 comments

Hi, so nice library!

and this fix for https://github.com/eBay/nice-modal-react/issues/53

just in time (I've just updated to 1.2.4) Thank you!

But could we do the same for whole object returned from useModal?

const userModal = useModal(UserInfoModal);

to make userModal referential equality all the time? so if I test it here like this:

const userModal = useModal(UserInfoModal);

React.useEffect(() => {
    console.log('endless show...');
    userModal.show();
}, [userModal]) // <- referential identity will change every time and we have endless loop here

mrudowski avatar Apr 13 '22 13:04 mrudowski

after reading this discussion at https://github.com/tannerlinsley/react-query/issues/1858 and here https://github.com/tannerlinsley/react-query/issues/1905 I have to say it's not so simple... and maybe not quite in the right direction

after react-hook-form which return "stable" object from useForm hook I assumed that this is the way. But now I dunno

mrudowski avatar Apr 13 '22 16:04 mrudowski

Right, this is something we can enhance but not that simple. For now, you can destruct the methods show, hide from userModal as dependencies to avoid loops.

supnate avatar Apr 15 '22 07:04 supnate

Just adding my two cents 😄

There are many ways to avoid the infinite re-rendering, one of which was already mentioned (destructuring).

Other options:

Pass reference to a callback in useEffect (without destructuring):

const userModal = useModal(UserInfoModal);

React.useEffect(() => {
    console.log('endless show...');
    userModal.show();
}, [userModal.show]) // <-- pass userModal.show

Pass whole object, but call the callback conditionally:

const userModal = useModal(UserInfoModal);

React.useEffect(() => {
    if (!userModal.visible) { // <-- just as an example
      console.log('endless show...');
      userModal.show();
    }
}, [userModal])

As for memoizing the return value of useModal - as you mentioned it's probably not so easy. One way that I could think of could be some kind shallow equality checks of userModal.args either in the reducer in the useModal hook 🤔 But I think that will add more complexity to the lib.

janczizikow avatar Apr 20 '22 08:04 janczizikow

Yes, of course,

but after using react-hook-form I started to question myself - what is right approach for it in react/hook world.

Because solution itself is rather easy, like here for example: https://github.com/react-hook-form/react-hook-form/blob/7c5009f5280f8192e71b39dae564e2e473701150/src/useForm.ts

// create ref
const _formControl = React.useRef<
    UseFormReturn<TFieldValues, TContext> | undefined
  >();

// update it's attributes
_formControl.current.formState = ...

// and finally return it
return _formControl.current;

But when I think more about it it's rather not as common as I thought. For example React useState will always returns new array so we always are destructuring the array to get "stable" state variable and setState method

const [ourState, setOurState] = useState();

mrudowski avatar Apr 20 '22 14:04 mrudowski

@mrudowski did you manage to make react-hook-form work with modals in react-nice-modal?

jilherme avatar Apr 13 '23 21:04 jilherme

@mrudowski did you manage to make react-hook-form work with modals in react-nice-modal?

I use it all the time without any problems. I was only wondering about returning object from any hook and the right approach to it in react/hook world.

mrudowski avatar Apr 14 '23 08:04 mrudowski

should be resolved in v1.2.11.

supnate avatar Oct 01 '23 04:10 supnate

should be resolved in v1.2.11.

@supnate does this mean that from v1.2.11... we can use the userModal inside of the effect dependencies array without having to worry about infinite re-renders? OR we would still need to destruct the methods show, hide from userModal as dependencies to avoid loops.

nik32 avatar Jan 10 '24 12:01 nik32

@janczizikow

` const userModal = useModal(UserInfoModal);

React.useEffect(() => { console.log('endless show...'); userModal.show(); }, [userModal.show]) // <-- pass userModal.show `

This wont work with prettier nor eslint. They do not allow to pass functions due to "this" binding.

Even destructuring the show function, it stills ends up on infinite loop. I think this library should use internally useCallbacks so it returns the same functions when nothing really changes, otherwise on this Issue only workarounds are proposed.

Version 1.2.23 and the loop still exists.

xeinebiu avatar Jan 22 '24 21:01 xeinebiu