fix(virtual-list): keep the ref handle stable across scroll renders

Author: RuudBurger

.changeset/virtuallist-stable-ref.md

@@ -0,0 +1,5 @@
+---
+'@plextv/react-lightning-components': patch
+---
+
+virtual list keeps one ref handle for its lifetime, so cells reading it from a layout effect during a scroll no longer get null.

packages/react-lightning-components/src/components/VirtualList/VirtualList.spec.tsx

@@ -0,0 +1,73 @@
+import { type ReactNode, useLayoutEffect } from 'react';
+import { describe, expect, it, vi } from 'vitest';
+
+vi.mock('@plextv/react-lightning', () => ({
+  FocusGroup: ({ children }: { children?: ReactNode }) => children,
+  useFocusManager: () => ({ setFocusedChild: () => {} }),
+}));
+
+vi.mock('@plextv/react-lightning-plugin-flexbox', () => ({
+  FlexBoundary: ({ children }: { children?: ReactNode }) => children,
+  FlexRoot: ({ children }: { children?: ReactNode }) => children,
+  useIsInFlex: () => false,
+  onFlexLayoutSettled: () => () => {},
+}));
+
+import { createTestRenderer } from './createTestRenderer';
+import { VirtualList } from './VirtualList';
+import type { ItemLayout, VirtualListRef } from './VirtualListTypes';
+
+type Read = {
+  index: number;
+  hasHandle: boolean;
+  range?: { startIndex: number; endIndex: number };
+  layout?: ItemLayout;
+};
+
+describe('VirtualList', () => {
+  it('keeps the ref attached while cells run layout effects after a scroll', () => {
+    const { render, flush } = createTestRenderer();
+    const listRef: { current: VirtualListRef | null } = { current: null };
+    const reads: Read[] = [];
+
+    function Cell({ index }: { index: number }) {
+      useLayoutEffect(() => {
+        const handle = listRef.current;
+
+        reads.push({
+          index,
+          hasHandle: handle != null,
+          range: handle?.getVisibleRange(),
+          layout: handle?.getLayout(index),
+        });
+      }, [index]);
+
+      return null;
+    }
+
+    render(
+      <VirtualList
+        ref={listRef}
+        data={Array.from({ length: 50 }, (_, i) => i)}
+        drawDistance={0}
+        overrideItemLayout={(layout) => {
+          layout.size = 100;
+        }}
+        renderItem={({ index }) => <Cell index={index} />}
+        style={{ w: 500, h: 300 }}
+      />,
+    );
+
+    reads.length = 0;
+    listRef.current?.scrollToOffset({ offset: 2000, animated: false });
+    flush();
+
+    expect(reads.length).toBeGreaterThan(0);
+
+    for (const read of reads) {
+      expect(read.hasHandle).toBe(true);
+      expect(read.range?.startIndex).toBe(20);
+      expect(read.layout?.y).toBe(read.index * 100);
+    }
+  });
+});

packages/react-lightning-components/src/components/VirtualList/VirtualList.tsx

@@ -7,6 +7,7 @@ import {
   useContext,
   useEffect,
   useImperativeHandle,
+  useInsertionEffect,
   useLayoutEffect,
   useRef,
   useState,
@@ -705,26 +706,64 @@ function VirtualListInner<T>(
     }
   };
 
-  useImperativeHandle(ref, () => ({
-    scrollToIndex: (params) =>
-      scrollToIndex(
-        params.index,
-        params.animated,
-        params.viewPosition,
-        params.viewOffset,
-      ),
-    scrollToOffset: (params) => scrollToOffset(params.offset, params.animated),
-    scrollToEnd: (params) => scrollToEnd(params?.animated),
-    getScrollOffset: () => scrollOffsetRef.current,
-    getVisibleRange: () => visibleRange,
-    getLayout: (index) => {
-      const layout = layoutManager.getLayout(index);
+  const handleStateRef = useRef({
+    scrollToIndex,
+    scrollToOffset,
+    scrollToEnd,
+    visibleRange,
+    itemAreaOffset,
+    paddingCross,
+    horizontal,
+  });
 
-      return layout
-        ? computeItemRect(layout, itemAreaOffset, paddingCross, horizontal)
-        : undefined;
-    },
-  }));
+  // Insertion effects run before any layout effect in the commit, so cells
+  // calling the handle from their own layout effects already see this render.
+  useInsertionEffect(() => {
+    handleStateRef.current = {
+      scrollToIndex,
+      scrollToOffset,
+      scrollToEnd,
+      visibleRange,
+      itemAreaOffset,
+      paddingCross,
+      horizontal,
+    };
+  });
+
+  // One handle for the list's lifetime: a rebuilt handle detaches the ref
+  // for the whole commit, so cells' layout effects would read null.
+  useImperativeHandle(
+    ref,
+    () => ({
+      scrollToIndex: (params) =>
+        handleStateRef.current.scrollToIndex(
+          params.index,
+          params.animated,
+          params.viewPosition,
+          params.viewOffset,
+        ),
+      scrollToOffset: (params) =>
+        handleStateRef.current.scrollToOffset(params.offset, params.animated),
+      scrollToEnd: (params) =>
+        handleStateRef.current.scrollToEnd(params?.animated),
+      getScrollOffset: () => scrollOffsetRef.current,
+      getVisibleRange: () => handleStateRef.current.visibleRange,
+      getLayout: (index) => {
+        const layout = layoutManager.getLayout(index);
+        const state = handleStateRef.current;
+
+        return layout
+          ? computeItemRect(
+              layout,
+              state.itemAreaOffset,
+              state.paddingCross,
+              state.horizontal,
+            )
+          : undefined;
+      },
+    }),
+    [],
+  );
 
   const loadTimeRef = useRef(Date.now());
   const hasLoadedRef = useRef(false);

