diff --git a/.changeset/funny-areas-doubt.md b/.changeset/funny-areas-doubt.md new file mode 100644 index 00000000000..63a6cd2c1ac --- /dev/null +++ b/.changeset/funny-areas-doubt.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Remove render phase setStates on SelectPanel diff --git a/.changeset/some-zebras-roll.md b/.changeset/some-zebras-roll.md new file mode 100644 index 00000000000..0c838d06637 --- /dev/null +++ b/.changeset/some-zebras-roll.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Improve SelectPanel performance diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index 112327b846c..e015be1a6d7 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -215,8 +215,6 @@ function Panel({ const [needsNoItemsAnnouncement, setNeedsNoItemsAnnouncement] = useState(false) const isNarrowScreenSize = useResponsiveValue({narrow: true, regular: false, wide: false}, false) const [selectedOnSort, setSelectedOnSort] = useState([]) - const [prevItems, setPrevItems] = useState([]) - const [prevOpen, setPrevOpen] = useState(open) const initialHeightRef = useRef(0) const initialScaleRef = useRef(1) const noticeRef = useRef(null) @@ -317,6 +315,19 @@ function Panel({ ], ) + // Pre-compute a Set of item IDs in the current filtered view for O(1) lookups + const itemsInViewSet = useMemo(() => { + const set = new Set() + for (const item of items) { + if (item.id !== undefined) { + set.add(item.id) + } else { + set.add(item) + } + } + return set + }, [items]) + const handleSelectAllChange = useCallback( (checked: boolean) => { // Exit early if not in multi-select mode @@ -327,9 +338,13 @@ function Panel({ const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange'] const selectedArray = selected as ItemInput[] - const selectedItemsNotInFilteredView = selectedArray.filter( - (selectedItem: ItemInput) => !items.some(item => areItemsEqual(item, selectedItem)), - ) + // Use Set for O(1) lookup instead of O(n) items.some() + const selectedItemsNotInFilteredView = selectedArray.filter((selectedItem: ItemInput) => { + if (selectedItem.id !== undefined) { + return !itemsInViewSet.has(selectedItem.id) + } + return !itemsInViewSet.has(selectedItem) + }) if (checked) { multiSelectOnChange([...selectedItemsNotInFilteredView, ...items]) @@ -337,7 +352,7 @@ function Panel({ multiSelectOnChange(selectedItemsNotInFilteredView) } }, - [items, onSelectedChange, selected], + [items, itemsInViewSet, onSelectedChange, selected], ) // disable body scroll when the panel is open on narrow screens @@ -535,11 +550,30 @@ function Panel({ } }, [placeholder, renderAnchor, selected]) + // Pre-compute a Set of selected item IDs/references for O(1) lookups + // This optimizes isItemCurrentlySelected from O(m) to O(1) per call + const selectedItemsSet = useMemo(() => { + const set = new Set() + if (isMultiSelectVariant(selected)) { + for (const item of selected) { + if (item.id !== undefined) { + set.add(item.id) + } else { + set.add(item) + } + } + } + return set + }, [selected]) + const isItemCurrentlySelected = useCallback( (item: ItemInput) => { - // For multi-select, we just need to check if the item is in the selected array + // For multi-select, use the pre-computed Set for O(1) lookup if (isMultiSelectVariant(selected)) { - return doesItemsIncludeItem(selected, item) + if (item.id !== undefined) { + return selectedItemsSet.has(item.id) + } + return selectedItemsSet.has(item) } // For single-select modal, there is an intermediate state when the user has selected @@ -553,10 +587,28 @@ function Panel({ // For single-select anchored, we just need to check if the item is the selected item return selected?.id !== undefined ? selected.id === item.id : selected === item }, - [selected, intermediateSelected, isSingleSelectModal], + [selected, selectedItemsSet, intermediateSelected, isSingleSelectModal], ) const itemsToRender = useMemo(() => { + // Pre-compute a Set of selected item IDs/references for O(1) lookups during sorting + // This avoids O(m * k) work per comparison in the sort function + const selectedOnSortSet = new Set() + for (const item of selectedOnSort) { + if (item.id !== undefined) { + selectedOnSortSet.add(item.id) + } else { + selectedOnSortSet.add(item) + } + } + + const isSelectedForSort = (item: ItemProps): boolean => { + if (item.id !== undefined) { + return selectedOnSortSet.has(item.id) + } + return selectedOnSortSet.has(item as unknown as ItemInput) + } + return items .map(item => { return { @@ -600,30 +652,13 @@ function Panel({ }) .sort((itemA, itemB) => { if (shouldOrderSelectedFirst) { - // itemA is selected (for sorting purposes) if an object in selectedOnSort matches every property of itemA, except for the selected property - const itemASelected = selectedOnSort.some(item => - Object.entries(item).every(([key, value]) => { - if (key === 'selected') { - return true - } - return itemA[key as keyof ItemProps] === value - }), - ) - - // itemB is selected (for sorting purposes) if an object in selectedOnSort matches every property of itemA, except for the selected property - const itemBSelected = selectedOnSort.some(item => - Object.entries(item).every(([key, value]) => { - if (key === 'selected') { - return true - } - return itemB[key as keyof ItemProps] === value - }), - ) + const itemASelected = isSelectedForSort(itemA) + const itemBSelected = isSelectedForSort(itemB) // order selected items first - if (itemASelected > itemBSelected) { + if (itemASelected && !itemBSelected) { return -1 - } else if (itemASelected < itemBSelected) { + } else if (!itemASelected && itemBSelected) { return 1 } } @@ -642,17 +677,25 @@ function Panel({ selectedOnSort, ]) - if (prevItems !== items) { - setPrevItems(items) - if (prevItems.length === 0 && items.length > 0) { - resetSort() + // Track previous items and reset sort when items first load + const prevItemsRef = useRef(items) + useEffect(() => { + if (prevItemsRef.current !== items) { + if (prevItemsRef.current.length === 0 && items.length > 0) { + resetSort() + } + prevItemsRef.current = items } - } + }, [items, resetSort]) - if (open !== prevOpen) { - setPrevOpen(open) - resetSort() - } + // Reset sort when panel opens + const prevOpenRef = useRef(open) + useEffect(() => { + if (prevOpenRef.current !== open) { + resetSort() + prevOpenRef.current = open + } + }, [open, resetSort]) const focusTrapSettings = { initialFocusRef: inputRef || undefined,