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

Memory leaks: too many detached nodes #1190

Open
Jayvora opened this issue May 25, 2024 · 8 comments
Open

Memory leaks: too many detached nodes #1190

Jayvora opened this issue May 25, 2024 · 8 comments
Labels

Comments

@Jayvora
Copy link

Jayvora commented May 25, 2024

image

There are too many detached nodes in my application, this is because of react-tooltip, if i comment it all are gone, how can i get rid of this. This i causing too many memory leaks and causing my app being laggy.

I am using v5.24.0

@gabrieljablonski
Copy link
Member

Difficult to say what the cause is without seeing the code.

Try taking a look at our troubleshooting page, specifically the "Dynamically generated anchor elements" section.

https://react-tooltip.com/docs/troubleshooting#dynamically-generated-anchor-elements

@Jayvora
Copy link
Author

Jayvora commented Jun 16, 2024

image

^^ Here i have added two tooltips component on root of my webapp that is "_app.js"

image

^^ And using as the data-attribute in whole app like this.

So i guess its the correct way of using, let me know if i am doing anything wrong

Seeing too many detached nodes, if i remove those two components from "_app.js" then all the detached nodes are gone

@gabrieljablonski
Copy link
Member

@Jayvora best I can think of is that the tooltip might be leaking nodes with the render prop. Can you test a couple things to help us narrow it down? Don't forget to do the test with both your tooltips (or simply remove one of them, so testing is easier)

  1. Remove the render prop from both your tooltip components, and just use the content prop with any text, so the tooltip still shows

  2. Remove your RenderTooltip component to simplify the render function. Your custom component might be causing a weird interaction with the tooltip

Something like this should be good enough to test # 2:

<Tooltip
  ...
  render={({ content }) => content}
  ...
/>

Please let us know if either test makes the detached nodes disappear. We might be able to investigate further from your findings.

@naresh-pin
Copy link

naresh-pin commented Jul 4, 2024

image Seems to be the case with example implementation aswell, additionally can see JS event listeners flooding on every refresh or even on hovering on any tooltips.

I am suspecting possibility of some memory leak here

