[internal] Refactor: use class for hover hook state#3791
Conversation
commit: |
Bundle size report
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Greptile SummaryThis PR refactors the hover interaction hooks to use a class-based approach instead of individual React refs, improving performance by reducing the number of ref objects created. Key Changes:
Observations:
Confidence Score: 5/5
Important Files Changed
|
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
# Conflicts: # packages/react/src/floating-ui-react/hooks/useHoverFloatingInteraction.ts # packages/react/src/floating-ui-react/hooks/useHoverReferenceInteraction.ts
Add dispose and disposeEffect methods to properly clean up openChangeTimeout and restTimeout when components unmount. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
| data.hoverInteractionState = instance; | ||
| } | ||
|
|
||
| useOnMount(data.hoverInteractionState.disposeEffect); |
There was a problem hiding this comment.
I'm not sure this is correct. Since this runs on all components using useHoverInteractionSharedState, unmounting a single Trigger will reset the timers. The more correct way of handling this would be triggering the cleanup when the Root part unmounts, but this would require having another hook for the Root.
There was a problem hiding this comment.
Doesn't that issue also apply to the already existing hook?
There was a problem hiding this comment.
It does, indeed. Let's tackle it in a separate PR, then.
# Conflicts: # packages/react/src/floating-ui-react/hooks/useHoverInteractionSharedState.ts
New attempt of #3451
Refactor to use a class for hover hook state.