dequelabs / dequelabs/cauldron
Listbox: make activeOption a properly controlled prop
- Dominant language
- TypeScript
- Stars
- 127
- Forks
- 31
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 8
Description
## Problem
`Listbox` accepts `activeOption` as a controlled prop, but continues to write its own local copy from 13 internal call sites (`packages/react/src/components/Listbox/Listbox.tsx`). Nothing gates those writes on whether the prop is present.
Its other controlled prop, `value`, does gate internal writes on `!isControlled`, and `Combobox` follows the same pattern. `activeOption` is the outlier — which means the idiomatic controlled usage is the unsafe one: a parent that holds `activeOption` and syncs it back down is fighting the component for ownership of the same state.
That two-way sync is what caused #2512. `Listbox`'s downward sync effect only re-runs when the prop's object identity changes, so under scheduling pressure it can fire late carrying an already-stale value and overwrite a newer local one. The stale value is reported back up, the parent adopts it, and the two copies ping-pong until React's update-depth guard trips.
## What was already done
#2515 fixed #2512 by removing the only affected usage: `ActionList` no longer mirrors `Listbox`'s reported active option back down. It keeps `activeOption` only as a one-shot mnemonic request.
That resolves the reported bug, but it leaves the underlying hazard in place — the prop still invites the exact usage that breaks. A search across the `dequelabs` org found no repo outside cauldron passing `activeOption` to a `Listbox`, so there are no known external consumers relying on the current behavior today.
## Suggested fix
Make `activeOption` behave like `value` does:
1. Gate `Listbox`'s internal `setActiveOption` writes on `!isActiveControlled`, so a controlled parent is the single owner.
2. Give the one-shot "move the active option here, then forget" case its own surface, so `ActionList` (and anyone else) is not overloading a controlled prop to express a command. Either a distinct prop (e.g. `requestActiveOption`) handled by an effect keyed on object identity, or an imperative `setActiveOption` on `Listbox`'s ref.
3. Once (2) exists, `ActionList` can stop passing `activeOption` altogether and the explanatory comments in `ActionList.tsx` can come out — the contract would be expressed in the interface instead.
## Constraint
This is a breaking change to `activeOption`'s behavior. Per `CONTRIBUTING.md` (Deprecating), it needs the prop flagged as deprecated for **at least two months** before the change ships, and any new property must be optional until that period has passed.
## Acceptance criteria
- [ ] `Listbox`'s internal active-option writes are gated on `!isActiveControlled`, matching how `value` / `isControlled` already works.
- [ ] A documented, non-overloaded way exists for a parent to request a one-time active-option move.
- [ ] `ActionList` uses that surface instead of passing `activeOption`.
- [ ] The deprecation was flagged for at least two months before the breaking behavior shipped.
- [ ] `docs/pages/components/Listbox.mdx` describes the final contract for both surfaces.
## Non-goals
- Re-fixing #2512. That is already resolved by #2515; this issue removes the hazard that allowed it, it does not restore any broken behavior.
Follow-up to #2515. Related: #2512.
Contributor guide
Research direction
Start in packages/react/src/components/Listbox/Listbox.tsx and inspect the 13 active-option write sites alongside the existing value/isControlled handling and Combobox pattern. Then read ActionList.tsx, CONTRIBUTING.md’s Deprecating guidance, and docs/pages/components/Listbox.mdx. Done means controlled writes are gated, ActionList uses a documented one-shot surface, the final contract is documented, and the deprecation period is met.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- api, frontend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100