Skip to content

Commit e475a82

Browse files
Fix Maximum update depth exceeded due to Transition (#3782)
This PR fixes an issue where you could run into a `Maximum update depth exceeded` error in the `Listbox`, `Menu` or `Combobox` component in combination with the `transition` prop. It seems like some people ran into this, but I was never able to reproduce it. But now I figured out what the issue is, how to reproduce it and this PR should fix it as well (it at least fixes the `Maximum update depth exceeded` I could reproduce). To reproduce it, you don't need a lot of code, but you do need a particular setup: ```tsx import { Listbox, ListboxButton, ListboxOption, ListboxOptions } from '@headlessui/react' import { useState } from 'react' let options = ['apple', 'banana', 'orange', 'grape', 'pear'] export default function App() { let [value, setValue] = useState('apple') return ( <> {/* Required to reproduce the issue */} <div className="min-h-dvh" /> <Listbox value={value} onChange={setValue}> <ListboxButton>{value}</ListboxButton> <ListboxOptions transition> {options.map((option) => ( <ListboxOption key={option} value={option}> {option} </ListboxOption> ))} </ListboxOptions> </Listbox> </> ) } ``` This setup means that you have to scroll down to see the Listbox. When you open the Listbox, the `ListboxOptions` will be rendered increasing the height of the page. If you then close it again, the page will decrease in height again, causing the issue to happen. The next question is, why does the crash happen? The crash happens because we are updating some internal state based on whether the `ListboxButton` moved or not. The reason we track whether the button moved or not is to improve the UX (a bit ironic I know, because it could crash…) In the Listbox component (and some others like Menu and Combobox), we track whether a button moved on the screen when you close the dropdown. If it does move, we will immediately hide the Listbox without any transitions. This is done to improve the UX because we don't want to see a fading-out dropdown somewhere else on the screen. Maybe it's easier if I show you an example. This example is using our UI Kit, Catalyst: https://catalyst.tailwindui.com/docs/listbox **Example 1:** 1. Open the dropdown 2. Click outside or press Escape to close it 3. Notice the subtle fade-out transition https://github.com/user-attachments/assets/0eecffd7-f4fe-4543-8a48-433dd27f1bf7 **Example 2:** 1. Open the dropdown 2. Press tab to focus the next element on the page 3. Notice the subtle fade-out transition https://github.com/user-attachments/assets/3bec2178-1f15-40cd-ab1e-ae8ca333bd92 **Example 3:** Now we're going to do something very similar, but this time the next focusable element will be just outside of what is visual to the user. This will cause the browser to scroll and scroll the new element into view. We'll make the page smaller by opening devtools. 1. Open the dropdown 2. Press tab to focus the next element on the page 3. Notice how the the dropdown is completely gone, no transitions whatsoever https://github.com/user-attachments/assets/4abd7fd4-250b-4f91-a172-5954e9fccae7 **Example 4:** Up until now, all these make sense as a user. But here is an example of what would happen if we didn't close the dropdown immediately when the button moved. Same steps and similar setup as Example 3, but without the special code that detects button moves. It's going to be quick, but look near the end of this video. Once I press tab, you will see the dropdown fade out and linger around near the top of the screen. https://github.com/user-attachments/assets/993fdec5-6a8c-468f-bbf8-6003f7be7048 (This was a little tricky to reproduce here because when you tab, the active element will be scrolled into view automatically by the browser, so the button sits in the middle of the page. But we want to make sure the dropdown button is still in the viewport~ish. This is why it's zoomed out a bit.) --- Now that we established that, why did it crash in some scenarios? The first important part is the `transition` prop. A transition allows us to keep the dropdown visible even though its internal state is "closed". This is important so that we will wait until all CSS transitions are done before actually unmounting the component. The next important part is about the extra div (`<div class="min-h-dvh" />`) in the reproduction code, because it will force a scrollbar. Opening and closing the listbox will change the height of the page. This in turn will move the button up to make room for the dropdown part, therefore the `button` will be marked as moved. However, we keep checking whether the button moved compared to the initial position, which could result in a loop: 0. Close the listbox. 1. Compute button position at **position A** 2. Re-render happens, and button is now at **position B** because the dropdown part is gone 3. Re-render happens, and button is now at **position A** again, because it's considered at the same initial position so instead of instantly hiding we actually show it again because the `visible` state (thanks to the `transition`) is still set to true. 4. Re-render happens, and button is now at **position B** again because now the dropdown is rendered again 5. …**position A** 6. …**position B** 7. … To actually solve the issue, we have to make sure that once the button is marked as moved, it will stay marked as "moved" until the `enabled` boolean (argument to the hook) changes again. I also did some internal refactors by moving this logic into the existing state machines instead of relying on React and when things render. I therefore also got rid of the `useDidElementMove` hook entirely. ## Test plan 2. Error does not reproduce with this code. 2. The expected behavior in Catalyst still works as expected. Simplified reproduction of the crash in action: ### Before https://github.com/user-attachments/assets/46254740-dac6-4d95-ba1a-aef4b4b7082c ### After https://github.com/user-attachments/assets/9ee4350b-3bd0-4f07-b4a5-edb4f3cb3d78 Fixes: #3655 Fixes: #3562 --------- Co-authored-by: Jordan Pittman <[email protected]>
1 parent 46ee372 commit e475a82

