eBay / nice-modal-react

A modal state manager for React.
https://ebay.github.io/nice-modal-react
MIT License
2.04k stars 118 forks source link

memoize returned callbacks of useModal #57

Closed janczizikow closed 2 years ago

janczizikow commented 2 years ago

Problem

Issue described in #53 is still present when calling the show handler with arguments.

The following works as desired after #54:

const [counter, setCounter] = React.useState(0);
const userModal = useModal(UserInfoModal);
const { show } = userModal; 

React.useEffect(() => {
  if (counter >= 2) {
    show();
  }
}, [counter, show])

The effect doesn't cause re-rendering loop as it was before. But if we call show with a non-primitive type it would again cause an infinite re-rendering loop 😢

const [counter, setCounter] = React.useState(0);
const userModal = useModal(UserInfoModal);
const { show } = userModal; 

React.useEffect(() => {
  if (counter >= 2) {
    show({ title: 'test' }); // <- passing some args to `show`
  }
}, [counter, show]) // <- show identity will change every time we call show with args

It's the same reason for it as described in #54. This time though the problem is modalInfo.args - it's an object and it will change on every render.

Solution

Fixes #55

supnate commented 2 years ago

It looks very good! Thanks @janczizikow ! Really appreciate your contribution. Published at v1.2.4.

mrudowski commented 2 years ago

Hi, so nice library!

and this fix 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]) // <- identity will change every time and we have endless loop here