-
Notifications
You must be signed in to change notification settings - Fork 971
[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
base: main
Are you sure you want to change the base?
[Context Menu] fix: stop infinite re-renders when the menu is closed #3386
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
Here is a patch for anyone facing this issue (untested, but resolves re-renders): @radix-ui+react-popper+1.2.2.patchdiff --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 });
}
); |
This comment was marked as duplicate.
This comment was marked as duplicate.
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? |
If anyone is wondering how to properly apply @jarvis394's patch in your project without forking the library, you can do so as follows:
Finally, make sure you add this script to your "scripts": {
+ "postinstall": "patch-package"
} I suppose most of you already do it this way but not everyone who finds this thread does. I hope a solution gets merged soon. This is a rather critical issue. |
…popper dependency See radix-ui/primitives#3386 for context.
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.