packages/react-lightning-components/src/components/VirtualList/createTestRenderer.ts

@@ -0,0 +1,116 @@
+import { type ReactNode, createContext } from 'react';
+import createReconciler, { type ReactContext } from 'react-reconciler';
+import { DiscreteEventPriority, NoEventPriority } from 'react-reconciler/constants';
+
+const noop = () => {};
+
+// Bare host config: nothing paints, only scheduling and effects matter. Every
+// update is sync so tests can flush in place.
+export type TestRenderer = {
+  render: (element: ReactNode) => void;
+  flush: () => void;
+};
+
+export function createTestRenderer(): TestRenderer {
+  const transitionContext = createContext(null);
+  let priority: number = NoEventPriority;
+
+  // oxlint-disable-next-line typescript/no-explicit-any -- test-only host config, nothing is a real instance
+  const reconciler = createReconciler<
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any,
+    any
+  >({
+    supportsMutation: true,
+    supportsPersistence: false,
+    supportsHydration: false,
+    isPrimaryRenderer: false,
+    warnsIfNotActing: false,
+    supportsMicrotasks: true,
+    scheduleMicrotask: queueMicrotask,
+    scheduleTimeout: setTimeout,
+    cancelTimeout: clearTimeout,
+    noTimeout: -1,
+    getRootHostContext: () => ({}),
+    getChildHostContext: (ctx: unknown) => ctx,
+    getPublicInstance: (instance: unknown) => instance,
+    createInstance: () => ({ node: { x: 0, y: 0, w: 0, h: 0 } }),
+    createTextInstance: () => ({}),
+    appendInitialChild: noop,
+    appendChild: noop,
+    appendChildToContainer: noop,
+    insertBefore: noop,
+    insertInContainerBefore: noop,
+    removeChild: noop,
+    removeChildFromContainer: noop,
+    clearContainer: noop,
+    commitUpdate: noop,
+    commitTextUpdate: noop,
+    hideInstance: noop,
+    unhideInstance: noop,
+    hideTextInstance: noop,
+    unhideTextInstance: noop,
+    detachDeletedInstance: noop,
+    finalizeInitialChildren: () => false,
+    prepareForCommit: () => null,
+    resetAfterCommit: noop,
+    shouldSetTextContent: () => false,
+    preparePortalMount: noop,
+    getInstanceFromNode: () => null,
+    beforeActiveInstanceBlur: noop,
+    afterActiveInstanceBlur: noop,
+    prepareScopeUpdate: noop,
+    getInstanceFromScope: () => null,
+    resetFormInstance: noop,
+    requestPostPaintCallback: noop,
+    shouldAttemptEagerTransition: () => false,
+    trackSchedulerEvent: noop,
+    resolveEventType: () => null,
+    resolveEventTimeStamp: () => -1.1,
+    maySuspendCommit: () => false,
+    startSuspendingCommit: noop,
+    suspendInstance: noop,
+    preloadInstance: () => true,
+    waitForCommitToBeReady: () => null,
+    setCurrentUpdatePriority: (next: number) => {
+      priority = next;
+    },
+    getCurrentUpdatePriority: () => priority,
+    resolveUpdatePriority: () => (priority !== NoEventPriority ? priority : DiscreteEventPriority),
+    NotPendingTransition: null,
+    // React's own context object carries the internal fields the typings ask for.
+    HostTransitionContext: transitionContext as unknown as ReactContext<null>,
+  });
+
+  const container = reconciler.createContainer(
+    {},
+    0,
+    null,
+    false,
+    null,
+    '',
+    noop,
+    noop,
+    noop,
+    noop,
+  );
+
+  return {
+    render: (element) => {
+      reconciler.updateContainerSync(element, container, null, null);
+      reconciler.flushSyncWork();
+    },
+    flush: () => reconciler.flushSyncWork(),
+  };
+}