File tree

11 files changed

+238
-64
lines changed

11 files changed

+238
-64
lines changed

packages/@headlessui-react/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Ensure we are not freezing data when the `static` prop is used ([#3779](https://github.com/tailwindlabs/headlessui/pull/3779))
1313
- Ensure `onChange` types are contravariant instead of bivariant ([#3781](https://github.com/tailwindlabs/headlessui/pull/3781))
1414
- Support `<summary>` as a focusable element inside `<details>` ([#3389](https://github.com/tailwindlabs/headlessui/pull/3389))
15+
- Fix `Maximum update depth exceeded` crash when using `transition` prop ([#3782](https://github.com/tailwindlabs/headlessui/pull/3782))
1516

1617
## [2.2.7] - 2025-07-30
1718

packages/@headlessui-react/src/components/combobox/combobox-machine.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ import { Machine } from '../../machine'
22
import { ActionTypes as StackActionTypes, stackMachines } from '../../machines/stack-machine'
33
import type { EnsureArray } from '../../types'
44
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
5+
import {
6+
ElementPositionState,
7+
computeVisualPosition,
8+
detectMovement,
9+
} from '../../utils/element-movement'
510
import { sortByDomNode } from '../../utils/focus-management'
611
import { match } from '../../utils/match'
712

@@ -74,6 +79,9 @@ export interface State<T> {
7479
buttonElement: HTMLButtonElement | null
7580
optionsElement: HTMLElement | null
7681

82+
// Track input to determine if it moved
83+
inputPositionState: ElementPositionState
84+
7785
__demoMode: boolean
7886
}
7987

@@ -96,6 +104,8 @@ export enum ActionTypes {
96104
SetInputElement,
97105
SetButtonElement,
98106
SetOptionsElement,
107+
108+
MarkInputAsMoved,
99109
}
100110

101111
function adjustOrderedState<T>(
@@ -160,13 +170,17 @@ type Actions<T> =
160170
| { type: ActionTypes.SetInputElement; element: HTMLInputElement | null }
161171
| { type: ActionTypes.SetButtonElement; element: HTMLButtonElement | null }
162172
| { type: ActionTypes.SetOptionsElement; element: HTMLElement | null }
173+
| { type: ActionTypes.MarkInputAsMoved }
163174

164175
let reducers: {
165176
[P in ActionTypes]: <T>(state: State<T>, action: Extract<Actions<T>, { type: P }>) => State<T>
166177
} = {
167178
[ActionTypes.CloseCombobox](state) {
168179
if (state.dataRef.current?.disabled) return state
169180
if (state.comboboxState === ComboboxState.Closed) return state
181+
let inputPositionState = state.inputElement
182+
? ElementPositionState.Tracked(computeVisualPosition(state.inputElement))
183+
: state.inputPositionState
170184

171185
return {
172186
...state,
@@ -181,6 +195,8 @@ let reducers: {
181195
// for example, not scrolling to the active option in a virtual list
182196
activationTrigger: ActivationTrigger.Other,
183197

198+
inputPositionState,
199+
184200
__demoMode: false,
185201
}
186202
},
@@ -197,11 +213,17 @@ let reducers: {
197213
activeOptionIndex: idx,
198214
comboboxState: ComboboxState.Open,
199215
__demoMode: false,
216+
inputPositionState: ElementPositionState.Idle,
200217
}
201218
}
202219
}
203220

204-
return { ...state, comboboxState: ComboboxState.Open, __demoMode: false }
221+
return {
222+
...state,
223+
comboboxState: ComboboxState.Open,
224+
inputPositionState: ElementPositionState.Idle,
225+
__demoMode: false,
226+
}
205227
},
206228
[ActionTypes.SetTyping](state, action) {
207229
if (state.isTyping === action.isTyping) return state
@@ -404,6 +426,14 @@ let reducers: {
404426
if (state.optionsElement === action.element) return state
405427
return { ...state, optionsElement: action.element }
406428
},
429+
[ActionTypes.MarkInputAsMoved](state) {
430+
if (state.inputPositionState.kind !== 'Tracked') return state
431+
432+
return {
433+
...state,
434+
inputPositionState: ElementPositionState.Moved,
435+
}
436+
},
407437
}
408438

409439
export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
@@ -438,6 +468,7 @@ export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
438468
buttonElement: null,
439469
optionsElement: null,
440470
__demoMode,
471+
inputPositionState: ElementPositionState.Idle,
441472
})
442473
}
443474

@@ -464,6 +495,20 @@ export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
464495
this.on(ActionTypes.OpenCombobox, () => stackMachine.actions.push(id))
465496
this.on(ActionTypes.CloseCombobox, () => stackMachine.actions.pop(id))
466497
}
498+
499+
// Track whether the input moved or not
500+
this.disposables.group((d) => {
501+
this.on(ActionTypes.CloseCombobox, (state) => {
502+
if (!state.inputElement) return
503+
504+
d.dispose()
505+
d.add(
506+
detectMovement(state.inputElement, state.inputPositionState, () => {
507+
this.send({ type: ActionTypes.MarkInputAsMoved })
508+
})
509+
)
510+
})
511+
})
467512
}
468513

469514
actions = {
@@ -640,6 +685,10 @@ export class ComboboxMachine<T> extends Machine<State<T>, Actions<T>> {
640685

641686
return true
642687
},
688+
689+
didInputMove(state: State<T>) {
690+
return state.inputPositionState.kind === 'Moved'
691+
},
643692
}
644693

645694
reduce(state: Readonly<State<T>>, action: Actions<T>): State<T> {

packages/@headlessui-react/src/components/combobox/combobox.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,21 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
12611261
),
12621262
})
12631263

1264+
// We keep track whether the input moved or not, we only check this when the
1265+
// combobox state becomes closed. If the input moved, then we want to cancel
1266+
// pending transitions to prevent that the attached `ComboboxOptions` is still
1267+
// transitioning while the input visually moved away.
1268+
//
1269+
// If we don't cancel these transitions then there will be a period where the
1270+
// `ComboboxOptions` is visible and moving around because it is trying to
1271+
// re-position itself based on the new position.
1272+
let didInputMove = useSlice(machine, machine.selectors.didInputMove)
1273+
1274+
// Now that we know that the input did move or not, we can either disable the
1275+
// panel and all of its transitions, or rely on the `visible` state to hide
1276+
// the panel whenever necessary.
1277+
let panelEnabled = didInputMove ? false : visible
1278+
12641279
useIsoMorphicEffect(() => {
12651280
data.optionsPropsRef.current.static = props.static ?? false
12661281
}, [data.optionsPropsRef, props.static])
@@ -1392,7 +1407,7 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
13921407
slot,
13931408
defaultTag: DEFAULT_OPTIONS_TAG,
13941409
features: OptionsRenderFeatures,
1395-
visible,
1410+
visible: panelEnabled,
13961411
name: 'Combobox.Options',
13971412
})}
13981413
</ComboboxDataContext.Provider>

