andria-dev / react-spring-modal

Animatable and accessible modals built with react-spring
34 stars 8 forks source link

Getting this error everytime I toggle the modal #8

Closed KaushikShivam closed 3 years ago

KaushikShivam commented 4 years ago
Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in ForwardRef (at BottomModal.jsx:34)
    in div (created by ForwardRef)
    in ForwardRef (at ModalBackdrop.jsx:16)
    in ForwardRef (at BaseModal.jsx:113)
andria-dev commented 4 years ago

Can I see an example of your code that's causing this? Or better yet, a reproduction on codesandbox.io or something similar

KaushikShivam commented 4 years ago

Hey @ChrisBrownie55 Thanks for such a quick response.

I tried reproducing it in the demo that you've provided.

You will get the error as soon as you toggle the bottom modal off.

andria-dev commented 4 years ago

Thank you! I'll look into it right away

andria-dev commented 4 years ago

I'm having a hard time pinpointing what's causing the React state update on the unmounted component but by putting debuggers I can see that it's happening right after onRest() is called for the animation in <BaseModal> which is right before the useEffect call that handles setting up closing the modal when the user presses Esc. The only dependency for this is onRequestClose so I'm not sure what's happening here.

While I'm still figuring this out, you should be able to ignore this error as the error that occurs is entirely benign, I don't believe it's causing a significant if any memory leak

andria-dev commented 4 years ago

It also appears to only happen the first time you open any of the modals. After that, it never occurs again.

andria-dev commented 4 years ago

@KaushikShivam I've figured out what the problem is. It has to do with the Backdrop's transition finishing before the transition of the modal itself. This affects the built-in modals as well. I'll have to do more research to find out how to fix this.

andria-dev commented 4 years ago

Just to note @KaushikShivam this error is still harmless so no need to worry about it affecting anything.

KaushikShivam commented 4 years ago

@ChrisBrownie55 - Thank you for taking time out to fix this. I'm using it without any issues. Will look at the error myself once I am done with the current project. Thanks.

javierjulio commented 3 years ago

Would it be possible to combine the separate modal transitions with a single property or somehow using a chain? That way it could be customized by the user safely, making sure the modal is removed only when the transition/chain is finished.

andria-dev commented 3 years ago

Hi @javierjulio and @KaushikShivam, I am working on a major change that should solve this issue by consolidating all of the transition values for the Overlay and the Root of the Modal into one call to useTransition() ensuring that both transitions happen simultaneously.

andria-dev commented 3 years ago

This should be fixed with ~2.0.1~. Closing this for now.

andria-dev commented 3 years ago

You can try this out now with 2.0.7, I had some publishing issues with NPM lol. I finally figured them out.