primitives icon indicating copy to clipboard operation
primitives copied to clipboard

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

Open Randulfe opened this issue 10 months ago • 8 comments

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 https://github.com/radix-ui/primitives/issues/2717#issuecomment-2649190273 and in my issue https://github.com/radix-ui/primitives/issues/3385 but due to my unfamiliarity with this project please make sure this does not have any unintentional side effects.

Randulfe avatar Feb 17 '25 15:02 Randulfe

Someone please give attention to this issue, this is open since February 2024.

Harukisatoh avatar Feb 27 '25 00:02 Harukisatoh

Bump, also facing this issue

jarvis394 avatar Mar 20 '25 20:03 jarvis394

Bumping, also facing this issue

recyclerobot avatar Mar 31 '25 15:03 recyclerobot

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 });
   }
 );

jarvis394 avatar Mar 31 '25 15:03 jarvis394

Bump, also facing this issue!

Matte0Serafini avatar Apr 04 '25 10:04 Matte0Serafini

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?

zocario avatar Apr 07 '25 14:04 zocario

https://github.com/radix-ui/primitives/pull/3500

raymondkneipp avatar Apr 25 '25 14:04 raymondkneipp

If anyone is wondering how to properly apply @jarvis394's patch in your project without forking the library, you can do so as follows:

npm i -D patch-package
mkdir patches
mv @radix-ui+react-popper+1.2.2.patch patches
npx patch-package

Finally, make sure you add this script to your package.json so the patches are applied whenever you run npm install:

 "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.

sleeyax avatar Jun 04 '25 08:06 sleeyax

waiting...

felipetodev avatar Jun 21 '25 19:06 felipetodev

If anyone is wondering how to properly apply @jarvis394's patch in your project without forking the library, you can do so as follows:

npm i -D patch-package
mkdir patches
mv @radix-ui+react-popper+1.2.2.patch patches
npx patch-package

Finally, make sure you add this script to your package.json so the patches are applied whenever you run npm install:

 "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.

Thanks for the suggestion! I used pnpm patch @radix-ui/react-popper — it's working fine now. If you're using pnpm, you can check out https://pnpm.io/cli/patch for more details. Still looking forward to an official fix from Radix.

exsesx avatar Jun 24 '25 11:06 exsesx

Actually, I don't think that's the solution. I ended up fixing it on my end by using virtualization — only rendering the elements visible on the screen. That significantly reduced the number of popovers, and the error disappeared. So my take is that the library itself works fine, as long as it's not overloaded with too many elements to track.

exsesx avatar Jun 30 '25 05:06 exsesx

Should be addressed here: https://github.com/radix-ui/primitives/pull/3614

chaance avatar Jul 03 '25 00:07 chaance