fix: improve command list perf

sanity-io · Dec 13, 2024 · 60d3f65 · 60d3f65
1 parent 9d075f4
commit 60d3f65
Showing 1 changed file with 117 additions and 50 deletions.
diff --git a/packages/sanity/src/core/components/commandList/CommandList.tsx b/packages/sanity/src/core/components/commandList/CommandList.tsx
@@ -9,6 +9,7 @@ import {
   forwardRef,
   Fragment,
   isValidElement,
+  memo,
   type ReactElement,
   useCallback,
   useEffect,
@@ -91,17 +92,7 @@ const VirtualListChildBox = styled(Box) //
   width: 100%;
 `
 
-/**
- * Renders a Command List with support for the following:
- *
- * - Keyboard navigation (↑ / ↓ / ENTER) to children with a specified container (`childContainerRef`)
- * - Focus redirection when clicking child elements
- * - Pointer blocking when navigating with arrow keys (to ensure that only one active state is visible at any given time)
- * - ARIA attributes to define a `combobox` input that controls a separate `listbox`
- *
- * @internal
- */
-export const CommandList = forwardRef<CommandListHandle, CommandListProps>(function CommandList(
+const CommandListComponent = forwardRef<CommandListHandle, CommandListProps>(function CommandList(
   {
     activeItemDataAttr = LIST_ITEM_DATA_ATTR_ACTIVE,
     ariaLabel,
@@ -129,7 +120,7 @@ export const CommandList = forwardRef<CommandListHandle, CommandListProps>(funct
   ref,
 ) {
   const isMountedRef = useRef(false)
-  const commandListId = useRef(useId())
+  const [commandListId] = useState(useId())
   const activeIndexRef = useRef(initialIndex ?? 0)
 
   const [childContainerElement, setChildContainerElement] = useState<HTMLElement | null>(null)
@@ -217,11 +208,11 @@ export const CommandList = forwardRef<CommandListHandle, CommandListProps>(funct
   )
 
   const getChildDescendantId = useCallback(
-    (index: number) => `${commandListId.current}-item-${index}`,
-    [],
+    (index: number) => `${commandListId}-item-${index}`,
+    [commandListId],
   )
 
-  const getCommandListChildrenId = useCallback(() => `${commandListId.current}-children`, [])
+  const getCommandListChildrenId = useCallback(() => `${commandListId}-children`, [commandListId])
 
   /**
    * Iterate through all virtual list children and apply the active data-attribute on the selected index.
@@ -617,52 +608,128 @@ export const CommandList = forwardRef<CommandListHandle, CommandListProps>(funct
               virtualIndex,
             }) as ReactElement
 
-            const clonedItem = cloneElement(
-              itemToRender,
-              isValidElement(itemToRender) && itemToRender.type == Fragment
-                ? {}
-                : {
+            // @TODO can we avoid using cloneElement here?
+            const clonedItem =
+              isValidElement(itemToRender) && itemToRender.type != Fragment
+                ? cloneElement(itemToRender, {
+                    // @ts-expect-error @TODO shift the responsibility of setting tabIndex to the consumer, so we can remove the need to clone
                     tabIndex: -1,
-                  },
-            )
-
-            const activeAriaAttributes =
-              typeof activeIndex === 'number' && !disabled
-                ? {
-                    'aria-posinset': activeIndex + 1,
-                    ...(ariaMultiselectable ? {'aria-selected': selected.toString()} : {}),
-                    'aria-setsize': activeItemCount,
-                    'id': getChildDescendantId(activeIndex),
-                    'role': 'option',
-                    'onMouseEnter': handleChildMouseEnter(activeIndex),
-                  }
-                : {}
+                  })
+                : itemToRender
 
             return (
-              <Stack
-                as="li"
-                data-index={virtualIndex}
+              <CommandListItem
                 key={virtualRow.key}
                 ref={fixedHeight ? undefined : virtualizer.measureElement}
-                style={{
-                  flex: 1,
-                  ...(fixedHeight ? {height: `${virtualRow.size}px`} : {}),
-                  left: 0,
-                  position: 'absolute',
-                  top: 0,
-                  transform: `translateY(${virtualRow.start}px)`,
-                  width: '100%',
-                }}
-                tabIndex={-1}
-                {...activeAriaAttributes}
+                activeIndex={activeIndex}
+                activeItemCount={activeItemCount}
+                ariaMultiselectable={ariaMultiselectable}
+                disabled={disabled}
+                fixedHeight={fixedHeight ? `${virtualRow.size}px` : undefined}
+                getChildDescendantId={getChildDescendantId}
+                handleChildMouseEnter={handleChildMouseEnter}
+                selected={selected}
+                virtualIndex={virtualIndex}
+                virtualRowStart={virtualRow.start}
               >
                 {clonedItem}
-              </Stack>
+              </CommandListItem>
             )
           })}
         </VirtualListChildBox>
       )}
     </VirtualListBox>
   )
 })
-CommandList.displayName = 'ForwardRef(CommandList)'
+CommandListComponent.displayName = 'ForwardRef(CommandList)'
+/**
+ * Renders a Command List with support for the following:
+ *
+ * - Keyboard navigation (↑ / ↓ / ENTER) to children with a specified container (`childContainerRef`)
+ * - Focus redirection when clicking child elements
+ * - Pointer blocking when navigating with arrow keys (to ensure that only one active state is visible at any given time)
+ * - ARIA attributes to define a `combobox` input that controls a separate `listbox`
+ *
+ * @internal
+ */
+export const CommandList = memo(CommandListComponent)
+
+const CommandListItemComponent = forwardRef(function CommandListItem(
+  props: {
+    children: React.ReactNode
+    activeIndex: number | null
+    activeItemCount: number
+    ariaMultiselectable: boolean
+    disabled: boolean
+    fixedHeight: `${number}px` | undefined
+    getChildDescendantId: (index: number) => string
+    handleChildMouseEnter: (index: number) => () => void
+    selected: boolean
+    virtualIndex: number
+    virtualRowStart: number
+  },
+  forwardedRef: React.ForwardedRef<HTMLDivElement>,
+) {
+  const {
+    children,
+    activeIndex,
+    activeItemCount,
+    ariaMultiselectable,
+    disabled,
+    fixedHeight,
+    getChildDescendantId,
+    handleChildMouseEnter,
+    selected,
+    virtualIndex,
+    virtualRowStart,
+  } = props
+
+  const onMouseEnter = useMemo(
+    () =>
+      typeof activeIndex === 'number' && !disabled ? handleChildMouseEnter(activeIndex) : undefined,
+    [activeIndex, disabled, handleChildMouseEnter],
+  )
+  const activeAriaAttributes = useMemo(
+    () =>
+      typeof activeIndex === 'number' && !disabled
+        ? {
+            'aria-posinset': activeIndex + 1,
+            ...(ariaMultiselectable ? {'aria-selected': selected.toString()} : {}),
+            'aria-setsize': activeItemCount,
+            'id': getChildDescendantId(activeIndex),
+            'role': 'option',
+          }
+        : {},
+    [activeIndex, activeItemCount, ariaMultiselectable, disabled, getChildDescendantId, selected],
+  )
+
+  const style = useMemo(
+    () => ({
+      flex: 1,
+      height: fixedHeight,
+      left: 0,
+      position: 'absolute' as const,
+      top: 0,
+      transform: `translateY(${virtualRowStart}px)`,
+      width: '100%',
+    }),
+    [fixedHeight, virtualRowStart],
+  )
+
+  return (
+    <Stack
+      as="li"
+      data-index={virtualIndex}
+      ref={forwardedRef}
+      style={style}
+      tabIndex={-1}
+      {...activeAriaAttributes}
+      onMouseEnter={onMouseEnter}
+    >
+      {children}
+    </Stack>
+  )
+})
+CommandListItemComponent.displayName = 'ForwardRef(CommandListItem)'
+
+const CommandListItem = memo(CommandListItemComponent)