primitives icon indicating copy to clipboard operation
primitives copied to clipboard

[Tooltip] Wrapping the app in TooltipProvider causes every single tooltip to re-render on hover

Open lauri865 opened this issue 2 years ago • 5 comments

Bug report

Current Behavior

When wrapping the app in Tooltip provider, every single component that implements a tooltip gets re-rendered on hovering any of the components that implement a tooltip, even if nothing changes, causing excessive re-renders.

Expected behavior

Only components with an active tooltip should be affected or when default settings change.

Reproducible example

Wrap an app in TooltipProvider and turn on Highlight updates when components render in React devtools, and then hover any component with a tooltip to notice that every component that implements a tooltip gets re-rendered.

Suggested solution

Additional context

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-tooltip 1.0.6
React react 18.2.0
Browser Chrome Version 116.0.5845.140 (Official Build) (arm64)
Assistive tech
Node n/a
npm/yarn
Operating System

lauri865 avatar Sep 02 '23 23:09 lauri865

bump?

CarlosVergikosk avatar Sep 10 '24 10:09 CarlosVergikosk

Oh wow, we also just ran into this. Does this mean that we need to be wrapping every single Tooltip in TooltipProvider? I thought the opposite was supposed to be true.

divmgl avatar Sep 15 '24 18:09 divmgl

Based on the Profiler highlight,

  1. It seems like anything under the Tooltip Provider will re-render upon the first detection (mouse-wise, keyboard-wise)
  2. after that, only the target item will be re-render when you hover them, rest wouldn't.
  3. then the last detection when you leave the Provider "gray area", will trig the re-render of all, like the first detection.

Hence, the Provider is pretty similar to how React.Context works, Where your issue might be from the first and last detection as far as I can tell, can't tell about the detection algorithm without digging though the source code. So you were claiming might not be true.

That also means, the following case can cause excessive re-renders, where you have a local Tooltip Provider that wraps lots of small items. For example, a calendar panel. That the total size of the Tooltip Provider (panel) in page is also small enough and your mouse is constantly in and out across the Provider detection area (panel edge). So you constantly trig the first(in)/last(out) detection. If you also have some heavy computations in each date button, without some sort of useMemo or useCallback. You might experience lag.

But you can't always make the conclusion based on the dev-tools highlight, always need to measure, the render might be cheap.

aboveyunhai avatar Sep 20 '24 00:09 aboveyunhai

ran into the same thing, this should be mentioned in the docs. would having multiple providers cause issues with render priority?

clicktodev avatar Oct 06 '24 01:10 clicktodev

Here is a patch that fixes the issue

The trick is to use useRef for isOpenDelayed instead of useState. Seems like a safe change since the variable is read inside event callback and not inside render

Our app has tons of elements with tooltips on screen so the issue was quite critical for us. After the change only one element re-renders on hover

diff --git a/dist/index.js b/dist/index.js
index a805b534d2f34981afe9270bfbf0ad9e9a115d21..17f07a96ec77e4b1dcefa8554ee14fab602ba95b 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -79,7 +79,7 @@ var TooltipProvider = (props) => {
     disableHoverableContent = false,
     children
   } = props;
-  const [isOpenDelayed, setIsOpenDelayed] = React.useState(true);
+  const isOpenDelayedRef = React.useRef(true);
   const isPointerInTransitRef = React.useRef(false);
   const skipDelayTimerRef = React.useRef(0);
   React.useEffect(() => {
@@ -90,16 +90,16 @@ var TooltipProvider = (props) => {
     TooltipProviderContextProvider,
     {
       scope: __scopeTooltip,
-      isOpenDelayed,
+      isOpenDelayedRef,
       delayDuration,
       onOpen: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
-        setIsOpenDelayed(false);
+        isOpenDelayedRef.current = (false);
       }, []),
       onClose: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
         skipDelayTimerRef.current = window.setTimeout(
-          () => setIsOpenDelayed(true),
+          () => isOpenDelayedRef.current = (true),
           skipDelayDuration
         );
       }, [skipDelayDuration]),
