feat(use-collection): add shared utilities for collection-based primitives #74
Conversation
|
- useSafeObjectRef: SSR-safe ref handling (fixes Vitest browser mode frozen refs) - useCollectionState: rules-of-hooks compliant state management - renderWithOptionalContext: generic context wrapper for standalone/composed usage - useKeyboardDelegate: memoized keyboard delegate with explicit deps This package extracts shared utilities from @bento/listbox and @bento/menu, providing a common foundation for collection-based primitives.
kawikabader
force-pushed
the
use-collection
branch
from
43f2f79 to
7d998dd
Compare
| } | ||
| ``` | ||
|
|
||
| ### `renderWithOptionalContext<TState>(content, state, contextState, StateContext)` |
There was a problem hiding this comment.
does this really belong in a utility called use-collection, it doesn't seem to have anything to do with that specific functionality, and IMHO would be better suited as standalone.
There was a problem hiding this comment.
Good point on naming clarity. While renderWithOptionalContext looks generic, it's actually solving a collection-specific pattern: ListBox and Menu both need to work both standalone AND composed within Select/Menu etc.
The dual-mode behavior (check for context, wrap if missing) is unique to collection primitives. Generic components typically don't have this requirement.
That said, I'm open to two alternatives if you feel strongly:
- Rename to
renderCollectionWithContextto make the use case explicit - Move to separate package if we anticipate non-collection components needing this pattern
What's your preference?
There was a problem hiding this comment.
Going to leave that up to the team decide, just wanted to call this out specifically as it doesn't really fit in a hook package.
There was a problem hiding this comment.
I'm just not sure if this abstraction belongs in a hook outside of the current usage. Also, there seems to be a lot of exports in use-collection....that seem loosely associated with interacting with collections.
|
From an organizational POV, I'd worry about us identifying a pattern too quickly here. I think it's likely we find this is not used as often as we think or that we need to make specific changes for different compositions. I'm under the impression that it would be better to have the separate imports for now opposed to a single collect project. |
| import { forwardRef } from 'react'; | ||
|
|
||
| const MyComponent = forwardRef<HTMLDivElement, Props>((props, ref) => { | ||
| const safeRef = useSafeObjectRef(ref); |
There was a problem hiding this comment.
Does React Aria's useObjectRef not account/solve for this? https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/utils/src/useObjectRef.ts
| } | ||
| ``` | ||
|
|
||
| ### `useKeyboardDelegate(config): ListKeyboardDelegate<unknown>` |
There was a problem hiding this comment.
I could be wrong, but isn't the KeyboardDelegate used specifically for "search/typeahead" and not for navigating collections?
|
|
||
| Generic state hook for collection components. Always calls the state hook unconditionally (rules-of-hooks compliance), then chooses context state if available. | ||
|
|
||
| **Fixes rules-of-hooks violation**: The previous pattern `contextState ?? useListState(props)` conditionally called hooks based on `contextState`, which violates React's rules. This hook always calls `useStateHook` unconditionally. |
There was a problem hiding this comment.
Is the primary use of the hook to just avoid contextState ?? useListState(props) and defining the undefined props for children and items?
| } | ||
| ``` | ||
|
|
||
| ### `renderWithOptionalContext<TState>(content, state, contextState, StateContext)` |
There was a problem hiding this comment.
I'm just not sure if this abstraction belongs in a hook outside of the current usage. Also, there seems to be a lot of exports in use-collection....that seem loosely associated with interacting with collections.
Summary
Introduces
@bento/use-collection, a utility package providing shared hooks for collection primitives (ListBox, Menu, etc).Why this change is needed: Collection components were duplicating patterns for React Aria integration, state management, and SSR-safe ref handling. Centralizing these patterns eliminates duplication and ensures consistency.
Impact: Reduces maintenance burden, prevents implementation inconsistencies, and provides reusable utilities for current and future collection primitives.
Changes Made
useSafeObjectRef<T>- SSR-safe ref wrapper for React Aria hooks (handles Vitest browser mode frozen objects)useCollectionState<TState>- Rules-of-hooks compliant state hook with context fallbackrenderWithOptionalContext<TState>- Helper for standalone vs composed usage patternsuseKeyboardDelegate- Memoized keyboard delegate factory delegating to React Aria