Skip to content

fix(layout): invalidate cached row heights on in-place data swap#2287

Open
pranko17 wants to merge 1 commit into
Shopify:mainfrom
pranko17:fix/grid-overlap-on-data-swap
Open

fix(layout): invalidate cached row heights on in-place data swap#2287
pranko17 wants to merge 1 commit into
Shopify:mainfrom
pranko17:fix/grid-overlap-on-data-swap

Conversation

@pranko17
Copy link
Copy Markdown

@pranko17 pranko17 commented May 20, 2026

Description

When the data prop of a FlashList is replaced in place — that is, the list itself is not unmounted but the underlying array is swapped for a different one (e.g. on category / filter / sort change) — items in a multi-column grid can overlap with the row below them.

The screenshot below is a real reproduction in a production app on @shopify/flash-list@2.3.1 with numColumns={2}. After switching the category, the new product cards in row N are taller than the cards that were previously at those positions, and they overlap into row N+1:

image

Root cause

RecyclerViewManager.processDataUpdate is called from useRecyclerViewManager's useMemo([data]). Its current implementation forwards an empty layoutInfo array to modifyChildrenLayout, so on an in-place data swap the LayoutManager is told that the data length changed but nothing else:

processDataUpdate() {
  if (this.hasLayout()) {
    this.modifyChildrenLayout([], this.propsRef.data?.length ?? 0);
    // ...
  }
}

The per-index entries in LayoutManager.layouts therefore keep their previous values:

  • isHeightMeasured: true (set after the previous render's onLayout)
  • height (the previously measured pixel height)
  • minHeight (the row's normalized height set by processAndReturnTallestItemInRow)

On the next render the new item at that index is mounted by the ViewHolder with the stale minHeight applied as a style. Two things go wrong:

  1. Content-driven overflow. ViewHolder does not enforce height for non-grid items, so the new content stretches the cell past its cached height. The row above looks taller than the layout says it is.
  2. Stale row positioning. recomputeLayouts places the next row's y from the cached height of the current row before any fresh measurement arrives. The next row therefore starts too high, and the overflowing cell of the previous row visually overlaps it.

There is also a second-order effect inside GridLayoutManager.processAndReturnTallestItemInRow: a row's "tallest item" is only nominated if layout.height > layout.minHeight. After the previous render's normalization, the previously tallest item has minHeight = 0 while its neighbours have minHeight === height. When the new data is taller at a previously short position, the comparison height > minHeight fails for the previously normalized neighbour, so it cannot be picked as the tallest. The row's normalized height ends up driven by the wrong item, and one further frame is required before the layout converges — sometimes only after the user scrolls the affected rows back into view.

The library already has clearLayoutCacheOnUpdate() for the heavy case (e.g. orientation change), but that wipes the entire cache and the docs say it should be called manually before render, which doesn't fit in-place data updates.

The fix

Track each render's keyExtractor results per index. When processDataUpdate runs and the layout manager already exists, diff the new keys against the previously recorded ones; for any index whose key changed, clear isHeightMeasured and minHeight on its layout. The next time modifyLayout receives real measurements from the freshly rendered ViewHolders, computeEstimatesAndMinMaxChangedLayout sees !isHeightMeasured and triggers a fresh recompute of those rows. No row is positioned against a stale height for the new content.

The patch is entirely additive on a private code path and is a no-op when keyExtractor is not provided, so it cannot regress existing callers.

What's covered

scenario behavior
index identity changed (partial swap) invalidated
whole array replaced (e.g. category switch) every index invalidated
identical keys re-emitted (e.g. refetch returning the same payload) no-op
append at the tail (pagination) pre-existing indices untouched
no keyExtractor provided no-op, pre-fix behavior preserved
masonry / linear layouts no-op for row normalization but still clears stale minHeight

Reviewers' hat-rack 🎩

  • RecyclerViewManager.invalidateChangedLayouts only mutates layouts whose index is in [0, min(layoutCount, newLength)). Indices that exist only in the previous data (i.e., the array shrunk) are trimmed inside modifyLayout as before. Indices that exist only in the new data are created by modifyLayout after this call and are not touched.
  • recordCurrentDataKeys is called from both branches of processDataUpdate and from the early-return inside invalidateChangedLayouts, so prevDataKeys stays in sync with the most recently rendered data even when the layout manager isn't ready yet (first paint).
  • When keyExtractor is undefined, prevDataKeys is explicitly truncated to zero length so a later flip to providing a keyExtractor doesn't reuse stale keys from an earlier session.

Tests

src/__tests__/RecyclerViewManager.test.ts adds five new cases under processDataUpdate — layout invalidation on in-place data swap. Full suite (yarn jest) — 188/188 passing locally. yarn lint and yarn type-check clean.

Screenshots or videos

Repro is a 2-column product list with cards of varying height. Switching categories swaps the underlying data in place. Before the fix, the first frame after the swap shows cards from the new category overlapping the row below; after the fix, the next row is positioned against the fresh measurement and there is no overlap.

When the `data` prop is replaced with a different array of items (e.g.
filter / sort / category change) without remounting the list, indices
that were already measured retain their `isHeightMeasured`, `height`
and `minHeight` from the previous data. The ViewHolder for the new
item at that position then renders against the stale `minHeight`, and
the LayoutManager places the next row at a Y derived from the stale
`height`. In multi-column grids this surfaces as a taller new card
overflowing into the row below.

Track per-index `keyExtractor` results across renders. In
`processDataUpdate`, clear `isHeightMeasured` and `minHeight` for any
index whose key changed, so the next `modifyLayout` pass recomputes
the row against the new content. No-op when `keyExtractor` is not
provided.

Tests cover: per-index identity change, full array replacement,
re-emit of identical keys, append (pagination), and absence of
`keyExtractor`.
@pranko17 pranko17 force-pushed the fix/grid-overlap-on-data-swap branch from 2548d0f to 406e17e Compare May 20, 2026 09:09
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