@@ -178,9 +178,9 @@ var Tooltip = (props) => {
       trigger,
       onTriggerChange: setTrigger,
       onTriggerEnter: React.useCallback(() => {
-        if (providerContext.isOpenDelayed) handleDelayedOpen();
+        if (providerContext.isOpenDelayedRef.current) handleDelayedOpen();
         else handleOpen();
-      }, [providerContext.isOpenDelayed, handleDelayedOpen, handleOpen]),
+      }, [providerContext.isOpenDelayedRef, handleDelayedOpen, handleOpen]),
       onTriggerLeave: React.useCallback(() => {
         if (disableHoverableContent) {
           handleClose();
diff --git a/dist/index.mjs b/dist/index.mjs
index a6ac5447b420e89a93415510e2a6fc4b11ab96bd..065f5845826afdee9e01aa3191d6a48eeeed936d 100644
--- a/dist/index.mjs
+++ b/dist/index.mjs
@@ -32,7 +32,7 @@ var TooltipProvider = (props) => {
     disableHoverableContent = false,
     children
   } = props;
-  const [isOpenDelayed, setIsOpenDelayed] = React.useState(true);
+  const isOpenDelayedRef = React.useRef(true);
   const isPointerInTransitRef = React.useRef(false);
   const skipDelayTimerRef = React.useRef(0);
   React.useEffect(() => {
@@ -43,16 +43,16 @@ var TooltipProvider = (props) => {
     TooltipProviderContextProvider,
     {
       scope: __scopeTooltip,
-      isOpenDelayed,
+      isOpenDelayedRef,
       delayDuration,
       onOpen: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
-        setIsOpenDelayed(false);
+        isOpenDelayedRef.current = false;
       }, []),
       onClose: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
         skipDelayTimerRef.current = window.setTimeout(
-          () => setIsOpenDelayed(true),
+          () => isOpenDelayedRef.current = true,
           skipDelayDuration
         );
       }, [skipDelayDuration]),
@@ -131,9 +131,9 @@ var Tooltip = (props) => {
       trigger,
       onTriggerChange: setTrigger,
       onTriggerEnter: React.useCallback(() => {
-        if (providerContext.isOpenDelayed) handleDelayedOpen();
+        if (providerContext.isOpenDelayedRef.current) handleDelayedOpen();
         else handleOpen();
-      }, [providerContext.isOpenDelayed, handleDelayedOpen, handleOpen]),
+      }, [providerContext.isOpenDelayedRef, handleDelayedOpen, handleOpen]),
       onTriggerLeave: React.useCallback(() => {
         if (disableHoverableContent) {
           handleClose();

misha-erm avatar Oct 14 '24 11:10 misha-erm

Do you have plan to merge this? Still happening

suitux avatar Nov 09 '24 10:11 suitux

I have same issue

Fatihkrty avatar Nov 22 '24 18:11 Fatihkrty

bump

drewlowe avatar Dec 03 '24 16:12 drewlowe

bump x2

CarlosVergikosk avatar Dec 13 '24 14:12 CarlosVergikosk

bump

arybitskiy avatar Jan 07 '25 14:01 arybitskiy

same issue

rosko avatar Jan 17 '25 19:01 rosko

bump-bump, we have the same issue

BrickheadJohnny avatar Jan 31 '25 06:01 BrickheadJohnny

bump

elieven avatar Feb 03 '25 10:02 elieven

Seriously the simplest example with 5 tooltip items - the whole app being wrapped in a TooltipProvider or each item separately - goes from 144 to 80fps (60~fps drop) when mousing over them. It is beyond insane of a performance drop.

elieven avatar Feb 03 '25 11:02 elieven

Bump. Please

lucas-neuhaus-bairesdev avatar Mar 13 '25 13:03 lucas-neuhaus-bairesdev

Bump, please

d-novak avatar Mar 17 '25 07:03 d-novak

Bump

NEET64 avatar Mar 19 '25 16:03 NEET64

Dump

dnguyenfs avatar Apr 14 '25 03:04 dnguyenfs

Looks like the fix was released already https://www.radix-ui.com/primitives/docs/overview/releases#april-8-2025

misha-erm avatar Apr 14 '25 07:04 misha-erm