Skip to content

Commit

Permalink
Fix <Popover.Button as={Fragment} /> crash (#1889)
Browse files Browse the repository at this point in the history
* prevent infinite loop

When you use `as={Fragment}` an unmount and remount can happen. This
means that the `ref` gets called with `null` for the unmount and
`HTMLButtonElement` for the mount.

This keeps toggling which results in an infinite loop and eventually a
Maximum callback size exceeded issue.

This ensures that we only set the button if we have a button.

* update changelog
  • Loading branch information
RobinMalfait authored Sep 30, 2022
1 parent fdccf07 commit af68a34
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 2 deletions.
4 changes: 3 additions & 1 deletion packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Nothing yet!
### Fixed

- Fix `<Popover.Button as={Fragment} />` crash ([#1889](https://github.com/tailwindlabs/headlessui/pull/1889))

## [1.7.3] - 2022-09-30

Expand Down
27 changes: 27 additions & 0 deletions packages/@headlessui-react/src/components/popover/popover.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,33 @@ describe('Rendering', () => {
})

describe('Popover.Button', () => {
it(
'should be possible to render a Popover.Button using a fragment',
suppressConsoleLogs(async () => {
render(
<Popover>
<Popover.Button as={Fragment}>
<button>Trigger</button>
</Popover.Button>
<Popover.Panel></Popover.Panel>
</Popover>
)

assertPopoverButton({
state: PopoverState.InvisibleUnmounted,
attributes: { id: 'headlessui-popover-button-1' },
})
assertPopoverPanel({ state: PopoverState.InvisibleUnmounted })

await click(getPopoverButton())

assertPopoverButton({
state: PopoverState.Visible,
attributes: { id: 'headlessui-popover-button-1' },
})
assertPopoverPanel({ state: PopoverState.Visible })
})
)
it(
'should be possible to render a Popover.Button using a render prop',
suppressConsoleLogs(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
let buttonRef = useSyncRefs(
internalButtonRef,
ref,
isWithinPanel ? null : (button) => dispatch({ type: ActionTypes.SetButton, button })
isWithinPanel ? null : (button) => button && dispatch({ type: ActionTypes.SetButton, button })
)
let withinPanelButtonRef = useSyncRefs(internalButtonRef, ref)
let ownerDocument = useOwnerDocument(internalButtonRef)
Expand Down

0 comments on commit af68a34

Please sign in to comment.