dequelabs / dequelabs/cauldron

Listbox: make activeOption a properly controlled prop

Open
#2,522 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.