Skip to content

Improve VirtualizedList test for render area change with initialScrollIndex non zero#56358

Closed
chicio wants to merge 3 commits into
react:mainfrom
chicio:virtualizedlist_initialscrollindex_renderarea_test
Closed

Improve VirtualizedList test for render area change with initialScrollIndex non zero#56358
chicio wants to merge 3 commits into
react:mainfrom
chicio:virtualizedlist_initialscrollindex_renderarea_test

Conversation

@chicio

@chicio chicio commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary:

In VirtualizedList-test.js, the test adjusts render area with non-zero initialScrollIndex relied on a hardcoded timer jest.advanceTimersToNextTimer(3) to wait for VirtualizedList to expand its render window when initialScrollIndex is passed as prop.

When initialScrollIndex > 0, VirtualizedList blocks the render area recalculation until a valid scroll event is received (pendingScrollUpdateCount). Once unblocked, a series of batched setTimeout callbacks is scheduled through _scheduleCellsToRenderUpdate (triggered from _onLayout, _onContentSizeChange, and then from componentDidUpdate). Those callbacks eventually lead to updates of cellsAroundViewport.first/last in _updateCellsToRender.
The cellsAroundViewport property of VirtualizedList describes the currently rendered range of item indices, including overscan items outside the visible viewport. The hardcoded magic number 3 was effectively tied to how many internal timer callbacks fired from the chain of calls above.

So I replaced the fixed count (3) with a small helper function, advanceUntilRenderAreaChanged, that advances timers one tick at a time (every time in its own act to let the VirtualizedList component update) and stops as soon as cellsAroundViewport changes.
If this doesn't happened, I trigger a specific error to report that the render area did not been change as expected.

Changelog:

[GENERAL][FIXED] - Improve render area change with initialScrollIndex non zero test in VirtualizedList to avoid magic numbers timers

Test Plan:

  • Ran the VirtualizedList-test.js and verified all test cases passed.
  • Verified test modification correctness by intentionally breaking the related snapshot and check that it is breaking/it is failing.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2026
@chicio chicio changed the title Improve render area change with initialScrollIndex non zero test in VirtualizedList to avoid magic numbers timers Improve VirtualizedList test - render area change with initialScrollIndex non zero Apr 7, 2026
@chicio chicio changed the title Improve VirtualizedList test - render area change with initialScrollIndex non zero Improve VirtualizedList test for render area change with initialScrollIndex non zero Apr 7, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 7, 2026
@chicio

chicio commented Apr 13, 2026

Copy link
Copy Markdown
Contributor Author

Maybe @Abbondanzo you can help me to import this one? Thanks 🙏

@meta-codesync

meta-codesync Bot commented Apr 13, 2026

Copy link
Copy Markdown

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D100649294.

@chicio

chicio commented Apr 14, 2026

Copy link
Copy Markdown
Contributor Author

@Abbondanzo has imported this pull request. If you are a Meta employee, you can view this in D100649294.

Thank you @Abbondanzo 🙏. Let me know if you see any problem or if we can merge 🚀

@chicio

chicio commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@Abbondanzo can you help me to make this be merged in master? Thank you as always🙏

@meta-codesync meta-codesync Bot closed this in 9b966d1 Apr 15, 2026
@facebook-github-tools facebook-github-tools Bot added the Merged This PR has been merged. label Apr 15, 2026
@meta-codesync

meta-codesync Bot commented Apr 15, 2026

Copy link
Copy Markdown

@Abbondanzo merged this pull request in 9b966d1.

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @chicio in 9b966d1

When will my fix make it into a release? | How to file a pick request?

@chicio

chicio commented Apr 15, 2026

Copy link
Copy Markdown
Contributor Author

@Abbondanzo merged this pull request in 9b966d1.

You are the best, thank you 🙏

meta-codesync Bot pushed a commit that referenced this pull request May 8, 2026
…is appended" test (#56653)

Summary:
This PR is a follow-up of #56358, in order to improve and VirtualizedList tests.
I re-enabled the skipped test `retains batch render region when an item is appended`.
With React 19, the previous approach that was using `jest.runAllTimersAsync` in this test path was unstable because the pre-update render region could still be processing updates.
I adopted the same approach used in #56358 by introducing a small helper function, `advanceUntilLastCellIndexRendered`, which advances timers one step at a time and stops when the expected state is reached: `cellsAroundViewport.last === items.length - 1`.

As part of this follow-up, I also aligned `advanceUntilRenderAreaChanged` to use performNextBatch for consistent stepwise timer advancement.

## Changelog:

[GENERAL][FIXED] - Re-enabled VirtualizedList "retains batch render region when an item is appended" tes

Pull Request resolved: #56653

Test Plan:
- Ran the VirtualizedList-test.js and verified all test cases passed.
- Verified test modification correctness by intentionally breaking the related snapshot and check that it is breaking/it is failing.

Reviewed By: cortinico

Differential Revision: D104291284

Pulled By: Abbondanzo

fbshipit-source-id: e32f8707a837a1805ce130d526768228f684e3e7
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. Merged This PR has been merged. 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.

2 participants