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

Fix crash in ListboxOptions when using as={Fragment} #3513

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

RobinMalfait
Copy link
Member

This PR fixes an issue where a Maximum update depth exceeded error occurs if you use as={Fragment} in the ListboxOptions component.

This PR also includes a refactor to make sure this exact issue cannot happen anymore in other components.

Fixes: #3507

Copy link

vercel bot commented Oct 8, 2024

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 Oct 8, 2024 9:11pm
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 9:11pm

@@ -57,7 +57,16 @@ export type PropsForFeatures<T extends RenderFeatures> = Expand<
>
>

export function render<TFeature extends RenderFeatures, TTag extends ElementType, TSlot>({
export function useRender() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we can move way more things in here such that everything is simpler and more optimized. But want to do it in small steps.

This essentially wraps the render(…) function with the useMergeRefsFn() which means every component now gets the useMergeRefsFn() for free.

We already use the `render` function helper in all of our components,
but we also need _some_ way of storing persistent functions (such as the
`mergeRefsFn`).

With this hook, we can embed it in the hook and we can ensure it's
always stable. We also won't run into an issue where we accidentally
forget something because it will be shipped by default.
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.

"Maximum update depth exceeded" when rendering ListboxOptions as Fragment
2 participants