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

[tooltip] Preventing from unnecessarily rerendering the children component #41144

Open
samhuk opened this issue Feb 17, 2024 · 8 comments
Open
Assignees
Labels
component: tooltip This is the name of the generic UI component, not the React module!

Comments

@samhuk
Copy link

samhuk commented Feb 17, 2024

Summary

This RFC concerns the React Tooltip component and potential performance issues.

Problem

The React Tooltip component unnecessarily rerenders the provided children component ~2-3 times on initial Tooltip render and every time the tooltip's visibility state toggles.

Cause

There are a considerable number of code paths leading up to the final cloneElement call here that create a large number of new references. This leads to the observed unnecessary rerendering behavior.

Experimentation

I have tested this and confirmed that Tooltip indeed causes the children component to unnecessarily render at least 2-3 additional times.

As a PoC, I have also tested with a reduced-complexity (but-otherwise like-for-like) Tooltip component that proves that leveraging React memoization utils eliminates the problem. For example, the ultimate cloneElement call ends up looking something like this:

image

I do not however possess the resources to delve any deeper into this (e.g. in-depth perf benchmarks). Apologies.

Open Questions

  1. Would the (likely significant) refactors be worth-it?
  2. Would the performance gain of less children renders outweigh the performance loss of memoization?

What are the requirements?

Zero unnecessary rerenders of the provided children component.

What are our options?

With my current understanding, there is only one accepted way to achieve this - refactor Tooltip to use React memoization utils to prevent childrenProps from being a new reference or containing new references.

Proposed solution

The crux of the solution is to extract out the cloneElement call and place it inside a useMemo call with the properties of childrenProps as the memoization dependencies.

This may require some additional uses of useCallback in order to ensure the event handler functions such as handleEnter and onMouseEnter are not new references.

Resources and benchmarks

N/A

Search keywords: tooltip, react, performance, memoization

@samhuk samhuk added RFC Request For Comments status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 17, 2024
@zannager zannager added the component: tooltip This is the name of the generic UI component, not the React module! label Feb 18, 2024
@oliviertassinari oliviertassinari changed the title [RFC] Preventing Tooltip from unnecessarily rerendering the children component [tooltip] Preventing from unnecessarily rerendering the children component Feb 18, 2024
@oliviertassinari oliviertassinari removed the RFC Request For Comments label Feb 18, 2024
@oliviertassinari
Copy link
Member

Assignment moved to Michal per https://www.notion.so/mui-org/component-Tooltip-4764ddf0984c438f8b089decbe868e2e

@michaldudak
Copy link
Member

The Tooltip is being reimplemented in Base UI.
@atomiks, could you take this issue into consideration when working on the new version?

@michaldudak michaldudak assigned atomiks and unassigned michaldudak Apr 10, 2024
@atomiks
Copy link
Contributor

atomiks commented Apr 11, 2024

@samhuk just curious if this is actually causing a performance issue for you, or if it's just something technical to note in theory?

For performance, React has a built-in optimization for the children prop. The ExpensiveComponent won't re-render even if CheapComponent does in the following scenario:

<Tooltip title="Delete">
  <CheapComponent>
    <ExpensiveComponent />
  </CheapComponent>
</Tooltip>

You can ensure the direct child is a cheap node, while expensive children are either passed externally like in this example, or memo'd in the CheapComponent's render/return call. In the vast majority of cases though, 1 or 2 extra re-renders is not something to worry about, so that's why I think in this scenario for simple anchor components, it doesn't need investigating.

@samhuk
Copy link
Author

samhuk commented Apr 14, 2024

Thank you for the response folks.


if this is actually causing a performance issue

I have done some testing and verified that the child node renders 2-3 times. When there are many on the page, it's noticable (when I just shim out Tooltip to a no-op component, the rendering finish times are noticeably lower)


Regarding the Tooltip -> CheapComp -> ExpensiveComp (with no props) nesting and/or memo workaround(s) - I'm sorry, however I don't see how this is relevant or appropriate here. One would hope that a developer knows about the workarounds, providing appropriate dependencies to their elaborate hooks, and such things, however this is not often real.

That being said, this is not high priority, so I am open to this being closed, particularly since it is being reimplemented according to @michaldudak. I don't want to create extra work 🙏

@atomiks
Copy link
Contributor

atomiks commented Apr 15, 2024

