react-native-outside-press icon indicating copy to clipboard operation
react-native-outside-press copied to clipboard

Warning: Maximum update depth exceeded.

Open elenitaex5 opened this issue 2 years ago • 7 comments

Hi! I found your library and seems to works well, but there is an issue in react-native-web (running with EXPO and webpack) As soon as OutsidePressHandler is added it starts warning like below:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.

image
<>
... 
  <OutsidePressHandler onOutsidePress={closeAttachmentsMenu} disabled={!isAttachmentsMenuOpen}>
        <AttachmentsMenu isOpen={isAttachmentsMenuOpen} />
  </OutsidePressHandler>
  <ActionBar />
</>

functionality is ok, but I thing there something missing in the library.

elenitaex5 avatar Oct 03 '23 13:10 elenitaex5

I reopen the issue is happening

elenitaex5 avatar Oct 03 '23 13:10 elenitaex5

@elenitaex5 Can you create a demo snack? https://snack.expo.dev/

dcangulo avatar Oct 04 '23 01:10 dcangulo

I ran into the same issue. I was not able to create a repro yet and it's late but I narrowed down the problem in my application:

If onOutsidePress is not memoized (you just pass an anonymous function) then for some reason that I haven't found yet (probably some interaction between components and/or RNW) the OutsidePressHandler hook runs into an infinite loop and starts re-rendering like crazy. Specifically this hook:

  useEffect(() => {
    appendEvent({
      id,
      onOutsidePress,
      disabled
    });
    return () => removeEvent(id);
  }, [onOutsidePress, disabled]);

I'll try to figure out what exactly is causing the render loop tomorrow. An obvious workaround is to wrap the function passed to onOutsidePress in useCallback which prevents the effect from retriggering

d4rky-pl avatar Oct 09 '23 18:10 d4rky-pl

I'm creating a snack but there's a problem with a dependency in the snack. I include it as soon as I have it.

elenitaex5 avatar Oct 09 '23 19:10 elenitaex5

I won't be able to create a Snack for you because reproducing this issue in Snack causes the browser to crash due to how Snack works 😅

Here's a repro of the issue, or at least how the issue was triggered in my case: https://github.com/d4rky-pl/outside-press-bug

How does it happen? There are two crucial issues that both need to happen at the same time:

  • The EventProvider must render a component that re-renders its children
  • The OutsidePressHandler must be passed an anonymous function

In my case the bug was in my own SessionProvider which was passing unmemoized functions down the tree. This normally would not trigger an issue since it's a provider, it should not re-render too often but EventProvider re-renders entire tree every time a new OutsidePressHandler is registered or existing one registers with a new function. This caused the following loop:

  1. OutsidePressHandler registers an event in EventProvider
  2. This causes EventProvider to call setState and re-render its tree
  3. SessionProvider re-renders and passes new anonymous functions to the context, triggering subsequent re-render of its children
  4. This once again re-renders OutsidePressHandler which compares previous passed function with the new one, sees they're not identical, deregisters and registers again in EventProvider
  5. Go back to 2.

I'm not entirely sure if this should be considered a bug in the library or a bug in the custom code and if a fix or a warning and better documentation is required. I already spent few hours on debugging it and ran out of allotted time to continue so I didn't check if the setState in EventProvider is required for the library to work (is re-rendering the tree a crucial part of how the library works or is it an accidental side effect from using the hooks?). One way could be to detect that the function passed to OutsidePressHandler has changed on re-render and warn that this may cause issues.

For now the workaround is to make sure EventProvider is not rendering children that may run into a render loop, either by fixing them or by moving EventProvider deeper (in my case moving it to inside SessionProvider also helped)

d4rky-pl avatar Oct 10 '23 09:10 d4rky-pl

Same for me. Like said upper, I wrapped my onOutsidePress function in a useCallback and it works. Thanks @d4rky-pl !

<OutsidePressHandler
  onOutsidePress={useCallback(() => setFocused(false), [])}
  disabled={disabled}
>
...
</OutsidePressHandler>

rmarquois avatar Jan 11 '24 11:01 rmarquois