nice-modal-react
nice-modal-react copied to clipboard
returned object from useModal hook have "unstable" identity
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
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
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.
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.
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 did you manage to make react-hook-form work with modals in react-nice-modal?
@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.
should be resolved in v1.2.11
.
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.
@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.