packages/@headlessui-react/src/components/listbox/listbox-machine.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { Machine, batch } from '../../machine'
22
import { ActionTypes as StackActionTypes, stackMachines } from '../../machines/stack-machine'
33
import { Focus, calculateActiveIndex } from '../../utils/calculate-active-index'
4+
import {
5+
ElementPositionState,
6+
computeVisualPosition,
7+
detectMovement,
8+
} from '../../utils/element-movement'
49
import { sortByDomNode } from '../../utils/focus-management'
510
import { match } from '../../utils/match'
611

@@ -65,6 +70,9 @@ interface State<T> {
6570

6671
pendingShouldSort: boolean
6772
pendingFocus: { focus: Exclude<Focus, Focus.Specific> } | { focus: Focus.Specific; id: string }
73+
74+
// Track button to determine if it moved
75+
buttonPositionState: ElementPositionState
6876
}
6977

7078
export enum ActionTypes {
@@ -82,6 +90,8 @@ export enum ActionTypes {
8290
SetOptionsElement,
8391

8492
SortOptions,
93+
94+
MarkButtonAsMoved,
8595
}
8696

8797
function adjustOrderedState<T>(
@@ -135,19 +145,25 @@ type Actions<T> =
135145
| { type: ActionTypes.SetButtonElement; element: HTMLButtonElement | null }
136146
| { type: ActionTypes.SetOptionsElement; element: HTMLElement | null }
137147
| { type: ActionTypes.SortOptions }
148+
| { type: ActionTypes.MarkButtonAsMoved }
138149

139150
let reducers: {
140151
[P in ActionTypes]: <T>(state: State<T>, action: Extract<Actions<T>, { type: P }>) => State<T>
141152
} = {
142153
[ActionTypes.CloseListbox](state) {
143154
if (state.dataRef.current.disabled) return state
144155
if (state.listboxState === ListboxStates.Closed) return state
156+
let buttonPositionState = state.buttonElement
157+
? ElementPositionState.Tracked(computeVisualPosition(state.buttonElement))
158+
: state.buttonPositionState
159+
145160
return {
146161
...state,
147162
activeOptionIndex: null,
148163
pendingFocus: { focus: Focus.Nothing },
149164
listboxState: ListboxStates.Closed,
150165
__demoMode: false,
166+
buttonPositionState,
151167
}
152168
},
153169
[ActionTypes.OpenListbox](state, action) {
@@ -169,6 +185,7 @@ let reducers: {
169185
listboxState: ListboxStates.Open,
170186
activeOptionIndex,
171187
__demoMode: false,
188+
buttonPositionState: ElementPositionState.Idle,
172189
}
173190
},
174191
[ActionTypes.GoToOption](state, action) {
@@ -394,6 +411,14 @@ let reducers: {
394411
pendingShouldSort: false,
395412
}
396413
},
414+
[ActionTypes.MarkButtonAsMoved](state) {
415+
if (state.buttonPositionState.kind !== 'Tracked') return state
416+
417+
return {
418+
...state,
419+
buttonPositionState: ElementPositionState.Moved,
420+
}
421+
},
397422
}
398423

399424
export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
@@ -412,6 +437,7 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
412437
pendingShouldSort: false,
413438
pendingFocus: { focus: Focus.Nothing },
414439
__demoMode,
440+
buttonPositionState: ElementPositionState.Idle,
415441
})
416442
}
417443

