Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning: Maximum update depth exceeded. #281

Open
elenitaex5 opened this issue Oct 3, 2023 · 6 comments
Open

Warning: Maximum update depth exceeded. #281

elenitaex5 opened this issue Oct 3, 2023 · 6 comments

Comments

@elenitaex5
Copy link

elenitaex5 commented Oct 3, 2023

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 elenitaex5 changed the title console.log infinite in false and Warning: Maximum update depth exceeded. Warning: Maximum update depth exceeded. Oct 3, 2023
@elenitaex5 elenitaex5 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 3, 2023
@elenitaex5
Copy link
Author

I reopen the issue is happening

@elenitaex5 elenitaex5 reopened this Oct 3, 2023
@dcangulo
Copy link
Owner

dcangulo commented Oct 4, 2023

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

@d4rky-pl
Copy link

d4rky-pl commented Oct 9, 2023

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

@elenitaex5
Copy link
Author

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.

@d4rky-pl
Copy link

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)

@rmarquois
Copy link

rmarquois commented Jan 11, 2024

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants