Skip to content

Performance improvement: only re-render top-level component #3722

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

Merged
merged 6 commits into from
May 10, 2025

Conversation

RobinMalfait
Copy link
Member

@RobinMalfait RobinMalfait commented May 10, 2025

This PR fixes a performance issue where all components using the useIsTopLayer hook will re-render when the hook changes.

For context, the internal hook is used to know which component is the top most component. This is important in a situation like this:

<Dialog>
  <Menu />
</Dialog>

If the Menu inside the Dialog is open, it is considered the top most component. Clicking outside of the Menu or pressing escape should only close the Menu and not the Dialog.

This behavior is similar to the native #top-layer you see when using native dialogs for example.

The issue however is that the useIsTopLayer subscribes to an external store which is shared across all components. This means that when the store changes, all components using the hook will re-render.

To make things worse, since we can't use these hooks unconditionally, they will all be subscribed to the store even if the Menu component(s) are not open.

To solve this, we will use a new state machine and use the useMachine hook. This internally uses a useSyncExternalStoreWithSelector to subscribe to the store.

This means that the component will only re-render if the state computed by the selector changes.

This now means that at most 2 components will re-render when the store changes:

  1. The component that was in the top most position
  2. The component that is going to be in the top most position

Fixes: #3630
Closes: #3662

Test plan

Behavior before: notice how all Menu components re-render:

Screen.Recording.2025-05-10.at.02.00.21.mov

After this change, only the Menu that was opened / closed will re-render:

Screen.Recording.2025-05-10.at.01.58.51.mov

This will allow us to do some cleanup when components using the machine
get unmounted
As long as the state didn't _actually_ change, there is no need to even
call and compute event listeners.
Make use of the stack machine
Copy link

vercel bot commented May 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 10, 2025 0:22am
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 10, 2025 0:22am

@RobinMalfait RobinMalfait merged commit 18cf984 into main May 10, 2025
8 checks passed
@RobinMalfait RobinMalfait deleted the fix/issue-3630 branch May 10, 2025 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance issues when many <Menu> elements exist on one page
2 participants