Skip to content

ViewHolderCollection render throws "index out of bounds, not enough layouts" when the render stack outlives a layout-table shrink #2440

Description

@hoangzinh

Description

ViewHolderCollection can throw index out of bounds, not enough layouts while rendering, taking down the tree. It happens when the render stack (RenderStackManager.keyMap) still holds an entry whose index was removed from the layout table (LayoutManager.layouts): the render path reads layouts through an unguarded getLayout(index), which throws instead of returning undefined.

Same error and same getLayout family as #2291, different call site — that one is the validateItemSize measurement callback, this one is the render path.

Current behavior

RecyclerViewManager.modifyChildrenLayout() truncates the layout table on its first line, but prunes the render stack only on some of its exit paths (RecyclerViewManager.ts#L259-L281):

modifyChildrenLayout(layoutInfo, dataLength) {
  this.layoutManager?.modifyLayout(layoutInfo, dataLength);   // truncates layouts when shrinking
  if (dataLength === 0) return false;                          // render stack not pruned
  if (this.layoutManager?.requiresRepaint) { ; return true; } // render stack not pruned
  if (this.hasRenderedProgressively) {
    if (!this.isFirstPaintOnUiComplete) return false;          // render stack not pruned
    return this.recomputeEngagedIndices() !== undefined;       // prunes ONLY if the engaged range changed
  } else {
    this.renderProgressively();                                // this branch does sync the render stack
  }
  
}

On the main path, recomputeEngagedIndices()updateScrollOffset() calls updateRenderStack() behind if (engagedIndices) (#L111-L127), and EngagedIndicesTracker.updateScrollOffset() returns undefined when the recomputed range has the same endpoints as before (EngagedIndicesTracker.ts#L177-L182). When it reports no change, RenderStackManager.sync() never runs with the new dataLength, so keys pointing at removed indices survive.

RecyclerView renders that state immediately — setRenderId((prev) => prev + 1) on a truthy return, or viewHolderCollectionRef.current?.commitLayout() on a falsy one (RecyclerView.tsx#L237-L246). ViewHolderCollection then walks the render stack and spreads getLayout(index) for each entry (ViewHolderCollection.tsx#L178-L195), which is wired to the unguarded read (RecyclerView.tsx#L589) → LayoutManager.getLayout() throws (LayoutManager.ts#L230-L233):

index out of bounds, not enough layouts

In our production reports this arrives chained under React's "There was an error during concurrent rendering but React was able to recover by instead synchronously rendering the entire root."

Expected behavior

Where the out-of-bounds index comes from

processDataUpdate() maintains this invariant explicitly — it re-syncs the render stack even when recomputeEngagedIndices() reports no change, with the comment "It's important to update render stack so that elements are assgined right keys incase items were deleted" (RecyclerViewManager.ts#L302-L311). modifyChildrenLayout() performs the same truncation but has no equivalent fallback, so callers reaching it directly — the measurement useLayoutEffect at RecyclerView.tsx#L237-L246 — can leave the render stack holding removed indices, and its requiresRepaint and !isFirstPaintOnUiComplete exits return after the truncation too. We haven't attempted a fix there; flagging it in case it's the better place to solve this.

Suggested fix

Make the render read bounds-safe, as the library already does elsewhere: StickyHeaders (3 call sites), the measurement effect's measureItemLayout call, and the public getLayout ref method all use tryGetLayout (RecyclerViewManager.ts#L145-L153), and modifyLayout filters stale indices out of incoming measurements with the comment "layoutInfo may contain stale indices from ViewHolders that were rendered before the data shrunk. Filter out any indices that are now out of bounds." The render path at RecyclerView.tsx#L589 and validateItemSize (#2291) are the two reads that throw instead.

// RecyclerView.tsx
getLayout={(index) => recyclerViewManager.tryGetLayout(index)}

// ViewHolderCollection.tsx
const layout = getLayout(index);
if (layout === undefined) {
  return null;
}

This is safe on its own: the crash requires index >= layouts.length, and after a truncation layouts.length equals the new data length — so a skipped entry always points at an item that no longer exists and nothing visible is dropped. The only cost is that the recycled key stays unused until the next render-stack sync.

Reproduction

Expo Snack or minimal reproduction link:

We don't have a Snack: the runtime trigger is timing-dependent and we can't reproduce it on demand (see Additional context). The invariant break itself, though, is deterministic and reproducible in this repo's own harness — src/__tests__/RecyclerViewManager.test.ts already constructs new RecyclerViewManager(props) directly. The shape:

  1. new RecyclerViewManager(props) with 100 items (estimatedItemSize: 20, drawDistance: 0), then updateLayoutParams({ width: 400, height: 500 }, 0).
  2. Settle it: repeatedly call modifyChildrenLayout(measurementsForCurrentStack(20), 100) until progressive render completes, set isFirstPaintOnUiComplete = true, then feed a few rounds of measurements at height 200. Items measuring much taller than estimated shrink the engaged range, so the render stack now holds keys outside it — assert max(stackIndices) > getEngagedIndices().endIndex.
  3. Call modifyChildrenLayout([], 10)without updating props. This mirrors the measurement useLayoutEffect in RecyclerView.tsx, which passes its own render-closure data?.length ?? 0.
  4. Every retained stack index at or above 10 now returns undefined from tryGetLayout (observed [10 … 25]), and modifyChildrenLayout returns false, so nothing schedules a re-render to repair it. The render path reads those same indices through getLayout(index) (ViewHolderCollection.tsx:195), which throws index out of bounds, not enough layouts.
Failing test (drop into src/__tests__/RecyclerViewManager.invariant.test.ts, run with yarn test RecyclerViewManager)
import { FlashListProps } from "../FlashListProps";
import { RecyclerViewManager } from "../recyclerview/RecyclerViewManager";

type Item = { id: number };

const makeProps = (length: number): FlashListProps<Item> =>
  ({
    data: Array.from({ length }, (_, id) => ({ id })),
    renderItem: () => null,
    keyExtractor: (item: Item) => String(item.id),
    drawDistance: 0,
    estimatedItemSize: 20,
  }) as FlashListProps<Item>;

describe("RecyclerViewManager: render stack vs layout table after modifyChildrenLayout", () => {
  const stackIndices = (manager: RecyclerViewManager<Item>) =>
    Array.from(manager.getRenderStack().values(), ({ index }) => index).sort(
      (a, b) => a - b
    );

  const measurementsForStack = (
    manager: RecyclerViewManager<Item>,
    height: number
  ) =>
    stackIndices(manager).map((index) => ({
      index,
      dimensions: { width: 400, height },
    }));

  /** Settles the manager into a state whose render stack holds keys outside the engaged range. */
  const settledManager = () => {
    const manager = new RecyclerViewManager<Item>(makeProps(100));
    manager.updateLayoutParams({ width: 400, height: 500 }, 0);
    for (let i = 0; i < 12; i++) {
      manager.modifyChildrenLayout(measurementsForStack(manager, 20), 100);
    }
    manager.isFirstPaintOnUiComplete = true;
    for (let i = 0; i < 4; i++) {
      manager.modifyChildrenLayout(measurementsForStack(manager, 200), 100);
    }
    return manager;
  };

  it("leaves out-of-bounds entries when the passed dataLength is smaller than props.data.length", () => {
    const manager = settledManager();

    // Precondition: the render stack holds keys beyond the engaged range.
    expect(Math.max(...stackIndices(manager))).toBeGreaterThan(
      manager.getEngagedIndices().endIndex
    );

    // RecyclerView's measurement useLayoutEffect calls
    // modifyChildrenLayout(layoutInfo, data?.length ?? 0) with its own `data`.
    // Here that argument (10) is smaller than props.data.length (100).
    const needsRerender = manager.modifyChildrenLayout([], 10);

    const outOfBounds = stackIndices(manager).filter(
      (index) => manager.tryGetLayout(index) === undefined
    );

    // The layout table was truncated with the ARGUMENT, but the render stack is only ever
    // pruned with props.data.length (updateRenderStack -> RenderStackManager.sync(...,
    // this.getDataLength())), so entries survive that no longer have a layout. Renders read
    // every entry through getLayout(index) and throw.
    expect({ needsRerender, outOfBounds }).toEqual({
      needsRerender: false,
      outOfBounds: [],
    });
  });

  it("prunes correctly when props and the passed dataLength agree", () => {
    const manager = settledManager();
    manager.updateProps(makeProps(10));
    manager.modifyChildrenLayout([], 10);

    expect(
      stackIndices(manager).filter(
        (index) => manager.tryGetLayout(index) === undefined
      )
    ).toEqual([]);
  });
});

First test fails on main; second passes.

Platform

  • iOS
  • Android
  • Web

Environment

React Native info output:
Paste output here

FlashList version: 2.3.0 — the same code paths are present on current main at the permalinks above.

Additional context

Roughly 116 users and 118 events over about 2.5 months in production, all on the same chat/report screen, and no user has a reliable reproduction — consistent with it requiring the engaged range to recompute unchanged while the render stack holds keys outside it.

Workaround: we carry the tryGetLayout guard above as a package patch. It removes the crash, but it doesn't repair the render-stack / layout-table disagreement, so we'd be glad to see the guard land upstream and would be happy to open the PR.

Related: #2291 (same error message, validateItemSize call site) — we patch that one locally too.

Checklist

  • I've searched existing issues and couldn't find a duplicate
  • I've provided a minimal reproduction (Expo Snack preferred)
  • I'm using the latest version of @shopify/flash-list
  • I've included all required information above

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions