Skip to content

[Context Menu] fix: stop infinite re-renders when the menu is closed #3386

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Randulfe
Copy link

@Randulfe Randulfe commented Feb 17, 2025

Description

When the context menu is closed the anchor component bringing the menu is still open so if a component using the context menu trigger re-renders quite frequently it will cause an infinite re-render loop with the menu as it's still rendered even when the context menu is supposed to be closed.

The problem seem to come from <MenuPrimitive.Anchor> which uses a virtual ref to measure its bounding client rect. If the trigger is frequently re-rendered it triggers constant state updates in the positioning logic (even the the context menu is closed) and ultimately causes an infinite update loop that has probably to do with radix's menu (not so familiar with it but it must be reacting with this in an inifinite rendering way).

Warning

This fixed the issue on my end that has also been reported here #2717 (comment) and in my issue #3385 but due to my unfamiliarity with this project please make sure this does not have any unintentional side effects.

@Harukisatoh

This comment was marked as duplicate.

@jarvis394

This comment was marked as duplicate.

@recyclerobot

This comment was marked as duplicate.

@jarvis394
Copy link

Here is a patch for anyone facing this issue (untested, but resolves re-renders):

@radix-ui+react-popper+1.2.2.patch
diff --git a/node_modules/@radix-ui/react-popper/dist/index.js b/node_modules/@radix-ui/react-popper/dist/index.js
index 93cc745..4eb9b92 100644
--- a/node_modules/@radix-ui/react-popper/dist/index.js
+++ b/node_modules/@radix-ui/react-popper/dist/index.js
@@ -76,7 +76,7 @@ var PopperAnchor = React.forwardRef(
     const composedRefs = (0, import_react_compose_refs.useComposedRefs)(forwardedRef, ref);
     React.useEffect(() => {
       context.onAnchorChange(virtualRef?.current || ref.current);
-    });
+    }, [context.onAnchorChange]);
     return virtualRef ? null : /* @__PURE__ */ (0, import_jsx_runtime.jsx)(import_react_primitive.Primitive.div, { ...anchorProps, ref: composedRefs });
   }
 );
diff --git a/node_modules/@radix-ui/react-popper/dist/index.mjs b/node_modules/@radix-ui/react-popper/dist/index.mjs
index 3cf1db5..de67935 100644
--- a/node_modules/@radix-ui/react-popper/dist/index.mjs
+++ b/node_modules/@radix-ui/react-popper/dist/index.mjs
@@ -41,7 +41,7 @@ var PopperAnchor = React.forwardRef(
     const composedRefs = useComposedRefs(forwardedRef, ref);
     React.useEffect(() => {
       context.onAnchorChange(virtualRef?.current || ref.current);
-    });
+    }, [context.onAnchorChange]);
     return virtualRef ? null : /* @__PURE__ */ jsx(Primitive.div, { ...anchorProps, ref: composedRefs });
   }
 );

@MatteoNet

This comment was marked as duplicate.

@zocario
Copy link

zocario commented Apr 7, 2025

Thanks @jarvis394, we are facing the same issue and your patch is fixing the problem 🙏 Maybe the content of this PR should be updated to reflect this fix @Randulfe?

@raymondkneipp
Copy link

#3500

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

Successfully merging this pull request may close these issues.

7 participants