Release high-priority list updates after no-op commits (#59295)

Summary:
Pull Request resolved: #59295

`VirtualizedList` can leave its high-priority update flag set when a queued cells updater becomes a no-op, because `componentDidUpdate` does not run. Complete the high-priority attempt using a `setState` callback so the next urgent window update does not wait for the batching timer.

Scope completion to its attempt so an older callback cannot clear newer work scheduled during `componentDidUpdate` or `onEndReached`. Preserve the no-op fast path and the existing once-per-commit scheduling protection.

Changelog:
[General][Fixed] - Avoid delaying urgent `VirtualizedList` rendering after queued no-op updates

Reviewed By: andrewdacenko

Differential Revision: D126635244

fbshipit-source-id: 174d822412f048d556642eb358103ea9a811320f

Author: javache

packages/virtualized-lists/Lists/VirtualizedList.js

@@ -1482,6 +1482,7 @@ class VirtualizedList extends StateSafePureComponent<
   _hasWarned: {[string]: boolean} = {};
   _headerLength = 0;
   _hiPriInProgress: boolean = false; // flag to prevent infinite hiPri cell limit update
+  _hiPriUpdateId: number = 0;
   _indicesToKeys: Map<number, string> = new Map();
   _isAncestorSuppressed: boolean = false;
   _lastFocusedCellKey: ?string = null;
@@ -2094,7 +2095,12 @@ class VirtualizedList extends StateSafePureComponent<
         clearTimeout(this._updateCellsToRenderTimeoutID);
         this._updateCellsToRenderTimeoutID = null;
       }
