Fix useSelectionList bug. (#9171)

packages/cli/src/ui/hooks/useSelectionList.test.ts

@@ -744,6 +744,109 @@ describe('useSelectionList', () => {
       rerender({ items: [] });
       expect(result.current.activeIndex).toBe(0);
     });
+
+    it('should not reset activeIndex when items are deeply equal', () => {
+      const initialItems = [
+        { value: 'A' },
+        { value: 'B', disabled: true },
+        { value: 'C' },
+        { value: 'D' },
+      ];
+
+      const { result, rerender } = renderHook(
+        ({ items: testItems }: { items: Array<SelectionListItem<string>> }) =>
+          useSelectionList({
+            onSelect: mockOnSelect,
+            onHighlight: mockOnHighlight,
+            initialIndex: 2,
+            items: testItems,
+          }),
+        { initialProps: { items: initialItems } },
+      );
+
+      expect(result.current.activeIndex).toBe(2);
+
+      act(() => {
+        result.current.setActiveIndex(3);
+      });
+      expect(result.current.activeIndex).toBe(3);
+
+      mockOnHighlight.mockClear();
+
+      // Create new array with same content (deeply equal but not identical)
+      const newItems = [
+        { value: 'A' },
+        { value: 'B', disabled: true },
+        { value: 'C' },
+        { value: 'D' },
+      ];
+
+      rerender({ items: newItems });
+
+      // Active index should remain the same since items are deeply equal
+      expect(result.current.activeIndex).toBe(3);
+      // onHighlight should NOT be called since the index didn't change
+      expect(mockOnHighlight).not.toHaveBeenCalled();
+    });
+
+    it('should update activeIndex when items change structurally', () => {
+      const initialItems = [
+        { value: 'A' },
+        { value: 'B', disabled: true },
+        { value: 'C' },
+        { value: 'D' },
+      ];
+
+      const { result, rerender } = renderHook(
+        ({ items: testItems }: { items: Array<SelectionListItem<string>> }) =>
+          useSelectionList({
+            onSelect: mockOnSelect,
+            onHighlight: mockOnHighlight,
+            initialIndex: 3,
+            items: testItems,
+          }),
+        { initialProps: { items: initialItems } },
+      );
+
+      expect(result.current.activeIndex).toBe(3);
+      mockOnHighlight.mockClear();
+
+      // Change item values (not deeply equal)
+      const newItems = [{ value: 'X' }, { value: 'Y' }, { value: 'Z' }];
+
+      rerender({ items: newItems });
+
+      // Active index should update based on initialIndex and new items
+      expect(result.current.activeIndex).toBe(0);
+    });
+
+    it('should handle partial changes in items array', () => {
+      const initialItems = [{ value: 'A' }, { value: 'B' }, { value: 'C' }];
+
+      const { result, rerender } = renderHook(
+        ({ items: testItems }: { items: Array<SelectionListItem<string>> }) =>
+          useSelectionList({
+            onSelect: mockOnSelect,
+            initialIndex: 1,
+            items: testItems,
+          }),
+        { initialProps: { items: initialItems } },
+      );
+
+      expect(result.current.activeIndex).toBe(1);
+
+      // Change only one item's disabled status
+      const newItems = [
+        { value: 'A' },
+        { value: 'B', disabled: true },
+        { value: 'C' },
+      ];
+
+      rerender({ items: newItems });
+
+      // Should find next valid index since current became disabled
+      expect(result.current.activeIndex).toBe(2);
+    });
   });
 
   describe('Manual Control', () => {

packages/cli/src/ui/hooks/useSelectionList.ts

@@ -26,10 +26,12 @@ export interface UseSelectionListResult {
   setActiveIndex: (index: number) => void;
 }
 
-interface SelectionListState {
+interface SelectionListState<T> {
   activeIndex: number;
+  initialIndex: number;
   pendingHighlight: boolean;
   pendingSelect: boolean;
+  items: Array<SelectionListItem<T>>;
 }
 
 type SelectionListAction<T> =
@@ -68,6 +70,27 @@ type SelectionListAction<T> =
 
 const NUMBER_INPUT_TIMEOUT_MS = 1000;
 
+/**
+ * Performs an equality check on two arrays of SelectionListItem<T>.
+ *
+ * It compares the length of the arrays and then the 'value' and 'disabled'
+ * properties of each item.
+ */
+const areItemsEqual = <T>(
+  a: Array<SelectionListItem<T>>,
+  b: Array<SelectionListItem<T>>,
+): boolean => {
+  if (a.length !== b.length) {
+    return false;
+  }
+  for (let i = 0; i < a.length; i++) {
+    if (a[i]!.value !== b[i]!.value || a[i]!.disabled !== b[i]!.disabled) {
+      return false;
+    }
+  }
+  return true;
+};
+
 /**
  * Helper function to find the next enabled index in a given direction, supporting wrapping.
  */
@@ -96,10 +119,32 @@ const findNextValidIndex = <T>(
   return currentIndex;
 };
 
+const computeInitialIndex = <T>(
+  initialIndex: number,
+  items: Array<SelectionListItem<T>>,
+): number => {
+  if (items.length === 0) {
+    return 0;
+  }
+
+  let targetIndex = initialIndex;
+
+  if (targetIndex < 0 || targetIndex >= items.length) {
+    targetIndex = 0;
+  }
+
+  if (items[targetIndex]?.disabled) {
+    const nextValid = findNextValidIndex(targetIndex, 'down', items);
+    targetIndex = nextValid;
+  }
+
+  return targetIndex;
+};
+
 function selectionListReducer<T>(
-  state: SelectionListState,
+  state: SelectionListState<T>,
   action: SelectionListAction<T>,
-): SelectionListState {
+): SelectionListState<T> {
   switch (action.type) {
     case 'SET_ACTIVE_INDEX': {
       const { index, items } = action.payload;
@@ -139,30 +184,20 @@ function selectionListReducer<T>(
 
     case 'INITIALIZE': {
       const { initialIndex, items } = action.payload;
-
-      if (items.length === 0) {
-        const newIndex = 0;
-        return newIndex === state.activeIndex
-          ? state
-          : { ...state, activeIndex: newIndex };
+      if (
+        state.initialIndex === initialIndex &&
+        areItemsEqual(state.items, items)
+      ) {
+        return state;
       }
+      const targetIndex = computeInitialIndex(initialIndex, items);
 
-      let targetIndex = initialIndex;
-
-      if (targetIndex < 0 || targetIndex >= items.length) {
-        targetIndex = 0;
-      }
-
-      if (items[targetIndex]?.disabled) {
-        const nextValid = findNextValidIndex(targetIndex, 'down', items);
-        targetIndex = nextValid;
-      }
-
-      // Only return new state if activeIndex actually changed
-      // Don't set pendingHighlight on initialization
-      return targetIndex === state.activeIndex
-        ? state
-        : { ...state, activeIndex: targetIndex, pendingHighlight: false };
+      return {
+        ...state,
+        items,
+        activeIndex: targetIndex,
+        pendingHighlight: false,
+      };
     }
 
     case 'CLEAR_PENDING_FLAGS': {
@@ -201,9 +236,11 @@ export function useSelectionList<T>({
   showNumbers = false,
 }: UseSelectionListOptions<T>): UseSelectionListResult {
   const [state, dispatch] = useReducer(selectionListReducer<T>, {
-    activeIndex: initialIndex,
+    activeIndex: computeInitialIndex(initialIndex, items),
+    initialIndex,
     pendingHighlight: false,
     pendingSelect: false,
+    items,
   });
   const numberInputRef = useRef('');
   const numberInputTimer = useRef<NodeJS.Timeout | null>(null);