@@ -447,6 +473,20 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
447473
this.on(ActionTypes.OpenListbox, () => stackMachine.actions.push(id))
448474
this.on(ActionTypes.CloseListbox, () => stackMachine.actions.pop(id))
449475
}
476+
477+
// Track whether the button moved or not
478+
this.disposables.group((d) => {
479+
this.on(ActionTypes.CloseListbox, (state) => {
480+
if (!state.buttonElement) return
481+
482+
d.dispose()
483+
d.add(
484+
detectMovement(state.buttonElement, state.buttonPositionState, () => {
485+
this.send({ type: ActionTypes.MarkButtonAsMoved })
486+
})
487+
)
488+
})
489+
})
450490
}
451491

452492
actions = {
@@ -566,6 +606,10 @@ export class ListboxMachine<T> extends Machine<State<T>, Actions<T>> {
566606
if (state.activationTrigger === ActivationTrigger.Pointer) return false
567607
return this.isActive(state, id)
568608
},
609+
610+
didButtonMove(state: State<T>) {
611+
return state.buttonPositionState.kind === 'Moved'
612+
},
569613
}
570614

571615
reduce(state: Readonly<State<T>>, action: Actions<T>): State<T> {

packages/@headlessui-react/src/components/listbox/listbox.tsx

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import { useActivePress } from '../../hooks/use-active-press'
2424
import { useByComparator, type ByComparator } from '../../hooks/use-by-comparator'
2525
import { useControllable } from '../../hooks/use-controllable'
2626
import { useDefaultValue } from '../../hooks/use-default-value'
27-
import { useDidElementMove } from '../../hooks/use-did-element-move'
2827
import { useDisposables } from '../../hooks/use-disposables'
2928
import { useElementSize } from '../../hooks/use-element-size'
3029
import { useEvent } from '../../hooks/use-event'
@@ -610,20 +609,19 @@ function OptionsFn<TTag extends ElementType = typeof DEFAULT_OPTIONS_TAG>(
610609
allowed: useCallback(() => [buttonElement, optionsElement], [buttonElement, optionsElement]),
611610
})
612611

