From e0ceb69f6da1d5fb1963bd4bf188a16bf7348705 Mon Sep 17 00:00:00 2001 From: Brian Geihsler Date: Mon, 9 Mar 2026 18:28:01 +0000 Subject: [PATCH] Adjust `activeItemIndex` when items are unregistered in Menu and Listbox MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `MenuItem` or `ListboxOption` components are dynamically removed from the DOM while a Menu/Listbox is open, the `UnregisterItems`/ `UnregisterOptions` reducer removes items from the internal array but does not adjust `activeItemIndex`/`activeOptionIndex`. This causes the index to point to a non-existent entry, and the next keyboard event crashes with: TypeError: Cannot destructure property 'dataRef' of 'state.items[state.activeItemIndex]' as it is undefined. Fix by delegating to `adjustOrderedState()` in both reducers — the same approach already used by the Combobox component. Also add defensive null checks at the two direct crash sites in case the index is stale for any other reason. --- .../src/components/listbox/listbox-machine.ts | 26 ++----- .../src/components/listbox/listbox.test.tsx | 75 +++++++++++++++++++ .../src/components/menu/menu-machine.ts | 22 +----- .../src/components/menu/menu.test.tsx | 75 +++++++++++++++++++ .../src/components/menu/menu.tsx | 4 +- 5 files changed, 164 insertions(+), 38 deletions(-) diff --git a/packages/@headlessui-react/src/components/listbox/listbox-machine.ts b/packages/@headlessui-react/src/components/listbox/listbox-machine.ts index fa91da11eb..d3f3034b78 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox-machine.ts +++ b/packages/@headlessui-react/src/components/listbox/listbox-machine.ts @@ -390,28 +390,14 @@ let reducers: { } }, [ActionTypes.UnregisterOptions]: (state, action) => { - let options = state.options - - let idxs = [] let ids = new Set(action.options) - for (let [idx, option] of options.entries()) { - if (ids.has(option.id)) { - idxs.push(idx) - ids.delete(option.id) - if (ids.size === 0) break - } - } - - if (idxs.length > 0) { - options = options.slice() - for (let idx of idxs.reverse()) { - options.splice(idx, 1) - } - } + let adjustedState = adjustOrderedState(state, (options) => { + return options.filter((option) => !ids.has(option.id)) + }) return { ...state, - options, + ...adjustedState, activationTrigger: ActivationTrigger.Other, } }, @@ -590,7 +576,9 @@ export class ListboxMachine extends Machine, Actions> { selectActiveOption: () => { if (this.state.activeOptionIndex !== null) { - let { dataRef } = this.state.options[this.state.activeOptionIndex] + let activeOption = this.state.options[this.state.activeOptionIndex] + if (!activeOption) return + let { dataRef } = activeOption this.actions.selectOption(dataRef.current.value) } else if (this.state.dataRef.current.mode === ValueMode.Single) { this.actions.closeListbox() diff --git a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx index 15b2d75750..58c9bc898e 100644 --- a/packages/@headlessui-react/src/components/listbox/listbox.test.tsx +++ b/packages/@headlessui-react/src/components/listbox/listbox.test.tsx @@ -4705,3 +4705,78 @@ describe('transitions', () => { }) ) }) + +describe('Dynamic option removal', () => { + it( + 'should not crash when the active option is removed', + suppressConsoleLogs(async () => { + function Example({ hide = false }) { + return ( + console.log(x)}> + Trigger + + alice + {!hide && bob} + {!hide && charlie} + + + ) + } + + let { rerender } = render() + + // Open the listbox + await click(getListboxButton()) + assertListbox({ state: ListboxState.Visible }) + + // Navigate to the last option + await press(Keys.End) + let options = getListboxOptions() + assertActiveListboxOption(options[2]) + + // Remove options while the listbox is open + rerender() + + // Press Enter — should not crash, should close the listbox gracefully + await press(Keys.Enter) + assertListbox({ state: ListboxState.InvisibleUnmounted }) + }) + ) + + it( + 'should adjust activeOptionIndex when options before the active option are removed', + suppressConsoleLogs(async () => { + function Example({ hide = false }) { + return ( + console.log(x)}> + Trigger + + {!hide && alice} + bob + charlie + + + ) + } + + let { rerender } = render() + + // Open the listbox + await click(getListboxButton()) + assertListbox({ state: ListboxState.Visible }) + + // Navigate to the last option ("charlie") + await press(Keys.End) + let options = getListboxOptions() + assertActiveListboxOption(options[2]) + + // Remove the first option ("alice") while "charlie" is active + rerender() + + // "charlie" should still be the active option (now at index 1) + options = getListboxOptions() + expect(options).toHaveLength(2) + assertActiveListboxOption(options[1]) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/menu/menu-machine.ts b/packages/@headlessui-react/src/components/menu/menu-machine.ts index ac363af1d9..db8c10943f 100644 --- a/packages/@headlessui-react/src/components/menu/menu-machine.ts +++ b/packages/@headlessui-react/src/components/menu/menu-machine.ts @@ -305,28 +305,14 @@ let reducers: { } }, [ActionTypes.UnregisterItems]: (state, action) => { - let items = state.items - - let idxs = [] let ids = new Set(action.items) - for (let [idx, item] of items.entries()) { - if (ids.has(item.id)) { - idxs.push(idx) - ids.delete(item.id) - if (ids.size === 0) break - } - } - - if (idxs.length > 0) { - items = items.slice() - for (let idx of idxs.reverse()) { - items.splice(idx, 1) - } - } + let adjustedState = adjustOrderedState(state, (items) => { + return items.filter((item) => !ids.has(item.id)) + }) return { ...state, - items, + ...adjustedState, activationTrigger: ActivationTrigger.Other, } }, diff --git a/packages/@headlessui-react/src/components/menu/menu.test.tsx b/packages/@headlessui-react/src/components/menu/menu.test.tsx index 14a2d3109a..9ccd1434cb 100644 --- a/packages/@headlessui-react/src/components/menu/menu.test.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.test.tsx @@ -3604,3 +3604,78 @@ describe('transitions', () => { }) ) }) + +describe('Dynamic item removal', () => { + it( + 'should not crash when the active item is removed', + suppressConsoleLogs(async () => { + function Example({ hide = false }) { + return ( + + Trigger + + alice + {!hide && bob} + {!hide && charlie} + + + ) + } + + let { rerender } = render() + + // Open the menu + await click(getMenuButton()) + assertMenu({ state: MenuState.Visible }) + + // Navigate to the last item + await press(Keys.End) + let items = getMenuItems() + assertMenuLinkedWithMenuItem(items[2]) + + // Remove items while the menu is open + rerender() + + // Press Enter — should not crash, should close the menu gracefully + await press(Keys.Enter) + assertMenu({ state: MenuState.InvisibleUnmounted }) + }) + ) + + it( + 'should adjust activeItemIndex when items before the active item are removed', + suppressConsoleLogs(async () => { + function Example({ hide = false }) { + return ( + + Trigger + + {!hide && alice} + bob + charlie + + + ) + } + + let { rerender } = render() + + // Open the menu + await click(getMenuButton()) + assertMenu({ state: MenuState.Visible }) + + // Navigate to the last item ("charlie") + await press(Keys.End) + let items = getMenuItems() + assertMenuLinkedWithMenuItem(items[2]) + + // Remove the first item ("alice") while "charlie" is active + rerender() + + // "charlie" should still be the active item (now at index 1) + items = getMenuItems() + expect(items).toHaveLength(2) + assertMenuLinkedWithMenuItem(items[1]) + }) + ) +}) diff --git a/packages/@headlessui-react/src/components/menu/menu.tsx b/packages/@headlessui-react/src/components/menu/menu.tsx index 6356320728..a14f251861 100644 --- a/packages/@headlessui-react/src/components/menu/menu.tsx +++ b/packages/@headlessui-react/src/components/menu/menu.tsx @@ -473,7 +473,9 @@ function ItemsFn( event.preventDefault() event.stopPropagation() if (machine.state.activeItemIndex !== null) { - let { dataRef } = machine.state.items[machine.state.activeItemIndex] + let activeItem = machine.state.items[machine.state.activeItemIndex] + if (!activeItem) break + let { dataRef } = activeItem dataRef.current?.domRef.current?.click() } machine.send({ type: ActionTypes.CloseMenu })