I have done some testing and verified that the child node renders 2-3 times. When there are many on the page, it's noticable (when I just shim out Tooltip to a no-op component, the rendering finish times are noticeably lower)

I understand that it's rendering multiple times, it's just that this shouldn't be a problem. Instead of shimming out the entire Tooltip component, can you shim out whatever the child anchor is to a plain <span />? Since the crux of this issue is that the child component is being re-rendered multiple times, what's the difference if it's only a plain DOM node? If there's a difference here, that must mean the anchor component you're rendering is expensive? And concretely, what do you mean by "noticeable" %-wise?

This issue is still relevant since the re-implementation has the same "extra" re-renders (in order to set the anchor element state, along with props, and even more when enabling grouping and transitions). However, I haven't had any issues filed about these extra re-renders being a problem. In my experience, slow renders are a problem, not extra re-renders.

@samhuk
Copy link
Author

samhuk commented Apr 23, 2024

For context, one of the projects I am on is a typical mid-enterpise-size web app, where there is Tooltip on all of the typical web app components plus, for some pages, on almost all cells within some very large MUI DataGridPros.

Some of the pages end up with ~300-500 Tooltip instances. It easily reaches ~600 rerenders (300 * 2 = 600 best case, up to 500 * 3 = 1500 worst case), and, more consequentially, a considerable number of expensive hooks being reevaluated (e.g. grabbing state from URL, session state, doing API requests, DOM manip, etc.).

Percentage-wise, I've managed to estimate it at around a 5-10% increase in page load times for a page without a chunky tooltip-for-each-cell table, and 20-30% otherwise. It really is dependant on the app and the engineering habits of the project.

I understand that it's rendering multiple times, it's just that this shouldn't be a problem

Doesn't this require accepting either the assumption that all Tooltip usage is around simple lightweight elements, or the assumption that all developers are aware of the workarounds necessary to avoid expensive rerenders? My personal view from experience is that neither of these are true. Developers use Tooltip as they see fit, which is usually just <Tooltip><WhateverTheyWant/></Tooltip>. Perhaps I could have better elucidated this versus "I'm sorry, however I don't see how this is relevant or appropriate here" from my previous message. Apologies.

However, I haven't had any issues filed about these extra re-renders being a problem

To be honest, I'm surprised that nobody noticed the double or triple render of their components that are wrapped with Tooltip 😅


Moving forward

To me, I see two possible courses of action:

  1. We close this issue due to it being insufficiently consequential.
  2. The reimplementers of Tooltip do some digging and weigh up the benefit of less rerenders against the cost of adding in the necessary hooks that prevent those rerenders.

@ZeeshanTamboli ZeeshanTamboli removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 28, 2024
@tolluset
Copy link

tolluset commented Jun 4, 2024

I've also reached this issue for performance problem.

The main issue was caused by <Tooltip /> component.

We used to hamburger menu icon for show menu list, but some how related components are more and more the button reflect really slow.

<Tooltip
  title="menu"
>
  <IconButton onClick={handleHambugerMenuOpen} size="large">
    {hamburgerMenuOpened ? (
      <HamburgerMenuCloseIcon />
    ) : (
      <HamburgerMenuIcon />
    )}
  </IconButton>
</Tooltip>

We test it to wrap <Tooltip /> as memo, but it does not got efficiently. So we decide it to hide tooltip on touch devices.

export const Tooltip = React.memo(function Tooltip({
  isArrow = false,
  sx = [],
  children,
  ...rest
}: TooltipProps & { isArrow?: boolean }) {
  const isTouchDevice = useMediaQuery('(pointer: coarse)');

  if (isTouchDevice) {
    return children;
  }
  
  return (
    <MuiTooltip
      arrow={isArrow}
      componentsProps={{
        tooltip: {
  
  ...

The performance issue is seem to more revealed in mobile(touch) devices.


p.s. We'd got issue when get 60~80 tooltips in same page.

@lukaselmer
Copy link
Contributor

@michaldudak @atomiks It's not just a theory. We also have issues in our production project.

Without the tooltips, the table renders in about 200-300ms (it's an admin table, and 200-300ms is OK). With tooltips enabled, it takes 2.7 seconds to render the same page, and this lag becomes noticeable, even for an admin page 😲 We render about 100 items, and each item has between 1 and 15 tooltips.

Oh, and I'm using a MacBook Pro with Apple M2 Max - it's probably even slower on lower spec devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tooltip This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

9 participants