-      this._updateCellsToRender();
+      const hiPriUpdateId = ++this._hiPriUpdateId;
+      this._updateCellsToRender(() => {
+        if (this._hiPriUpdateId === hiPriUpdateId) {
+          this._hiPriInProgress = false;
+        }
+      });
       return;
     } else {
       if (this._updateCellsToRenderTimeoutID == null) {
@@ -2200,7 +2206,7 @@ class VirtualizedList extends StateSafePureComponent<
     this.props.onMomentumScrollEnd && this.props.onMomentumScrollEnd(e);
   };
 
-  _updateCellsToRender = () => {
+  _updateCellsToRender = (onUpdateComplete?: () => void) => {
     this._updateViewableItems(this.props, this.state.cellsAroundViewport);
     this._notifyCrossOrientationChildren();
 
@@ -2217,17 +2223,15 @@ class VirtualizedList extends StateSafePureComponent<
       this._listMetrics.getContentLength() > 0 &&
       VirtualizedList._computeCellsUpdate(this, this.props, this.state) == null
     ) {
-      // No update means no componentDidUpdate to reset this, which would
-      // push the next hiPri update onto the batching timer.
-      this._hiPriInProgress = false;
+      onUpdateComplete?.();
       return;
     }
 
     this._hasPendingStateUpdate = true;
     this.setState<'cellsAroundViewport' | 'renderMask'>((state, props) => {
       this._hasPendingStateUpdate = false;
       return VirtualizedList._computeCellsUpdate(this, props, state);
-    });
+    }, onUpdateComplete);
   };
 
   _createViewToken = (

packages/virtualized-lists/Lists/__tests__/VirtualizedList-test.js

@@ -1695,6 +1695,17 @@ it('does not move render area when initialScrollIndex is > 0 and offset not yet
   // 5 cells should be present starting at index 1, since we have not seen a
   // scroll event yet for current position.
   expect(component).toMatchSnapshot();
+  const instance = component.getInstance();
+  expect(instance.state.pendingScrollUpdateCount).toBe(1);
+  expect(instance._hiPriInProgress).toBe(false);
+
+  await act(() => {
+    simulateScroll(component, {x: 0, y: 10});
+  });
+
+  expect(instance.state.pendingScrollUpdateCount).toBe(0);
+  expect(instance.state.cellsAroundViewport.last).toBeGreaterThan(5);
+  expect(instance._hiPriInProgress).toBe(false);
 });
 
 it('clamps render area when items removed for initialScrollIndex > 0 and scroller position not yet known', async () => {
@@ -2278,6 +2289,7 @@ it('renders windowSize derived region at top', async () => {
 it('does not update state on a scroll that keeps the render window', async () => {
   const items = generateItems(10);
   const ITEM_HEIGHT = 10;
+  const onEndReached = jest.fn();
 
   let component;
   await act(() => {
@@ -2286,6 +2298,7 @@ it('does not update state on a scroll that keeps the render window', async () =>
         initialNumToRender={1}
         maxToRenderPerBatch={1}
         windowSize={3}
+        onEndReached={onEndReached}
         {...baseItemProps(items)}
         {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
       />,
@@ -2314,6 +2327,96 @@ it('does not update state on a scroll that keeps the render window', async () =>
   });
 
   expect(setStateSpy).not.toHaveBeenCalled();
+
+  const instance = component.getInstance();
+  const componentDidUpdateSpy = jest.spyOn(instance, 'componentDidUpdate');
+
+  await act(() => {
+    // The high-priority update reads the final scroll position when React
+    // processes it, so returning to the top makes the queued update a no-op.
+    simulateScroll(component, {x: 0, y: 60});
+    simulateScroll(component, {x: 0, y: 0});
+  });
+
+  expect(setStateSpy).toHaveBeenCalledTimes(1);
+  expect(componentDidUpdateSpy).not.toHaveBeenCalled();
+  expect(instance._hiPriInProgress).toBe(false);
+
+  const previousWindow = instance.state.cellsAroundViewport;
+  await act(() => {
+    simulateScroll(component, {x: 0, y: 60});
+  });
+
+  // No timer tick should be needed for the next high-priority update.
+  expect(instance.state.cellsAroundViewport.last).toBeGreaterThan(
+    previousWindow.last,
+  );
+  expect(componentDidUpdateSpy).toHaveBeenCalledTimes(1);
+  expect(instance._hiPriInProgress).toBe(false);
+
+  const updateCellsToRenderSpy = jest.spyOn(instance, '_updateCellsToRender');
+  const computeCellsUpdateSpy = jest.spyOn(
+    VirtualizedList,
+    '_computeCellsUpdate',
+  );
+  componentDidUpdateSpy.mockClear();
+  await act(() => {
+    simulateScroll(component, {x: 0, y: 0});
+    for (let i = 0; i < items.length; i++) {
+      instance._updateCellsToRender();
+    }
+  });
+
+  expect(
+    computeCellsUpdateSpy.mock.results.some(({value}) => value == null),
+  ).toBe(true);
+  expect(
+    computeCellsUpdateSpy.mock.results.some(({value}) => value != null),
+  ).toBe(true);
+  expect(componentDidUpdateSpy).toHaveBeenCalledTimes(1);
+  expect(updateCellsToRenderSpy).toHaveBeenCalledTimes(items.length + 1);
+  expect(instance._hiPriInProgress).toBe(false);
+  computeCellsUpdateSpy.mockRestore();
+
+  let reenteredFromOnEndReached = false;
+  onEndReached.mockImplementationOnce(() => {
+    reenteredFromOnEndReached = true;
+    simulateScroll(component, {x: 0, y: 0});
+    expect(instance._hiPriInProgress).toBe(true);
+  });
+
+  const setState = VirtualizedList.prototype.setState.bind(instance);
+  setStateSpy.mockRestore();
+  jest.spyOn(instance, 'setState').mockImplementation((updater, onComplete) => {
+    setState(
+      updater,
+      onComplete == null
+        ? undefined
+        : () => {
+            onComplete();
+            if (
+              reenteredFromOnEndReached &&
+              instance.state.cellsAroundViewport.last === items.length - 1
+            ) {
+              // The old completion must not release the new update queued
+              // by onEndReached during componentDidUpdate.
+              expect(instance._hiPriInProgress).toBe(true);
+              instance._updateCellsToRender();
+              expect(instance._hiPriInProgress).toBe(true);
+            }
+          },
+    );
+  });
+
+  await act(() => {
+    simulateScroll(component, {x: 0, y: 80});
+    for (let i = 0; i < items.length; i++) {
+      instance._updateCellsToRender();
+    }
+  });
+
+  expect(reenteredFromOnEndReached).toBe(true);
+  expect(instance._hiPriInProgress).toBe(false);
 });
 
 it('renders windowSize derived region in middle', async () => {