Skip to content
Merged
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
5 changes: 5 additions & 0 deletions .changeset/virtuallist-stable-ref.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
useContext,
useEffect,
useImperativeHandle,
useInsertionEffect,
useLayoutEffect,
useRef,
useState,
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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(),
};
}
Loading
Loading