613-
// We keep track whether the button moved or not, we only check this when the menu state becomes
614-
// closed. If the button moved, then we want to cancel pending transitions to prevent that the
615-
// attached `MenuItems` is still transitioning while the button moved away.
612+
// We keep track whether the button moved or not, we only check this when the
613+
// listbox state becomes closed. If the button moved, then we want to cancel
614+
// pending transitions to prevent that the attached `ListboxOptions` is
615+
// still transitioning while the button visually moved away.
616616
//
617-
// If we don't cancel these transitions then there will be a period where the `MenuItems` is
618-
// visible and moving around because it is trying to re-position itself based on the new position.
619-
//
620-
// This can be solved by only transitioning the `opacity` instead of everything, but if you _do_
621-
// want to transition the y-axis for example you will run into the same issue again.
622-
let didElementMoveEnabled = listboxState !== ListboxStates.Open
623-
let didButtonMove = useDidElementMove(didElementMoveEnabled, buttonElement)
624-
625-
// Now that we know that the button did move or not, we can either disable the panel and all of
626-
// its transitions, or rely on the `visible` state to hide the panel whenever necessary.
617+
// If we don't cancel these transitions then there will be a period where the
618+
// `ListboxOptions` is visible and moving around because it is trying to
619+
// re-position itself based on the new position.
620+
let didButtonMove = useSlice(machine, machine.selectors.didButtonMove)
621+
622+
// Now that we know that the button did move or not, we can either disable the
623+
// panel and all of its transitions, or rely on the `visible` state to hide
624+
// the panel whenever necessary.
627625
let panelEnabled = didButtonMove ? false : visible
628626

629627
// We should freeze when the listbox is visible but "closed". This means that

0 commit comments

Comments
 (0)