From 14dd07be00bb4cdf48b9da91ec362297040dd03c Mon Sep 17 00:00:00 2001 From: Jacob Richman Date: Fri, 27 Feb 2026 08:00:07 -0800 Subject: [PATCH] fix(cli): ensure dialogs stay scrolled to bottom in alternate buffer mode (#20527) --- .../ui/components/shared/Scrollable.test.tsx | 39 ++- .../src/ui/components/shared/Scrollable.tsx | 149 ++++++---- .../components/shared/ScrollableList.test.tsx | 273 ++++++++++++++++++ .../ui/components/shared/ScrollableList.tsx | 32 +- .../shared/VirtualizedList.test.tsx | 9 +- .../ui/components/shared/VirtualizedList.tsx | 261 ++++++++++------- 6 files changed, 571 insertions(+), 192 deletions(-) diff --git a/packages/cli/src/ui/components/shared/Scrollable.test.tsx b/packages/cli/src/ui/components/shared/Scrollable.test.tsx index db32a1a2e9..7772cdf22c 100644 --- a/packages/cli/src/ui/components/shared/Scrollable.test.tsx +++ b/packages/cli/src/ui/components/shared/Scrollable.test.tsx @@ -6,20 +6,11 @@ import { renderWithProviders } from '../../../test-utils/render.js'; import { Scrollable } from './Scrollable.js'; -import { Text } from 'ink'; +import { Text, Box } from 'ink'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import * as ScrollProviderModule from '../../contexts/ScrollProvider.js'; import { act } from 'react'; - -vi.mock('ink', async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - getInnerHeight: vi.fn(() => 5), - getScrollHeight: vi.fn(() => 10), - getBoundingBox: vi.fn(() => ({ x: 0, y: 0, width: 10, height: 5 })), - }; -}); +import { waitFor } from '../../../test-utils/async.js'; vi.mock('../../hooks/useAnimatedScrollbar.js', () => ({ useAnimatedScrollbar: ( @@ -129,20 +120,26 @@ describe('', () => { , ); await waitUntilReady2(); - expect(capturedEntry.getScrollState().scrollTop).toBe(5); + await waitFor(() => { + expect(capturedEntry?.getScrollState().scrollTop).toBe(5); + }); // Call scrollBy multiple times (upwards) in the same tick await act(async () => { - capturedEntry!.scrollBy(-1); - capturedEntry!.scrollBy(-1); + capturedEntry?.scrollBy(-1); + capturedEntry?.scrollBy(-1); }); // Should have moved up by 2 (5 -> 3) - expect(capturedEntry.getScrollState().scrollTop).toBe(3); + await waitFor(() => { + expect(capturedEntry?.getScrollState().scrollTop).toBe(3); + }); await act(async () => { - capturedEntry!.scrollBy(-2); + capturedEntry?.scrollBy(-2); + }); + await waitFor(() => { + expect(capturedEntry?.getScrollState().scrollTop).toBe(1); }); - expect(capturedEntry.getScrollState().scrollTop).toBe(1); unmount2(); }); @@ -191,10 +188,6 @@ describe('', () => { keySequence, expectedScrollTop, }) => { - // Dynamically import ink to mock getScrollHeight - const ink = await import('ink'); - vi.mocked(ink.getScrollHeight).mockReturnValue(scrollHeight); - let capturedEntry: ScrollProviderModule.ScrollableEntry | undefined; vi.spyOn(ScrollProviderModule, 'useScrollable').mockImplementation( async (entry, isActive) => { @@ -206,7 +199,9 @@ describe('', () => { const { stdin, waitUntilReady, unmount } = renderWithProviders( - Content + + Content + , ); await waitUntilReady(); diff --git a/packages/cli/src/ui/components/shared/Scrollable.tsx b/packages/cli/src/ui/components/shared/Scrollable.tsx index a830cbecfe..87ec6e72d6 100644 --- a/packages/cli/src/ui/components/shared/Scrollable.tsx +++ b/packages/cli/src/ui/components/shared/Scrollable.tsx @@ -4,15 +4,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { - useState, - useEffect, - useRef, - useLayoutEffect, - useCallback, - useMemo, -} from 'react'; -import { Box, getInnerHeight, getScrollHeight, type DOMElement } from 'ink'; +import type React from 'react'; +import { useState, useRef, useCallback, useMemo, useLayoutEffect } from 'react'; +import { Box, ResizeObserver, type DOMElement } from 'ink'; import { useKeypress, type Key } from '../../hooks/useKeypress.js'; import { useScrollable } from '../../contexts/ScrollProvider.js'; import { useAnimatedScrollbar } from '../../hooks/useAnimatedScrollbar.js'; @@ -41,62 +35,101 @@ export const Scrollable: React.FC = ({ flexGrow, }) => { const [scrollTop, setScrollTop] = useState(0); - const ref = useRef(null); + const viewportRef = useRef(null); + const contentRef = useRef(null); const [size, setSize] = useState({ - innerHeight: 0, + innerHeight: typeof height === 'number' ? height : 0, scrollHeight: 0, }); const sizeRef = useRef(size); - useEffect(() => { + const scrollTopRef = useRef(scrollTop); + + useLayoutEffect(() => { sizeRef.current = size; }, [size]); - const childrenCountRef = useRef(0); - - // This effect needs to run on every render to correctly measure the container - // and scroll to the bottom if new children are added. - // eslint-disable-next-line react-hooks/exhaustive-deps useLayoutEffect(() => { - if (!ref.current) { - return; + scrollTopRef.current = scrollTop; + }, [scrollTop]); + + const viewportObserverRef = useRef(null); + const contentObserverRef = useRef(null); + + const viewportRefCallback = useCallback((node: DOMElement | null) => { + viewportObserverRef.current?.disconnect(); + viewportRef.current = node; + + if (node) { + const observer = new ResizeObserver((entries) => { + const entry = entries[0]; + if (entry) { + const innerHeight = Math.round(entry.contentRect.height); + setSize((prev) => { + const scrollHeight = prev.scrollHeight; + const isAtBottom = + scrollHeight > prev.innerHeight && + scrollTopRef.current >= scrollHeight - prev.innerHeight - 1; + + if (isAtBottom) { + setScrollTop(Number.MAX_SAFE_INTEGER); + } + return { ...prev, innerHeight }; + }); + } + }); + observer.observe(node); + viewportObserverRef.current = observer; } - const innerHeight = Math.round(getInnerHeight(ref.current)); - const scrollHeight = Math.round(getScrollHeight(ref.current)); + }, []); - const isAtBottom = - scrollHeight > innerHeight && scrollTop >= scrollHeight - innerHeight - 1; + const contentRefCallback = useCallback( + (node: DOMElement | null) => { + contentObserverRef.current?.disconnect(); + contentRef.current = node; - if ( - size.innerHeight !== innerHeight || - size.scrollHeight !== scrollHeight - ) { - setSize({ innerHeight, scrollHeight }); - if (isAtBottom) { - setScrollTop(Math.max(0, scrollHeight - innerHeight)); + if (node) { + const observer = new ResizeObserver((entries) => { + const entry = entries[0]; + if (entry) { + const scrollHeight = Math.round(entry.contentRect.height); + setSize((prev) => { + const innerHeight = prev.innerHeight; + const isAtBottom = + prev.scrollHeight > innerHeight && + scrollTopRef.current >= prev.scrollHeight - innerHeight - 1; + + if ( + isAtBottom || + (scrollToBottom && scrollHeight > prev.scrollHeight) + ) { + setScrollTop(Number.MAX_SAFE_INTEGER); + } + return { ...prev, scrollHeight }; + }); + } + }); + observer.observe(node); + contentObserverRef.current = observer; } - } - - const childCountCurrent = React.Children.count(children); - if (scrollToBottom && childrenCountRef.current !== childCountCurrent) { - setScrollTop(Math.max(0, scrollHeight - innerHeight)); - } - childrenCountRef.current = childCountCurrent; - }); + }, + [scrollToBottom], + ); const { getScrollTop, setPendingScrollTop } = useBatchedScroll(scrollTop); const scrollBy = useCallback( (delta: number) => { const { scrollHeight, innerHeight } = sizeRef.current; - const current = getScrollTop(); - const next = Math.min( - Math.max(0, current + delta), - Math.max(0, scrollHeight - innerHeight), - ); + const maxScroll = Math.max(0, scrollHeight - innerHeight); + const current = Math.min(getScrollTop(), maxScroll); + let next = Math.max(0, current + delta); + if (next >= maxScroll) { + next = Number.MAX_SAFE_INTEGER; + } setPendingScrollTop(next); setScrollTop(next); }, - [sizeRef, getScrollTop, setPendingScrollTop], + [getScrollTop, setPendingScrollTop], ); const { scrollbarColor, flashScrollbar, scrollByWithAnimation } = @@ -107,10 +140,11 @@ export const Scrollable: React.FC = ({ const { scrollHeight, innerHeight } = sizeRef.current; const scrollTop = getScrollTop(); const maxScroll = Math.max(0, scrollHeight - innerHeight); + const actualScrollTop = Math.min(scrollTop, maxScroll); // Only capture scroll-up events if there's room; // otherwise allow events to bubble. - if (scrollTop > 0) { + if (actualScrollTop > 0) { if (keyMatchers[Command.PAGE_UP](key)) { scrollByWithAnimation(-innerHeight); return true; @@ -123,7 +157,7 @@ export const Scrollable: React.FC = ({ // Only capture scroll-down events if there's room; // otherwise allow events to bubble. - if (scrollTop < maxScroll) { + if (actualScrollTop < maxScroll) { if (keyMatchers[Command.PAGE_DOWN](key)) { scrollByWithAnimation(innerHeight); return true; @@ -140,21 +174,21 @@ export const Scrollable: React.FC = ({ { isActive: hasFocus }, ); - const getScrollState = useCallback( - () => ({ - scrollTop: getScrollTop(), + const getScrollState = useCallback(() => { + const maxScroll = Math.max(0, size.scrollHeight - size.innerHeight); + return { + scrollTop: Math.min(getScrollTop(), maxScroll), scrollHeight: size.scrollHeight, innerHeight: size.innerHeight, - }), - [getScrollTop, size.scrollHeight, size.innerHeight], - ); + }; + }, [getScrollTop, size.scrollHeight, size.innerHeight]); const hasFocusCallback = useCallback(() => hasFocus, [hasFocus]); const scrollableEntry = useMemo( () => ({ // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - ref: ref as React.RefObject, + ref: viewportRef as React.RefObject, getScrollState, scrollBy: scrollByWithAnimation, hasFocus: hasFocusCallback, @@ -167,7 +201,7 @@ export const Scrollable: React.FC = ({ return ( = ({ based on the children's content. It also adds a right padding to make room for the scrollbar. */} - + {children} diff --git a/packages/cli/src/ui/components/shared/ScrollableList.test.tsx b/packages/cli/src/ui/components/shared/ScrollableList.test.tsx index 8b8c4e3fdf..1dd72b89a2 100644 --- a/packages/cli/src/ui/components/shared/ScrollableList.test.tsx +++ b/packages/cli/src/ui/components/shared/ScrollableList.test.tsx @@ -479,4 +479,277 @@ describe('ScrollableList Demo Behavior', () => { }); }); }); + + it('regression: remove last item and add 2 items when scrolled to bottom', async () => { + let listRef: ScrollableListRef | null = null; + let setItemsFunc: React.Dispatch> | null = + null; + + const TestComp = () => { + const [items, setItems] = useState( + Array.from({ length: 10 }, (_, i) => ({ + id: String(i), + title: `Item ${i}`, + })), + ); + useEffect(() => { + setItemsFunc = setItems; + }, []); + + return ( + + + + + { + listRef = ref; + }} + data={items} + renderItem={({ item }) => {item.title}} + estimatedItemHeight={() => 1} + keyExtractor={(item) => item.id} + hasFocus={true} + initialScrollIndex={Number.MAX_SAFE_INTEGER} + /> + + + + + ); + }; + + let result: ReturnType; + await act(async () => { + result = render(); + }); + + await result!.waitUntilReady(); + + // Scrolled to bottom, max scroll = 10 - 5 = 5 + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(5); + }); + + // Remove last element and add 2 elements + await act(async () => { + setItemsFunc!((prev) => { + const next = prev.slice(0, prev.length - 1); + next.push({ id: '10', title: 'Item 10' }); + next.push({ id: '11', title: 'Item 11' }); + return next; + }); + }); + + await result!.waitUntilReady(); + + // Auto scrolls to new bottom: max scroll = 11 - 5 = 6 + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(6); + }); + + // Scroll up slightly + await act(async () => { + listRef?.scrollBy(-2); + }); + await result!.waitUntilReady(); + + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(4); + }); + + // Scroll back to bottom + await act(async () => { + listRef?.scrollToEnd(); + }); + await result!.waitUntilReady(); + + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(6); + }); + + // Add two more elements + await act(async () => { + setItemsFunc!((prev) => [ + ...prev, + { id: '12', title: 'Item 12' }, + { id: '13', title: 'Item 13' }, + ]); + }); + + await result!.waitUntilReady(); + + // Auto scrolls to bottom: max scroll = 13 - 5 = 8 + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(8); + }); + + result!.unmount(); + }); + + it('regression: bottom-most element changes size but list does not update', async () => { + let listRef: ScrollableListRef | null = null; + let expandLastFunc: (() => void) | null = null; + + const ItemWithState = ({ + item, + isLast, + }: { + item: Item; + isLast: boolean; + }) => { + const [expanded, setExpanded] = useState(false); + useEffect(() => { + if (isLast) { + expandLastFunc = () => setExpanded(true); + } + }, [isLast]); + return ( + + {item.title} + {expanded && Expanded content} + + ); + }; + + const TestComp = () => { + // items array is stable + const [items] = useState(() => + Array.from({ length: 5 }, (_, i) => ({ + id: String(i), + title: `Item ${i}`, + })), + ); + + return ( + + + + + { + listRef = ref; + }} + data={items} + renderItem={({ item, index }) => ( + + )} + estimatedItemHeight={() => 1} + keyExtractor={(item) => item.id} + hasFocus={true} + initialScrollIndex={Number.MAX_SAFE_INTEGER} + /> + + + + + ); + }; + + let result: ReturnType; + await act(async () => { + result = render(); + }); + + await result!.waitUntilReady(); + + // Initially, total height is 5. viewport is 4. scroll is 1. + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(1); + }); + + // Expand the last item locally, without re-rendering the list! + await act(async () => { + expandLastFunc!(); + }); + + await result!.waitUntilReady(); + + // The total height becomes 6. It should remain scrolled to bottom, so scroll becomes 2. + // This is expected to FAIL currently because VirtualizedList won't remeasure + // unless data changes or container height changes. + await waitFor( + () => { + expect(listRef?.getScrollState()?.scrollTop).toBe(2); + }, + { timeout: 1000 }, + ); + + result!.unmount(); + }); + + it('regression: prepending items does not corrupt heights (total height correct)', async () => { + let listRef: ScrollableListRef | null = null; + let setItemsFunc: React.Dispatch> | null = + null; + + const TestComp = () => { + // Items 1 to 5. Item 1 is very tall. + const [items, setItems] = useState( + Array.from({ length: 5 }, (_, i) => ({ + id: String(i + 1), + title: `Item ${i + 1}`, + })), + ); + useEffect(() => { + setItemsFunc = setItems; + }, []); + + return ( + + + + + { + listRef = ref; + }} + data={items} + renderItem={({ item }) => ( + + {item.title} + + )} + estimatedItemHeight={() => 2} + keyExtractor={(item) => item.id} + hasFocus={true} + initialScrollIndex={Number.MAX_SAFE_INTEGER} + /> + + + + + ); + }; + + let result: ReturnType; + await act(async () => { + result = render(); + }); + + await result!.waitUntilReady(); + + // Scroll is at bottom. + // Heights: Item 1: 10, Item 2: 2, Item 3: 2, Item 4: 2, Item 5: 2. + // Total height = 18. Container = 10. Max scroll = 8. + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(8); + }); + + // Prepend an item! + await act(async () => { + setItemsFunc!((prev) => [{ id: '0', title: 'Item 0' }, ...prev]); + }); + + await result!.waitUntilReady(); + + // Now items: 0(2), 1(10), 2(2), 3(2), 4(2), 5(2). + // Total height = 20. Container = 10. Max scroll = 10. + // Auto-scrolls to bottom because it was sticking! + await waitFor(() => { + expect(listRef?.getScrollState()?.scrollTop).toBe(10); + }); + + result!.unmount(); + }); }); diff --git a/packages/cli/src/ui/components/shared/ScrollableList.tsx b/packages/cli/src/ui/components/shared/ScrollableList.tsx index e51acd6446..b7085329a3 100644 --- a/packages/cli/src/ui/components/shared/ScrollableList.tsx +++ b/packages/cli/src/ui/components/shared/ScrollableList.tsx @@ -10,7 +10,7 @@ import { useImperativeHandle, useCallback, useMemo, - useEffect, + useLayoutEffect, } from 'react'; import type React from 'react'; import { @@ -105,7 +105,7 @@ function ScrollableList( smoothScrollState.current.active = false; }, []); - useEffect(() => stopSmoothScroll, [stopSmoothScroll]); + useLayoutEffect(() => stopSmoothScroll, [stopSmoothScroll]); const smoothScrollTo = useCallback( ( @@ -120,15 +120,19 @@ function ScrollableList( innerHeight: 0, }; const { - scrollTop: startScrollTop, + scrollTop: rawStartScrollTop, scrollHeight, innerHeight, } = scrollState; const maxScrollTop = Math.max(0, scrollHeight - innerHeight); + const startScrollTop = Math.min(rawStartScrollTop, maxScrollTop); let effectiveTarget = targetScrollTop; - if (targetScrollTop === SCROLL_TO_ITEM_END) { + if ( + targetScrollTop === SCROLL_TO_ITEM_END || + targetScrollTop >= maxScrollTop + ) { effectiveTarget = maxScrollTop; } @@ -138,8 +142,11 @@ function ScrollableList( ); if (duration === 0) { - if (targetScrollTop === SCROLL_TO_ITEM_END) { - virtualizedListRef.current?.scrollTo(SCROLL_TO_ITEM_END); + if ( + targetScrollTop === SCROLL_TO_ITEM_END || + targetScrollTop >= maxScrollTop + ) { + virtualizedListRef.current?.scrollTo(Number.MAX_SAFE_INTEGER); } else { virtualizedListRef.current?.scrollTo(Math.round(clampedTarget)); } @@ -168,8 +175,11 @@ function ScrollableList( ease; if (progress >= 1) { - if (targetScrollTop === SCROLL_TO_ITEM_END) { - virtualizedListRef.current?.scrollTo(SCROLL_TO_ITEM_END); + if ( + targetScrollTop === SCROLL_TO_ITEM_END || + targetScrollTop >= maxScrollTop + ) { + virtualizedListRef.current?.scrollTo(Number.MAX_SAFE_INTEGER); } else { virtualizedListRef.current?.scrollTo(Math.round(current)); } @@ -200,9 +210,13 @@ function ScrollableList( ) { const direction = keyMatchers[Command.PAGE_UP](key) ? -1 : 1; const scrollState = getScrollState(); + const maxScroll = Math.max( + 0, + scrollState.scrollHeight - scrollState.innerHeight, + ); const current = smoothScrollState.current.active ? smoothScrollState.current.to - : scrollState.scrollTop; + : Math.min(scrollState.scrollTop, maxScroll); const innerHeight = scrollState.innerHeight; smoothScrollTo(current + direction * innerHeight); return true; diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx index 0edc323d38..60b8bfc421 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.test.tsx @@ -5,6 +5,7 @@ */ import { render } from '../../../test-utils/render.js'; +import { waitFor } from '../../../test-utils/async.js'; import { VirtualizedList, type VirtualizedListRef } from './VirtualizedList.js'; import { Text, Box } from 'ink'; import { @@ -275,9 +276,11 @@ describe('', () => { await waitUntilReady(); // Now Item 0 is 1px, so Items 1-9 should also be visible to fill 10px - expect(lastFrame()).toContain('Item 0'); - expect(lastFrame()).toContain('Item 1'); - expect(lastFrame()).toContain('Item 9'); + await waitFor(() => { + expect(lastFrame()).toContain('Item 0'); + expect(lastFrame()).toContain('Item 1'); + expect(lastFrame()).toContain('Item 9'); + }); unmount(); }); diff --git a/packages/cli/src/ui/components/shared/VirtualizedList.tsx b/packages/cli/src/ui/components/shared/VirtualizedList.tsx index 98e45a695e..669b1bc035 100644 --- a/packages/cli/src/ui/components/shared/VirtualizedList.tsx +++ b/packages/cli/src/ui/components/shared/VirtualizedList.tsx @@ -10,7 +10,6 @@ import { useLayoutEffect, forwardRef, useImperativeHandle, - useEffect, useMemo, useCallback, } from 'react'; @@ -19,7 +18,7 @@ import { theme } from '../../semantic-colors.js'; import { useBatchedScroll } from '../../hooks/useBatchedScroll.js'; import { useUIState } from '../../contexts/UIStateContext.js'; -import { type DOMElement, measureElement, Box } from 'ink'; +import { type DOMElement, Box, ResizeObserver } from 'ink'; export const SCROLL_TO_ITEM_END = Number.MAX_SAFE_INTEGER; @@ -81,7 +80,7 @@ function VirtualizedList( } = props; const { copyModeEnabled } = useUIState(); const dataRef = useRef(data); - useEffect(() => { + useLayoutEffect(() => { dataRef.current = data; }, [data]); @@ -108,6 +107,7 @@ function VirtualizedList( return { index: 0, offset: 0 }; }); + const [isStickingToBottom, setIsStickingToBottom] = useState(() => { const scrollToEnd = initialScrollIndex === SCROLL_TO_ITEM_END || @@ -116,73 +116,75 @@ function VirtualizedList( initialScrollOffsetInIndex === SCROLL_TO_ITEM_END); return scrollToEnd; }); - const containerRef = useRef(null); + + const containerRef = useRef(null); const [containerHeight, setContainerHeight] = useState(0); const itemRefs = useRef>([]); - const [heights, setHeights] = useState([]); + const [heights, setHeights] = useState>({}); const isInitialScrollSet = useRef(false); + const containerObserverRef = useRef(null); + const nodeToKeyRef = useRef(new WeakMap()); + + const containerRefCallback = useCallback((node: DOMElement | null) => { + containerObserverRef.current?.disconnect(); + containerRef.current = node; + if (node) { + const observer = new ResizeObserver((entries) => { + const entry = entries[0]; + if (entry) { + setContainerHeight(Math.round(entry.contentRect.height)); + } + }); + observer.observe(node); + containerObserverRef.current = observer; + } + }, []); + + const itemsObserver = useMemo( + () => + new ResizeObserver((entries) => { + setHeights((prev) => { + let next: Record | null = null; + for (const entry of entries) { + const key = nodeToKeyRef.current.get(entry.target); + if (key !== undefined) { + const height = Math.round(entry.contentRect.height); + if (prev[key] !== height) { + if (!next) { + next = { ...prev }; + } + next[key] = height; + } + } + } + return next ?? prev; + }); + }), + [], + ); + + useLayoutEffect( + () => () => { + containerObserverRef.current?.disconnect(); + itemsObserver.disconnect(); + }, + [itemsObserver], + ); + const { totalHeight, offsets } = useMemo(() => { const offsets: number[] = [0]; let totalHeight = 0; for (let i = 0; i < data.length; i++) { - const height = heights[i] ?? estimatedItemHeight(i); + const key = keyExtractor(data[i], i); + const height = heights[key] ?? estimatedItemHeight(i); totalHeight += height; offsets.push(totalHeight); } return { totalHeight, offsets }; - }, [heights, data, estimatedItemHeight]); + }, [heights, data, estimatedItemHeight, keyExtractor]); - useEffect(() => { - setHeights((prevHeights) => { - if (data.length === prevHeights.length) { - return prevHeights; - } - - const newHeights = [...prevHeights]; - if (data.length < prevHeights.length) { - newHeights.length = data.length; - } else { - for (let i = prevHeights.length; i < data.length; i++) { - newHeights[i] = estimatedItemHeight(i); - } - } - return newHeights; - }); - }, [data, estimatedItemHeight]); - - // This layout effect needs to run on every render to correctly measure the - // container and ensure we recompute the layout if it has changed. - // eslint-disable-next-line react-hooks/exhaustive-deps - useLayoutEffect(() => { - if (containerRef.current) { - const height = Math.round(measureElement(containerRef.current).height); - if (containerHeight !== height) { - setContainerHeight(height); - } - } - - let newHeights: number[] | null = null; - for (let i = startIndex; i <= endIndex; i++) { - const itemRef = itemRefs.current[i]; - if (itemRef) { - const height = Math.round(measureElement(itemRef).height); - if (height !== heights[i]) { - if (!newHeights) { - newHeights = [...heights]; - } - newHeights[i] = height; - } - } - } - if (newHeights) { - setHeights(newHeights); - } - }); - - const scrollableContainerHeight = containerRef.current - ? Math.round(measureElement(containerRef.current).height) - : containerHeight; + const scrollableContainerHeight = containerHeight; const getAnchorForScrollTop = useCallback( ( @@ -199,23 +201,36 @@ function VirtualizedList( [], ); - const scrollTop = useMemo(() => { + const actualScrollTop = useMemo(() => { const offset = offsets[scrollAnchor.index]; if (typeof offset !== 'number') { return 0; } if (scrollAnchor.offset === SCROLL_TO_ITEM_END) { - const itemHeight = heights[scrollAnchor.index] ?? 0; + const item = data[scrollAnchor.index]; + const key = item ? keyExtractor(item, scrollAnchor.index) : ''; + const itemHeight = heights[key] ?? 0; return offset + itemHeight - scrollableContainerHeight; } return offset + scrollAnchor.offset; - }, [scrollAnchor, offsets, heights, scrollableContainerHeight]); + }, [ + scrollAnchor, + offsets, + heights, + scrollableContainerHeight, + data, + keyExtractor, + ]); + + const scrollTop = isStickingToBottom + ? Number.MAX_SAFE_INTEGER + : actualScrollTop; const prevDataLength = useRef(data.length); const prevTotalHeight = useRef(totalHeight); - const prevScrollTop = useRef(scrollTop); + const prevScrollTop = useRef(actualScrollTop); const prevContainerHeight = useRef(scrollableContainerHeight); useLayoutEffect(() => { @@ -226,9 +241,7 @@ function VirtualizedList( prevTotalHeight.current - prevContainerHeight.current - 1; const wasAtBottom = contentPreviouslyFit || wasScrolledToBottomPixels; - // If the user was at the bottom, they are now sticking. This handles - // manually scrolling back to the bottom. - if (wasAtBottom && scrollTop >= prevScrollTop.current) { + if (wasAtBottom && actualScrollTop >= prevScrollTop.current) { setIsStickingToBottom(true); } @@ -236,9 +249,6 @@ function VirtualizedList( const containerChanged = prevContainerHeight.current !== scrollableContainerHeight; - // We scroll to the end if: - // 1. The list grew AND we were already at the bottom (or sticking). - // 2. We are sticking to the bottom AND the container size changed. if ( (listGrew && (isStickingToBottom || wasAtBottom)) || (isStickingToBottom && containerChanged) @@ -247,34 +257,28 @@ function VirtualizedList( index: data.length > 0 ? data.length - 1 : 0, offset: SCROLL_TO_ITEM_END, }); - // If we are scrolling to the bottom, we are by definition sticking. if (!isStickingToBottom) { setIsStickingToBottom(true); } - } - // Scenario 2: The list has changed (shrunk) in a way that our - // current scroll position or anchor is invalid. We should adjust to the bottom. - else if ( + } else if ( (scrollAnchor.index >= data.length || - scrollTop > totalHeight - scrollableContainerHeight) && + actualScrollTop > totalHeight - scrollableContainerHeight) && data.length > 0 ) { const newScrollTop = Math.max(0, totalHeight - scrollableContainerHeight); setScrollAnchor(getAnchorForScrollTop(newScrollTop, offsets)); } else if (data.length === 0) { - // List is now empty, reset scroll to top. setScrollAnchor({ index: 0, offset: 0 }); } - // Update refs for the next render cycle. prevDataLength.current = data.length; prevTotalHeight.current = totalHeight; - prevScrollTop.current = scrollTop; + prevScrollTop.current = actualScrollTop; prevContainerHeight.current = scrollableContainerHeight; }, [ data.length, totalHeight, - scrollTop, + actualScrollTop, scrollableContainerHeight, scrollAnchor.index, getAnchorForScrollTop, @@ -334,10 +338,10 @@ function VirtualizedList( const startIndex = Math.max( 0, - findLastIndex(offsets, (offset) => offset <= scrollTop) - 1, + findLastIndex(offsets, (offset) => offset <= actualScrollTop) - 1, ); const endIndexOffset = offsets.findIndex( - (offset) => offset > scrollTop + scrollableContainerHeight, + (offset) => offset > actualScrollTop + scrollableContainerHeight, ); const endIndex = endIndexOffset === -1 @@ -348,6 +352,32 @@ function VirtualizedList( const bottomSpacerHeight = totalHeight - (offsets[endIndex + 1] ?? totalHeight); + // Maintain a stable set of observed nodes using useLayoutEffect + const observedNodes = useRef>(new Set()); + useLayoutEffect(() => { + const currentNodes = new Set(); + for (let i = startIndex; i <= endIndex; i++) { + const node = itemRefs.current[i]; + const item = data[i]; + if (node && item) { + currentNodes.add(node); + const key = keyExtractor(item, i); + // Always update the key mapping because React can reuse nodes at different indices/keys + nodeToKeyRef.current.set(node, key); + if (!observedNodes.current.has(node)) { + itemsObserver.observe(node); + } + } + } + for (const node of observedNodes.current) { + if (!currentNodes.has(node)) { + itemsObserver.unobserve(node); + nodeToKeyRef.current.delete(node); + } + } + observedNodes.current = currentNodes; + }); + const renderedItems = []; for (let i = startIndex; i <= endIndex; i++) { const item = data[i]; @@ -356,6 +386,8 @@ function VirtualizedList( { itemRefs.current[i] = el; }} @@ -376,27 +408,39 @@ function VirtualizedList( setIsStickingToBottom(false); } const currentScrollTop = getScrollTop(); - const newScrollTop = Math.max( - 0, - Math.min( - totalHeight - scrollableContainerHeight, - currentScrollTop + delta, - ), - ); + const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); + const actualCurrent = Math.min(currentScrollTop, maxScroll); + let newScrollTop = Math.max(0, actualCurrent + delta); + if (newScrollTop >= maxScroll) { + setIsStickingToBottom(true); + newScrollTop = Number.MAX_SAFE_INTEGER; + } setPendingScrollTop(newScrollTop); - setScrollAnchor(getAnchorForScrollTop(newScrollTop, offsets)); + setScrollAnchor( + getAnchorForScrollTop(Math.min(newScrollTop, maxScroll), offsets), + ); }, scrollTo: (offset: number) => { - setIsStickingToBottom(false); - const newScrollTop = Math.max( - 0, - Math.min(totalHeight - scrollableContainerHeight, offset), - ); - setPendingScrollTop(newScrollTop); - setScrollAnchor(getAnchorForScrollTop(newScrollTop, offsets)); + const maxScroll = Math.max(0, totalHeight - scrollableContainerHeight); + if (offset >= maxScroll || offset === SCROLL_TO_ITEM_END) { + setIsStickingToBottom(true); + setPendingScrollTop(Number.MAX_SAFE_INTEGER); + if (data.length > 0) { + setScrollAnchor({ + index: data.length - 1, + offset: SCROLL_TO_ITEM_END, + }); + } + } else { + setIsStickingToBottom(false); + const newScrollTop = Math.max(0, offset); + setPendingScrollTop(newScrollTop); + setScrollAnchor(getAnchorForScrollTop(newScrollTop, offsets)); + } }, scrollToEnd: () => { setIsStickingToBottom(true); + setPendingScrollTop(Number.MAX_SAFE_INTEGER); if (data.length > 0) { setScrollAnchor({ index: data.length - 1, @@ -416,10 +460,14 @@ function VirtualizedList( setIsStickingToBottom(false); const offset = offsets[index]; if (offset !== undefined) { + const maxScroll = Math.max( + 0, + totalHeight - scrollableContainerHeight, + ); const newScrollTop = Math.max( 0, Math.min( - totalHeight - scrollableContainerHeight, + maxScroll, offset - viewPosition * scrollableContainerHeight + viewOffset, ), ); @@ -441,10 +489,14 @@ function VirtualizedList( if (index !== -1) { const offset = offsets[index]; if (offset !== undefined) { + const maxScroll = Math.max( + 0, + totalHeight - scrollableContainerHeight, + ); const newScrollTop = Math.max( 0, Math.min( - totalHeight - scrollableContainerHeight, + maxScroll, offset - viewPosition * scrollableContainerHeight + viewOffset, ), ); @@ -454,11 +506,14 @@ function VirtualizedList( } }, getScrollIndex: () => scrollAnchor.index, - getScrollState: () => ({ - scrollTop: getScrollTop(), - scrollHeight: totalHeight, - innerHeight: containerHeight, - }), + getScrollState: () => { + const maxScroll = Math.max(0, totalHeight - containerHeight); + return { + scrollTop: Math.min(getScrollTop(), maxScroll), + scrollHeight: totalHeight, + innerHeight: containerHeight, + }; + }, }), [ offsets, @@ -475,7 +530,7 @@ function VirtualizedList( return ( ( flexShrink={0} width="100%" flexDirection="column" - marginTop={copyModeEnabled ? -scrollTop : 0} + marginTop={copyModeEnabled ? -actualScrollTop : 0} > {renderedItems}