Skip to content

fix(recyclerview): move the scroll anchor in the same commit as the items it compensates for - #2537

Open
neddtu wants to merge 2 commits into
Shopify:mainfrom
neddtu:fix/anchor-shift-in-same-commit
Open

neddtu wants to merge 2 commits into
Shopify:mainfrom
neddtu:fix/anchor-shift-in-same-commit

Conversation

@neddtu

@neddtu neddtu commented Oct 13, 2026

Copy link
Copy Markdown

Depends on #2536 (stacked): the first commit of this branch is that PR; only the second commit is new here. I will rebase once #2536 is merged.

Problem

When a measurement changes item sizes above the anchor, RecyclerView's layout effect sees modifyChildrenLayout(...) === true and re-renders (setRenderId) so the items move. The scroll anchor (ScrollAnchor.scrollBy, compensated natively by maintainVisibleContentPosition) is only moved later, in applyOffsetCorrection, once a pass has nothing left to re-render — i.e. one commit after the items moved. A frame drawn between the two commits shows the content displaced by the correction, then snapping back. On a fast device this is rarely visible; on a slow one it is a hitch on every measurement while scrolling (each newly measured item above the viewport).

Fix

  • Extract the anchor part of applyOffsetCorrection into shiftAnchor(hasDataChanged).
  • New applyAnchorShift() returned by useRecyclerViewController: applies only the anchor shift, for measurements only (data length unchanged), and returns the shift.
  • RecyclerView calls applyAnchorShift() right before setRenderId(...) in the branch that re-renders after a measurement, so the anchor moves in the same commit as the items and the native side compensates in the same frame.
  • A change in data length keeps the existing path (correction after the commit, JS offset sync, ignoreScrollEvents); pending scroll callbacks are left for the render they wait on.
  • firstVisibleItemRecordedLayout: the "anchor still in view" test from PR 2 now uses the anchor as recorded. After an early shift the stored layout has moved but the JS scroll offset only catches up with the next native scroll event; comparing the two would turn away an anchor that is still on screen and lose a later correction in the same pass.

Tests

Added to src/__tests__/useRecyclerViewController.test.tsx (on top of PR 2's file):

  • moves the anchor in the same pass that measures an item above it
  • shifts once per measurement, however many passes follow it
  • holds the anchor through measurements that follow a shift in the same pass — fails if the in-view test uses the shifted layout instead of the recorded one (Expected: 1250, Received: 1200), which is why firstVisibleItemRecordedLayout exists.
  • leaves a change in the number of items to the correction after the commit — early shift returns 0; the after-commit path still syncs the offset and sets ignoreScrollEvents.
  • is called by the list in the pass that re-renders measured items — checks that RecyclerView.tsx calls applyAnchorShift() right before setRenderId(. This reads the source file; happy to drop it or replace it with a mounted RecyclerView test if you prefer.

All 7 tests in the file fail on PR 2 alone (no applyAnchorShift) and pass with this change. Verified locally on the published src of 2.3.3 with PRs 1–3 applied: 13/13 tests in the touched test files pass; tsc reports no new errors outside the test file's Node imports (fs, path; src/__tests__ is excluded from the build's typecheck).

Notes

  • Behaviour change is limited to when the anchor moves for measurement-driven corrections; the amount is the same.
  • Platforms without offset-correction support keep using scrollTo, now called from the same place.

@neddtu

neddtu commented Oct 13, 2026

Copy link
Copy Markdown
Author

I have signed the CLA.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant