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

Multiple Menus rendered to page causes Maximum update depth exceeded. #3214

Open
IanVS opened this issue Dec 15, 2023 · 3 comments · Fixed by #3241
Open

Multiple Menus rendered to page causes Maximum update depth exceeded. #3214

IanVS opened this issue Dec 15, 2023 · 3 comments · Fixed by #3241

Comments

@IanVS
Copy link

IanVS commented Dec 15, 2023

Current behavior

In my app, I have a table and each row renders a kabab menu. I noticed recently that when there are more than ~20 rows, I get a warning in my browser console during dev:

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.

The exact number of menus needed seems to vary. In my app, it was 24, but in the codesandbox, it's something like 60.

Steps to reproduce the bug

  1. Open sandbox: https://codesandbox.io/p/sandbox/affectionate-merkle-28qvm9
  2. See console logs

Expected behavior

I think I should be able to render as many menus as needed.

Workaround

The menus still seem to work, but the warning is concerning.

#3214 (comment)

Possible solutions

No response

@jasikpark
Copy link

related to #3118 potentially

@diegohaz
Copy link
Member

Passing unmountOnHide to Menu appears to resolve the problem. It's also better not to render everything all at once if you're dealing with so many rows.

However, the warning shouldn't be there. I'll investigate this.

@diegohaz
Copy link
Member

I initially believed that #3241 had fixed this, but I was wrong. Nevertheless, it appears to be a false positive warning from React. I'll keep this open just in case.

diegohaz added a commit that referenced this issue Dec 30, 2023
Fix a regression introduced by #3241 where the `renderedItems` state was
being reset on every scroll event.

Closes #3285
Re-opens #3214
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants