From df7b6ae092d03385ebd05efd0f068c59e727f723 Mon Sep 17 00:00:00 2001 From: Andrei Marchenko Date: Tue, 26 Nov 2024 10:44:30 -0800 Subject: [PATCH] fix item disappearing with scroll in VirtualizedList (#47965) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47965 Changelog: [General] [Changed] - fix item disappearing with scroll in VirtualizedList It was caused because the function `computeWindowedRenderLimits` collapsed the current window size to just 1 element. So, users start scroll current window increase from {left:0, right:5 } -> {left:0, right:6 } and after some edge cases the window collapsed to `{left:6, right:6 }` which cause to remove all elements and recreate them later. As a result users have a lot of lags and blank pages. The diff fixes the collapsing window size to 1 element. Also fix other decreasing `left` position even if windowSize more than current amount of elements. Reviewed By: NickGerleman Differential Revision: D66334188 fbshipit-source-id: 2162d00d03d64ab6325c0492d87449051e68a4e9 --- .../ReactNativeFeatureFlags.config.js | 9 + .../featureflags/ReactNativeFeatureFlags.js | 8 +- .../Lists/VirtualizeUtils.js | 16 +- .../Lists/__tests__/VirtualizeUtils-test.js | 160 +++++++++++++++++- 4 files changed, 189 insertions(+), 4 deletions(-) diff --git a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js index 3b57f30d22b27a..71beaac5cc39aa 100644 --- a/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js +++ b/packages/react-native/scripts/featureflags/ReactNativeFeatureFlags.config.js @@ -511,6 +511,15 @@ const definitions: FeatureFlagDefinitions = { purpose: 'release', }, }, + fixVirtualizeListCollapseWindowSize: { + defaultValue: false, + metadata: { + dateAdded: '2024-11-22', + description: + 'Fixing an edge case where the current window size is not properly calculated with fast scrolling. Window size collapsed to 1 element even if windowSize more than the current amount of elements', + purpose: 'experimentation', + }, + }, isLayoutAnimationEnabled: { defaultValue: true, metadata: { diff --git a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js index 35d450fa8db65f..91814b1a9d16f3 100644 --- a/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js +++ b/packages/react-native/src/private/featureflags/ReactNativeFeatureFlags.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @generated SignedSource<<42e14ee5e2201b2fcbf9fe550e5165a6>> + * @generated SignedSource<<38ad29621eeb29a9f82735dc187c13d4>> * @flow strict */ @@ -36,6 +36,7 @@ export type ReactNativeFeatureFlagsJsOnly = { enableAnimatedAllowlist: Getter, enableAnimatedClearImmediateFix: Getter, enableAnimatedPropsMemo: Getter, + fixVirtualizeListCollapseWindowSize: Getter, isLayoutAnimationEnabled: Getter, shouldSkipStateUpdatesForLoopingAnimations: Getter, shouldUseAnimatedObjectForTransform: Getter, @@ -143,6 +144,11 @@ export const enableAnimatedClearImmediateFix: Getter = createJavaScript */ export const enableAnimatedPropsMemo: Getter = createJavaScriptFlagGetter('enableAnimatedPropsMemo', true); +/** + * Fixing an edge case where the current window size is not properly calculated with fast scrolling. Window size collapsed to 1 element even if windowSize more than the current amount of elements + */ +export const fixVirtualizeListCollapseWindowSize: Getter = createJavaScriptFlagGetter('fixVirtualizeListCollapseWindowSize', false); + /** * Function used to enable / disabled Layout Animations in React Native. */ diff --git a/packages/virtualized-lists/Lists/VirtualizeUtils.js b/packages/virtualized-lists/Lists/VirtualizeUtils.js index 012ef5ba97f07f..6697d479691eed 100644 --- a/packages/virtualized-lists/Lists/VirtualizeUtils.js +++ b/packages/virtualized-lists/Lists/VirtualizeUtils.js @@ -14,6 +14,8 @@ import type ListMetricsAggregator, { CellMetricProps, } from './ListMetricsAggregator'; +import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags'; + /** * Used to find the indices of the frames that overlap the given offsets. Useful for finding the * items that bound different windows of content, such as the visible area or the buffered overscan @@ -176,10 +178,20 @@ export function computeWindowedRenderLimits( break; } const maxNewCells = newCellCount >= maxToRenderPerBatch; - const firstWillAddMore = first <= prev.first || first > prev.last; + + let firstWillAddMore; + let lastWillAddMore; + + if (ReactNativeFeatureFlags.fixVirtualizeListCollapseWindowSize()) { + firstWillAddMore = first <= prev.first; + lastWillAddMore = last >= prev.last; + } else { + firstWillAddMore = first <= prev.first || first > prev.last; + lastWillAddMore = last >= prev.last || last < prev.first; + } + const firstShouldIncrement = first > overscanFirst && (!maxNewCells || !firstWillAddMore); - const lastWillAddMore = last >= prev.last || last < prev.first; const lastShouldIncrement = last < overscanLast && (!maxNewCells || !lastWillAddMore); if (maxNewCells && !firstShouldIncrement && !lastShouldIncrement) { diff --git a/packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js b/packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js index 9af659dac5ba9d..27ce5813f0276c 100644 --- a/packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js +++ b/packages/virtualized-lists/Lists/__tests__/VirtualizeUtils-test.js @@ -10,7 +10,13 @@ 'use strict'; -import {elementsThatOverlapOffsets, newRangeCount} from '../VirtualizeUtils'; +import ListMetricsAggregator from '../ListMetricsAggregator'; +import { + computeWindowedRenderLimits, + elementsThatOverlapOffsets, + newRangeCount, +} from '../VirtualizeUtils'; +import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags'; describe('newRangeCount', function () { it('handles subset', function () { @@ -116,3 +122,155 @@ function fakeProps(length) { getItemCount: () => length, }; } + +describe('computeWindowedRenderLimits', function () { + const defaultProps = { + getItemCount: () => 10, + getItem: (_, index) => ({key: `test_${index}`}), + getItemLayout: (_, index) => { + return {index, length: 100, offset: index * 100}; + }, + }; + + const defaultScrollMetrics = { + dt: 16, + dOffset: 0, + offset: 0, + timestamp: 0, + velocity: 0, + visibleLength: 500, + zoomScale: 1, + }; + + it('renders all items when list is small', function () { + const props = { + ...defaultProps, + getItemCount: () => 3, + }; + const result = computeWindowedRenderLimits( + props, + 5, + 10, + {first: 0, last: 2}, + new ListMetricsAggregator(), + defaultScrollMetrics, + ); + expect(result).toEqual({first: 0, last: 2}); + }); + + it('handles overflow cases when window size suddenly collapses', function () { + ReactNativeFeatureFlags.override({ + fixVirtualizeListCollapseWindowSize: () => true, + }); + + const listMetricsAggregator = new ListMetricsAggregator(); + listMetricsAggregator._contentLength = 2713.60009765625; + + const offsets = [ + {index: 0, length: 275, offset: 0}, + {index: 1, length: 352, offset: 275}, + {index: 2, length: 326, offset: 627}, + {index: 3, length: 352, offset: 953}, + {index: 4, length: 293, offset: 1305}, + {index: 5, length: 293, offset: 1598}, + {index: 6, length: 293, offset: 1891}, + {index: 7, length: 293, offset: 2184}, + ]; + + expect( + computeWindowedRenderLimits( + { + ...defaultProps, + getItemCount: () => 8, + getItemLayout: (_, index) => { + return offsets[index]; + }, + }, + 1, + 31, + {first: 0, last: 5}, + listMetricsAggregator, + { + dt: 949, + dOffset: 879.2000732421875, + offset: 2073.60009765625, + timestamp: 1732180589708, + velocity: 0.9264489707504611, + visibleLength: 640, + }, + ), + ).toEqual({first: 0, last: 6}); + }); + + it('handles reaching the end of the list', function () { + const listMetricsAggregator = new ListMetricsAggregator(); + listMetricsAggregator._contentLength = 1000; + + const offsets = Array.from({length: 10}, (_, index) => ({ + index, + length: 100, + offset: index * 100, + })); + + expect( + computeWindowedRenderLimits( + { + ...defaultProps, + getItemLayout: (_, index) => offsets[index], + }, + 2, + 5, + {first: 5, last: 9}, + listMetricsAggregator, + { + dt: 100, + dOffset: 100, + offset: 900, + timestamp: 1000, + velocity: 1, + visibleLength: 300, + }, + ), + ).toEqual({first: 3, last: 9}); + }); + + it('respects maxToRenderPerBatch when adding new cells', function () { + const scrollMetrics = { + ...defaultScrollMetrics, + offset: 0, + dOffset: 0, + velocity: 0, + }; + const prev = {first: 0, last: 2}; + const result = computeWindowedRenderLimits( + defaultProps, + 2, // maxToRenderPerBatch + 5, // windowSize + prev, + new ListMetricsAggregator(), + scrollMetrics, + ); + expect(result).toEqual({first: 0, last: 4}); + }); + + it('handles case where overscanFirst and overscanLast encompass entire list', function () { + const props = { + ...defaultProps, + getItemCount: () => 5, + }; + const scrollMetrics = { + ...defaultScrollMetrics, + offset: 0, + visibleLength: 1000, + }; + const result = computeWindowedRenderLimits( + props, + 5, + 10, // windowSize large enough to cover entire list + {first: 0, last: 4}, + new ListMetricsAggregator(), + scrollMetrics, + ); + expect(result).toEqual({first: 0, last: 4}); + }); +});