Skip to content

Fix maintainVisibleContentPosition prepend skipped when batched with a cells update - #58911

Open
mozzius wants to merge 1 commit into
react:mainfrom
mozzius:virtualized-list-batched-prepend
Open

mozzius wants to merge 1 commit into
react:mainfrom
mozzius:virtualized-list-batched-prepend

Conversation

@mozzius

@mozzius mozzius commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Summary:

VirtualizedList.getDerivedStateFromProps returns early when the item count equals prevState.renderMask.numCells(). React runs queued _updateCellsToRender updaters before derived state, so when a cells update renders together with a data change, the mask already has the new count and a maintainVisibleContentPosition prepend is skipped. This reads the key at minIndexForVisible before the early return, and on mVCP lists only returns early when both the count and the key are unchanged.

Fixes #58909. Repro: https://github.com/mozzius/virtualizedlist-batched-prepend-repro

Changelog:

[GENERAL] [FIXED] - VirtualizedList: handle a maintainVisibleContentPosition prepend that renders together with a cells update

Test Plan:

  • New test in VirtualizedList-test.js fails before the change and passes after. Existing virtualized-lists tests and snapshots are unchanged.
  • yarn flow-check and yarn lint are clean.
  • The repro jumps on stock and holds with the fix: 4/4 runs on iOS, 2/2 on Android.

🤖 Generated with Claude Code

@AlexNewStart

Copy link
Copy Markdown

We hit the same bug independently and ship this exact change as a patch-package patch on 0.86.3, so +1 from production. Two things that may help this land:

A second trigger, on Android. Our list is a chat transcript (maintainVisibleContentPosition={{minIndexForVisible: 0}}, onStartReached, disableVirtualization) that prepends older pages of 100 messages fetched from the network. On a Galaxy A15, 2 of 8 page loads left the reader on the oldest rows of the new page. The content height shrank to about one page when the count went from 100 to 200, so the previous 100 rows had been unmounted, and with them the anchor MaintainVisibleScrollPositionHelper needed: the offset stayed at 0. The cells update there came from the batching timer after a scroll event, not from the high-priority path, so this isn't limited to apps that update data from onScroll.

Since #59134 (2c635d7), the regression test in this PR passes on main without the fix. _updateCellsToRender now computes the update first and skips setState when the window wouldn't change. The test's explicit _updateCellsToRender() is such a no-op (nothing has scrolled, the window stays 0..4), so it never reaches the updater, and getDerivedStateFromProps sees the old count. The result is 10..14 / pendingScrollUpdateCount 1 / firstVisibleItemKey 20 with or without the change. Our at-rest timer case is masked on main for the same reason. The bug is still there whenever the batched cells update has real work to do. These two tests fail on main (cd45f30) and pass with this PR:

it('handles a maintainVisibleContentPosition prepend batched with a cells update that moves the window', async () => {
  const items = generateItems(20);
  const ITEM_HEIGHT = 10;
  const list = data => (
    <VirtualizedList
      initialNumToRender={1}
      windowSize={1}
      maintainVisibleContentPosition={{minIndexForVisible: 0}}
      {...baseItemProps(data)}
      {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
    />
  );

  let component;
  await act(() => {
    component = create(list(items));
  });
  await act(() => {
    simulateLayout(component, {
      viewport: {width: 10, height: 50},
      content: {width: 10, height: items.length * ITEM_HEIGHT},
    });
    performAllBatches();
  });
  const instance = component.getInstance();
  expect(instance.state.cellsAroundViewport).toEqual({first: 0, last: 4});

  // Scroll one viewport down and prepend 10 items from the same event. The
  // cells update has to move the window, so it isn't skipped, and its updater
  // runs with the new props.
  await act(() => {
    simulateScroll(component, {x: 0, y: 50});
    component.update(list([...generateItems(10, items.length), ...items]));
  });

  // Rows 5..9, now in the viewport, are at 15..19 after the prepend.
  // main: {first: 4, last: 9}, pendingScrollUpdateCount 0, firstVisibleItemKey 0
  expect(instance.state.cellsAroundViewport).toEqual({first: 14, last: 19});
  expect(instance.state.pendingScrollUpdateCount).toBe(1);
  expect(instance.state.firstVisibleItemKey).toBe(20);
});

it('handles a maintainVisibleContentPosition prepend after an equal-count change at the head', async () => {
  const items = generateItems(20);
  const ITEM_HEIGHT = 10;
  const list = data => (
    <VirtualizedList
      initialNumToRender={1}
      windowSize={1}
      maintainVisibleContentPosition={{minIndexForVisible: 0}}
      {...baseItemProps(data)}
      {...fixedHeightItemLayoutProps(ITEM_HEIGHT)}
    />
  );

  let component;
  await act(() => {
    component = create(list(items));
  });
  await act(() => {
    simulateLayout(component, {
      viewport: {width: 10, height: 50},
      content: {width: 10, height: items.length * ITEM_HEIGHT},
    });
    performAllBatches();
  });
  const instance = component.getInstance();

  // The head item leaves and one is appended: the count holds, the key at
  // minIndexForVisible changes.
  const shiftedItems = [...generateItems(19, 1), {key: 20}];
  await act(() => {
    component.update(list(shiftedItems));
    performAllBatches();
  });
  await act(() => {
    component.update(list([...generateItems(10, 100), ...shiftedItems]));
  });

  // main: {first: 0, last: 4}; firstVisibleItemKey was left at 0 by the update above
  expect(instance.state.cellsAroundViewport).toEqual({first: 10, last: 14});
  expect(instance.state.firstVisibleItemKey).toBe(100);
});

How I ran them: main's packages/virtualized-lists at cd45f30 under @react-native/jest-preset 0.86.3, with react-native/react-private-interface stubbed to the three flags it reads at their defaults (false). That isn't the monorepo's own Jest run, so it's worth re-checking there. The PR's existing test passes on that setup both with and without the change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

VirtualizedList: a maintainVisibleContentPosition prepend is ignored when it renders together with a cells update, and the content jumps

2 participants