useEffect(() => {
const elementRefs = new Set(anchorRefs)
anchorsBySelect.forEach((anchor) => {
elementRefs.add({ current: anchor })
})
const anchorById = document.querySelector<HTMLElement>(`[id='${anchorId}']`)
if (anchorById) {
elementRefs.add({ current: anchorById })
}
const handleScrollResize = () => {
handleShow(false)
}
const anchorScrollParent = getScrollParent(activeAnchor)
const tooltipScrollParent = getScrollParent(tooltipRef.current)
if (actualGlobalCloseEvents.scroll) {
window.addEventListener('scroll', handleScrollResize)
anchorScrollParent?.addEventListener('scroll', handleScrollResize)
tooltipScrollParent?.addEventListener('scroll', handleScrollResize)
}
let updateTooltipCleanup: null | (() => void) = null
if (actualGlobalCloseEvents.resize) {
window.addEventListener('resize', handleScrollResize)
} else if (activeAnchor && tooltipRef.current) {
updateTooltipCleanup = autoUpdate(
activeAnchor as HTMLElement,
tooltipRef.current as HTMLElement,
updateTooltipPosition,
{
ancestorResize: true,
elementResize: true,
layoutShift: true,
},
)
}
const handleEsc = (event: KeyboardEvent) => {
if (event.key !== 'Escape') {
return
}
handleShow(false)
}
if (actualGlobalCloseEvents.escape) {
window.addEventListener('keydown', handleEsc)
}
if (actualGlobalCloseEvents.clickOutsideAnchor) {
window.addEventListener('click', handleClickOutsideAnchors)
}
const enabledEvents: { event: string; listener: (event?: Event) => void }[] = []
const handleClickOpenTooltipAnchor = (event?: Event) => {
if (show && event?.target === activeAnchor) {
/**
* ignore clicking the anchor that was used to open the tooltip.
* this avoids conflict with the click close event.
*/
return
}
handleShowTooltip(event)
}
const handleClickCloseTooltipAnchor = (event?: Event) => {
if (!show || event?.target !== activeAnchor) {
/**
* ignore clicking the anchor that was NOT used to open the tooltip.
* this avoids closing the tooltip when clicking on a
* new anchor with the tooltip already open.
*/
return
}
handleHideTooltip()
}
const regularEvents = ['mouseover', 'mouseout', 'mouseenter', 'mouseleave', 'focus', 'blur']
const clickEvents = ['click', 'dblclick', 'mousedown', 'mouseup']
Object.entries(actualOpenEvents).forEach(([event, enabled]) => {
if (!enabled) {
return
}
if (regularEvents.includes(event)) {
enabledEvents.push({ event, listener: debouncedHandleShowTooltip })
} else if (clickEvents.includes(event)) {
enabledEvents.push({ event, listener: handleClickOpenTooltipAnchor })
} else {
// never happens
}
})
Object.entries(actualCloseEvents).forEach(([event, enabled]) => {
if (!enabled) {
return
}
if (regularEvents.includes(event)) {
enabledEvents.push({ event, listener: debouncedHandleHideTooltip })
} else if (clickEvents.includes(event)) {
enabledEvents.push({ event, listener: handleClickCloseTooltipAnchor })
} else {
// never happens
}
})
if (float) {
enabledEvents.push({
event: 'pointermove',
listener: handlePointerMove,
})
}
const handleMouseEnterTooltip = () => {
hoveringTooltip.current = true
}
const handleMouseLeaveTooltip = () => {
hoveringTooltip.current = false
handleHideTooltip()
}
if (clickable && !hasClickEvent) {
// used to keep the tooltip open when hovering content.
// not needed if using click events.
tooltipRef.current?.addEventListener('mouseenter', handleMouseEnterTooltip)
tooltipRef.current?.addEventListener('mouseleave', handleMouseLeaveTooltip)
}
enabledEvents.forEach(({ event, listener }) => {
elementRefs.forEach((ref) => {
ref.current?.addEventListener(event, listener)
})
})
return () => {
if (actualGlobalCloseEvents.scroll) {
window.removeEventListener('scroll', handleScrollResize)
anchorScrollParent?.removeEventListener('scroll', handleScrollResize)
tooltipScrollParent?.removeEventListener('scroll', handleScrollResize)
}
if (actualGlobalCloseEvents.resize) {
window.removeEventListener('resize', handleScrollResize)
} else {
updateTooltipCleanup?.()
}
if (actualGlobalCloseEvents.clickOutsideAnchor) {
window.removeEventListener('click', handleClickOutsideAnchors)
}
if (actualGlobalCloseEvents.escape) {
window.removeEventListener('keydown', handleEsc)
}
if (clickable && !hasClickEvent) {
tooltipRef.current?.removeEventListener('mouseenter', handleMouseEnterTooltip)
tooltipRef.current?.removeEventListener('mouseleave', handleMouseLeaveTooltip)
}
enabledEvents.forEach(({ event, listener }) => {
elementRefs.forEach((ref) => {
ref.current?.removeEventListener(event, listener)
})
})
}
/**
* rendered is also a dependency to ensure anchor observers are re-registered
* since `tooltipRef` becomes stale after removing/adding the tooltip to the DOM
*/
}, [
activeAnchor,
updateTooltipPosition,
rendered,
anchorRefs,
anchorsBySelect,
// the effect uses the `actual*Events` objects, but this should work
openEvents,
closeEvents,
globalCloseEvents,
shouldOpenOnClick,
delayShow,
delayHide,
])

@mattathompson
Copy link

👍 having the same issue. Single Tooltip at the root of my React application. Seeing many leaks when having lists of dynamic items that have the data-tooltip attributes.

I looked at the troubleshooting guide before coming here and confirmed we're doing what's suggested.

Here are comparison Heap Snapshots I took after using the UI and making the dynamic items re-render

First one is with the data-tooltip attributes in place:

Screenshot 2024-07-24 at 12 29 51 AM

Second one doing the same, but without the data-tooltip attributes in place:

Screenshot 2024-07-24 at 12 29 45 AM

@mattathompson
Copy link

We were using an older version of the library (5.19.0) and then upgraded to the latest (5.27.1) and reran snapshots. That solved the issue for us.

My guess is that this was fixed in a recent version and y'all can probably close this issue.

@gabrieljablonski
Copy link
Member

@mattathompson thanks for reporting back

@Jayvora @naresh-pin can you guys confirm if it still happens for you in the latest release?

Copy link

This issue is stale because it has not seen activity in 30 days. Remove the stale label or comment within 14 days, or it will be closed.

@github-actions github-actions bot added the Stale This has not seen activity in quite some time label Oct 23, 2024
@gabrieljablonski gabrieljablonski removed the Stale This has not seen activity in quite some time label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants