Skip to content

Fix SectionList crash when a section keyExtractor is combined with onViewableItemsChanged - #57939

Open
giaBaoJS wants to merge 1 commit into
react:mainfrom
giaBaoJS:fix-46588-section-viewable-key-extractor
Open

Fix SectionList crash when a section keyExtractor is combined with onViewableItemsChanged#57939
giaBaoJS wants to merge 1 commit into
react:mainfrom
giaBaoJS:fix-46588-section-viewable-key-extractor

Conversation

@giaBaoJS

Copy link
Copy Markdown

Summary:

Fixes #46588.

A SectionList whose sections define their own keyExtractor renders fine. Add onViewableItemsChanged and it crashes inside the section's keyExtractor, because the extractor is handed the section object instead of an item:

TypeError: Cannot read properties of undefined (reading 'id')
    at VirtualizedSectionList.keyExtractorWithNullableIndex [as _convertViewable] (packages/virtualized-lists/Lists/VirtualizedSectionList.js:329)
    at ViewabilityHelper.onViewableItemsChanged
    at VirtualizedList._updateViewableItems

Why it is gated on onViewableItemsChanged

A section list flattens to header, ...items, footer rows per section. VirtualizedSectionList._getItem deliberately returns the section itself for the header and footer rows ("When returning a header or footer item the section itself is the item"), and _subExtractor reports index: null for them.

The two key paths then disagree:

  • _keyExtractor (the normal path, always active) returns info.key. For a header/footer that is key + ':header' / key + ':footer', computed from the section, so no key extractor is ever invoked with a section.
  • _convertViewable (only reachable when onViewableItemsChanged is set, via _onViewableItemsChanged) instead called info.section.keyExtractor(viewable.item, info.index) for every viewable row, including headers and footers. So the section object {title, data, keyExtractor} was passed to a key extractor written for items, and any property access on the expected item shape threw.

That asymmetry is exactly why the crash only appears once onViewableItemsChanged is added.

The change

In _convertViewable, branch on info.index == null (the header/footer case) and reuse the key _subExtractor already derived from the section, instead of invoking a key extractor. Item rows keep going through the section's keyExtractor (or the list-wide keyExtractor/defaultKeyExtractor) exactly as before, and header/footer view tokens are still reported to onViewableItemsChanged — just with a safe key — so viewability reporting is not narrowed.

With the index-null case handled up front, info.index is a number on the remaining path, so the info.index ?? 0 fallback and the WithNullableIndex / WithNonNullableIndex naming are no longer meaningful and were dropped.

Changelog:

[GENERAL] [FIXED] - Fix SectionList crash when a section defines its own keyExtractor and onViewableItemsChanged is set

Test Plan:

Added a regression test to the existing packages/virtualized-lists/Lists/__tests__/VirtualizedSectionList-test.js. It renders a two-section list with a per-section keyExtractor that reads a nested item field, scrolls, and asserts three things:

  1. the section's keyExtractor is only ever given items from section.data, never a section;
  2. header and footer rows are still reported as viewable, keyed 0:header / 0:footer / 1:header;
  3. item rows are still keyed by the section's keyExtractor.

Assertion 2 is there on purpose: silently dropping header/footer view tokens would also make the crash go away, and would be a regression.

Counterfactual — with the test in place and only the VirtualizedSectionList.js change reverted:

● VirtualizedSectionList › onViewableItemsChanged › reports section headers and footers
  without running them through the section keyExtractor

  TypeError: Cannot read properties of undefined (reading 'id')

    at VirtualizedSectionList.keyExtractorWithNullableIndex [as _convertViewable] (packages/virtualized-lists/Lists/VirtualizedSectionList.js:329:11)
    at map (packages/virtualized-lists/Lists/VirtualizedSectionList.js:352:12)
    at ViewabilityHelper.onViewableItemsChanged [as _onUpdateSync] (packages/virtualized-lists/Lists/ViewabilityHelper.js:308:7)
    at VirtualizedList._updateViewableItems [as _onScroll] (packages/virtualized-lists/Lists/VirtualizedList.js:1767:10)

Tests: 1 failed, 11 skipped, 12 total

Restoring the fix makes it pass.

User-visible path. Before writing the unit test I reproduced the same crash through the public SectionList component (packages/react-native/Libraries/Lists/SectionList.js) with a scratch test, to confirm the reported symptom rather than only the internal helper. It failed with the identical stack on main, and after the fix reported:

[{"key":"0:header","index":null},{"key":"i1.1","index":0},{"key":"i1.2","index":1},
 {"key":"0:footer","index":null},{"key":"1:header","index":null},{"key":"i2.1","index":0}]

i.e. no crash, headers/footers still reported, items still keyed by the section's keyExtractor. That scratch test was removed; the committed test covers the same behaviour at the VirtualizedSectionList level, where the rest of the section-list tests live.

Commands run:

yarn jest packages/virtualized-lists/
  Test Suites: 8 passed, 8 total
  Tests:       1 skipped, 170 passed, 171 total
  Snapshots:   69 passed, 69 total

yarn test
  Test Suites: 218 passed, 218 total
  Tests:       1 skipped, 5586 passed, 5587 total
  Snapshots:   1666 passed, 1666 total

yarn lint                     -> clean (eslint --max-warnings 0)
yarn flow-check               -> Found 0 errors
yarn test-typescript-legacy   -> clean

No RNTester change: this restores existing behaviour of the current SectionList / onViewableItemsChanged examples rather than adding a user-facing capability, so there is no new surface to demonstrate. Happy to add a focused example if reviewers would like a manual repro in RNTester.

…actor

VirtualizedSectionList._convertViewable ran every viewable row through the
section's keyExtractor. For section header and footer rows _getItem returns
the section object itself, so a keyExtractor written for items received
{title, data, keyExtractor} and crashed. Use the key _subExtractor already
derived for those rows instead, matching what _keyExtractor does on the
non-viewability path.
@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 Aug 13, 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 Aug 13, 2026
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.

Adding onViewableItemsChanged to SectionList changes the items passed to each section's keyExtractor

1 participant