Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve ListBox perf caused by excessive re-rendering #8010

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/@react-stately/combobox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"@swc/helpers": "^0.5.0"
},
"peerDependencies": {
"react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1"
"react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1",
"react-dom": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1"
},
"publishConfig": {
"access": "public"
Expand Down
20 changes: 15 additions & 5 deletions packages/@react-stately/form/src/useFormValidationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,20 @@ export interface FormValidationState {
}

export function useFormValidationState<T>(props: FormValidationProps<T>): FormValidationState {
// Private prop for parent components to pass state to children.
if (props[privateValidationStateProp]) {
let {realtimeValidation, displayValidation, updateValidation, resetValidation, commitValidation} = props[privateValidationStateProp] as FormValidationState;
const privateProp = props[privateValidationStateProp];

let privateState = useMemo(() => {
if (!privateProp) {
return null;
}

let {realtimeValidation, displayValidation, updateValidation, resetValidation, commitValidation} = privateProp as FormValidationState;
return {realtimeValidation, displayValidation, updateValidation, resetValidation, commitValidation};
}, [privateProp]);

// Private prop for parent components to pass state to children.
if (privateState) {
return privateState;
}

// eslint-disable-next-line react-hooks/rules-of-hooks
Expand Down Expand Up @@ -152,7 +162,7 @@ function useFormValidationStateImpl<T>(props: FormValidationProps<T>): FormValid
? controlledError || serverError || currentValidity
: controlledError || serverError || clientError || builtinValidation || currentValidity;

return {
return useMemo(() => ({
realtimeValidation,
displayValidation,
updateValidation(value) {
Expand Down Expand Up @@ -188,7 +198,7 @@ function useFormValidationStateImpl<T>(props: FormValidationProps<T>): FormValid
}
setServerErrorCleared(true);
}
};
}), [realtimeValidation, displayValidation, validationBehavior, currentValidity]);
}

function asArray<T>(v: T | T[]): T[] {
Expand Down
15 changes: 8 additions & 7 deletions packages/@react-stately/list/src/useListState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,26 @@ export function useListState<T extends object>(props: ListProps<T>): ListState<T

useFocusedKeyReset(collection, selectionManager);

return {
return useMemo(() => ({
collection,
disabledKeys,
selectionManager
};
}), [collection, disabledKeys, selectionManager]);
}

/**
* Filters a collection using the provided filter function and returns a new ListState.
*/
export function UNSTABLE_useFilteredListState<T extends object>(state: ListState<T>, filter: ((nodeValue: string) => boolean) | null | undefined): ListState<T> {
let collection = useMemo(() => filter ? state.collection.UNSTABLE_filter!(filter) : state.collection, [state.collection, filter]);
let selectionManager = state.selectionManager.withCollection(collection);
let selectionManager = useMemo(() => state.selectionManager.withCollection(collection), [state, collection]);
useFocusedKeyReset(collection, selectionManager);
return {

return useMemo(() => ({
collection,
selectionManager,
disabledKeys: state.disabledKeys
};
disabledKeys: state.disabledKeys,
selectionManager
}), [collection, state.disabledKeys, selectionManager]);
}

function useFocusedKeyReset<T>(collection: Collection<Node<T>>, selectionManager: SelectionManager) {
Expand Down
15 changes: 8 additions & 7 deletions packages/@react-stately/list/src/useSingleSelectListState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

import {CollectionStateBase, Key, Node, Selection, SingleSelection} from '@react-types/shared';
import {ListState, useListState} from './useListState';
import {useCallback, useMemo} from 'react';
import {useControlledState} from '@react-stately/utils';
import {useMemo} from 'react';

export interface SingleSelectListProps<T> extends CollectionStateBase<T>, Omit<SingleSelection, 'disallowEmptySelection'> {
/** Filter function to generate a filtered list of nodes. */
Expand All @@ -40,13 +40,14 @@ export interface SingleSelectListState<T> extends ListState<T> {
export function useSingleSelectListState<T extends object>(props: SingleSelectListProps<T>): SingleSelectListState<T> {
let [selectedKey, setSelectedKey] = useControlledState(props.selectedKey, props.defaultSelectedKey ?? null, props.onSelectionChange);
let selectedKeys = useMemo(() => selectedKey != null ? [selectedKey] : [], [selectedKey]);
let onSelectionChange = props.onSelectionChange;
let {collection, disabledKeys, selectionManager} = useListState({
...props,
selectionMode: 'single',
disallowEmptySelection: true,
allowDuplicateSelectionEvents: true,
selectedKeys,
onSelectionChange: (keys: Selection) => {
onSelectionChange: useCallback((keys: Selection) => {
// impossible, but TS doesn't know that
if (keys === 'all') {
return;
Expand All @@ -55,24 +56,24 @@ export function useSingleSelectListState<T extends object>(props: SingleSelectLi

// Always fire onSelectionChange, even if the key is the same
// as the current key (useControlledState does not).
if (key === selectedKey && props.onSelectionChange) {
props.onSelectionChange(key);
if (key === selectedKey && onSelectionChange) {
onSelectionChange(key);
}

setSelectedKey(key);
}
}, [onSelectionChange, selectedKey, setSelectedKey])
});

let selectedItem = selectedKey != null
? collection.getItem(selectedKey)
: null;

return {
return useMemo(() => ({
collection,
disabledKeys,
selectionManager,
selectedKey,
setSelectedKey,
selectedItem
};
}), [collection, disabledKeys, selectionManager, selectedKey, setSelectedKey, selectedItem]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*/

import {OverlayTriggerProps} from '@react-types/overlays';
import {useCallback} from 'react';
import {useCallback, useMemo} from 'react';
import {useControlledState} from '@react-stately/utils';

export interface OverlayTriggerState {
Expand Down Expand Up @@ -46,11 +46,11 @@ export function useOverlayTriggerState(props: OverlayTriggerProps): OverlayTrigg
setOpen(!isOpen);
}, [setOpen, isOpen]);

return {
return useMemo(() => ({
isOpen,
setOpen,
open,
close,
toggle
};
}), [isOpen, setOpen, open, close, toggle]);
}
4 changes: 3 additions & 1 deletion packages/@react-stately/select/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"url": "https://github.com/adobe/react-spectrum"
},
"dependencies": {
"@react-aria/utils": "^3.28.1",
"@react-stately/form": "^3.1.2",
"@react-stately/list": "^3.12.0",
"@react-stately/overlays": "^3.6.14",
Expand All @@ -30,7 +31,8 @@
"@swc/helpers": "^0.5.0"
},
"peerDependencies": {
"react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1"
"react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1",
"react-dom": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1"
},
"publishConfig": {
"access": "public"
Expand Down
26 changes: 18 additions & 8 deletions packages/@react-stately/select/src/useSelectState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ import {FormValidationState, useFormValidationState} from '@react-stately/form';
import {OverlayTriggerState, useOverlayTriggerState} from '@react-stately/overlays';
import {SelectProps} from '@react-types/select';
import {SingleSelectListState, useSingleSelectListState} from '@react-stately/list';
import {useState} from 'react';
import {useCallback, useMemo, useRef, useState} from 'react';
import {useLayoutEffect} from '@react-aria/utils';

export interface SelectStateOptions<T> extends Omit<SelectProps<T>, 'children'>, CollectionStateBase<T> {}

Expand Down Expand Up @@ -44,26 +45,35 @@ export interface SelectState<T> extends SingleSelectListState<T>, OverlayTrigger
export function useSelectState<T extends object>(props: SelectStateOptions<T>): SelectState<T> {
let triggerState = useOverlayTriggerState(props);
let [focusStrategy, setFocusStrategy] = useState<FocusStrategy | null>(null);

// This is necessary to circumvent a circular dependency below
let validationStateRef = useRef<FormValidationState>(null);

let onSelectionChange = props.onSelectionChange;
let listState = useSingleSelectListState({
...props,
onSelectionChange: (key) => {
if (props.onSelectionChange != null) {
props.onSelectionChange(key);
onSelectionChange: useCallback(key => {
if (onSelectionChange != null) {
onSelectionChange(key);
}

triggerState.close();
validationState.commitValidation();
}
validationStateRef.current!.commitValidation();
}, [onSelectionChange, triggerState])
});

let validationState = useFormValidationState({
...props,
value: listState.selectedKey
});
useLayoutEffect(() => {
validationStateRef.current = validationState;
}, [validationState]);


let [isFocused, setFocused] = useState(false);

return {
return useMemo(() => ({
...validationState,
...listState,
...triggerState,
Expand All @@ -83,5 +93,5 @@ export function useSelectState<T extends object>(props: SelectStateOptions<T>):
},
isFocused,
setFocused
};
}), [validationState, listState, triggerState, focusStrategy, isFocused]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function useMultipleSelectionState(props: MultipleSelectionStateProps): M
}
}, [selectionBehaviorProp]);

return {
return useMemo(() => ({
selectionMode,
disallowEmptySelection,
selectionBehavior,
Expand Down Expand Up @@ -116,7 +116,7 @@ export function useMultipleSelectionState(props: MultipleSelectionStateProps): M
},
disabledKeys: disabledKeysProp,
disabledBehavior
};
}), [selectionMode, disallowEmptySelection, selectionBehavior, selectedKeys, setSelectedKeys, allowDuplicateSelectionEvents, disabledKeysProp, disabledBehavior]);
}

function convertSelection(selection: 'all' | Iterable<Key> | null | undefined, defaultValue?: Selection): 'all' | Set<Key> | undefined {
Expand Down
9 changes: 7 additions & 2 deletions packages/react-aria-components/docs/TagGroup.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,14 @@ import {ListStateContext} from 'react-aria-components';

function SelectionCount() {
/*- begin highlight -*/
let state = React.useContext(ListStateContext);
let context = React.useContext(ListStateContext);
/*- end highlight -*/
let selected = state?.selectionManager.selectedKeys.size ?? 0;
let selected = 0;
if(context) {
let [state] = context;
selected = state.selectionManager.selectedKeys.size;
}

return <small>{selected} tags selected.</small>;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react-aria-components/src/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ function ComboBoxInner<T extends object>({props, collection, comboBoxRef: ref}:
style: {'--trigger-width': menuWidth} as React.CSSProperties
}],
[ListBoxContext, {...listBoxProps, ref: listBoxRef}],
[ListStateContext, state],
[ListStateContext, [state, state.selectionManager.focusedKey]],
[TextContext, {
slots: {
description: descriptionProps,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-aria-components/src/GridList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ function GridListInner<T extends object>({props, collection, gridListRef: ref}:
data-layout={layout}>
<Provider
values={[
[ListStateContext, state],
[ListStateContext, [state, state.selectionManager.focusedKey]],
[DragAndDropContext, {dragAndDropHooks, dragState, dropState}],
[DropIndicatorContext, {render: GridListDropIndicatorWrapper}]
]}>
Expand Down Expand Up @@ -277,7 +277,7 @@ export interface GridListItemProps<T = object> extends RenderProps<GridListItemR
* A GridListItem represents an individual item in a GridList.
*/
export const GridListItem = /*#__PURE__*/ createLeafComponent('item', function GridListItem<T extends object>(props: GridListItemProps<T>, forwardedRef: ForwardedRef<HTMLDivElement>, item: Node<T>) {
let state = useContext(ListStateContext)!;
let [state] = useContext(ListStateContext)!;
let {dragAndDropHooks, dragState, dropState} = useContext(DragAndDropContext);
let ref = useObjectRef<HTMLDivElement>(forwardedRef);
let {isVirtualized} = useContext(CollectionRendererContext);
Expand Down
32 changes: 23 additions & 9 deletions packages/react-aria-components/src/ListBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {DraggableCollectionState, DroppableCollectionState, ListState, Node, Ori
import {filterDOMProps, mergeRefs, useObjectRef} from '@react-aria/utils';
import {forwardRefType, HoverEvents, Key, LinkDOMProps, RefObject} from '@react-types/shared';
import {HeaderContext} from './Header';
import React, {createContext, ForwardedRef, forwardRef, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react';
import React, {createContext, ForwardedRef, forwardRef, JSX, memo, ReactNode, useContext, useEffect, useMemo, useRef} from 'react';
import {SeparatorContext} from './Separator';
import {TextContext} from './Text';
import {UNSTABLE_InternalAutocompleteContext} from './Autocomplete';
Expand Down Expand Up @@ -78,22 +78,24 @@ export interface ListBoxProps<T> extends Omit<AriaListBoxProps<T>, 'children' |
}

export const ListBoxContext = createContext<ContextValue<ListBoxProps<any>, HTMLDivElement>>(null);
export const ListStateContext = createContext<ListState<any> | null>(null);
export const ListStateContext = createContext<[ListState<any>, Key | null] | null>(null);

/**
* A listbox displays a list of options and allows a user to select one or more of them.
*/
export const ListBox = /*#__PURE__*/ (forwardRef as forwardRefType)(function ListBox<T extends object>(props: ListBoxProps<T>, ref: ForwardedRef<HTMLDivElement>) {
[props, ref] = useContextProps(props, ref, ListBoxContext);
let state = useContext(ListStateContext);
let context = useContext(ListStateContext);

// The structure of ListBox is a bit strange because it needs to work inside other components like ComboBox and Select.
// Those components render two copies of their children so that the collection can be built even when the popover is closed.
// The first copy sends a collection document via context which we render the collection portal into.
// The second copy sends a ListState object via context which we use to render the ListBox without rebuilding the state.
// Otherwise, we have a standalone ListBox, so we need to create a collection and state ourselves.

if (state) {
if (context) {
let [state] = context;

return <ListBoxInner state={state} props={props} listBoxRef={ref} />;
}

Expand Down Expand Up @@ -248,8 +250,8 @@ function ListBoxInner<T extends object>({state: inputState, props, listBoxRef}:
<Provider
values={[
[ListBoxContext, props],
[ListStateContext, state],
[DragAndDropContext, {dragAndDropHooks, dragState, dropState}],
[ListStateContext, [state, state.selectionManager.focusedKey]],
[DragAndDropContext, useMemo(() => ({dragAndDropHooks, dragState, dropState}), [dragAndDropHooks, dragState, dropState])],
[SeparatorContext, {elementType: 'div'}],
[DropIndicatorContext, {render: ListBoxDropIndicatorWrapper}],
[SectionContext, {name: 'ListBoxSection', render: ListBoxSectionInner}]
Expand All @@ -270,7 +272,7 @@ function ListBoxInner<T extends object>({state: inputState, props, listBoxRef}:
export interface ListBoxSectionProps<T> extends SectionProps<T> {}

function ListBoxSectionInner<T extends object>(props: ListBoxSectionProps<T>, ref: ForwardedRef<HTMLElement>, section: Node<T>, className = 'react-aria-ListBoxSection') {
let state = useContext(ListStateContext)!;
let [state] = useContext(ListStateContext)!;
let {dragAndDropHooks, dropState} = useContext(DragAndDropContext)!;
let {CollectionBranch} = useContext(CollectionRendererContext);
let [headingRef, heading] = useSlot();
Expand Down Expand Up @@ -331,14 +333,26 @@ export interface ListBoxItemProps<T = object> extends RenderProps<ListBoxItemRen
*/
export const ListBoxItem = /*#__PURE__*/ createLeafComponent('item', function ListBoxItem<T extends object>(props: ListBoxItemProps<T>, forwardedRef: ForwardedRef<HTMLDivElement>, item: Node<T>) {
let ref = useObjectRef<any>(forwardedRef);
let state = useContext(ListStateContext)!;
let [state, focusedKey] = useContext(ListStateContext)!;

// ListBoxItemInner is memoized so that a focus change does not re-render all items in the ListBox.
// The data-focused attribute tells React which list boxes are affected by a focus change. It does not actually
// get passed through to the component - it could be named anything, as long as it changes so React knows to re-render.
return <ListBoxItemInner data-focused={item.key === focusedKey} props={props} state={state} passRef={ref} item={item} />;
});

const ListBoxItemInner = memo(function ListBoxItemInner<T extends object>({props, item, state, passRef}: {props: ListBoxItemProps<T>, state: ListState<T>, item: Node<T>, passRef: React.MutableRefObject<any>}) {
const ref = passRef;

let {dragAndDropHooks, dragState, dropState} = useContext(DragAndDropContext)!;
let {optionProps, labelProps, descriptionProps, ...states} = useOption(
let options = useOption(
{key: item.key, 'aria-label': props?.['aria-label']},
state,
ref
);

let {optionProps, labelProps, descriptionProps, ...states} = options;

let {hoverProps, isHovered} = useHover({
isDisabled: !states.allowsSelection && !states.hasAction,
onHoverStart: item.props.onHoverStart,
Expand Down
2 changes: 1 addition & 1 deletion packages/react-aria-components/src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ function SelectInner<T extends object>({props, selectRef: ref, collection}: Sele
'aria-labelledby': menuProps['aria-labelledby']
}],
[ListBoxContext, {...menuProps, ref: scrollRef}],
[ListStateContext, state],
[ListStateContext, [state, state.selectionManager.focusedKey]],
[TextContext, {
slots: {
description: descriptionProps,
Expand Down
Loading