Repository navigation
Conversation
FlashList can ask for the key of an index past the end of the data when the list shrinks while it is hidden by <Activity>. The keyExtractor used the 'in' operator on that undefined item and crashed. Fall back to the index instead, like BaseSelectionList already does, and add regression tests. Co-authored-by: thelullabyy <[email protected]>
Contributor
Author
|
I created this PR from an upstream branch because I don't have push access to your fork. To take over this branch and push updates yourself, run: Then you can close this PR and open a new one from your fork. |
Contributor
Author
Contributor
Author
|
🤖 Web test steps verification results: ✅ pass On dev NewDot web, the participants list in the create-expense flow stayed stable when search shrank and regrew it, and through repeated back-and-forward navigation. No crash or error screen appeared. The web test tool can't read the browser console, so the console check rests on the missing crash and error UI.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.









Explanation of Change
BaseSelectionListWithSectionspassed FlashList akeyExtractorthat ran'flatListKey' in itemwith no guard. FlashList can ask for the key of an index past the end of the data. When that happens,itemisundefinedand theinoperator throwsTypeError: right operand of 'in' is not an object, which crashes the participants step (Sentry APP-JQB).This happens when the list's data shrinks while
<Activity>hides it, as inMoneyRequestParticipantsSelector. Hiding the list runs FlashList's effect cleanup, which disposes itsRecyclerViewManager. A disposed manager doesn't recompute its engaged indices. So when the hidden list re-renders with shorter data,processDataUpdatesyncs the render stack with the old indices and callsgetDataKeyfor indices that no longer exist.This PR moves the
keyExtractorinto a named function. Whenitemis missing, the function falls back toindex.toString().BaseSelectionListalready does the same thing, andgetItemTypein this file already usesitem?.type. FlashList replaces the stale slots once the list is shown again.I added
tests/unit/BaseSelectionListWithSectionsActivityTest.tsx, which renders the real FlashList (not a mock) inside<Activity>:processDataUpdate→getDataKey→keyExtractorerror as Sentry.keyExtractor(undefined, 5)returns'5'. Without the fix, this case throws.Fixed Issues
$ #103776
PROPOSAL: #103776 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
AI Tests
Melvin ran these locally:
npm testonBaseSelectionListWithSectionsActivityTest.tsxandBaseSelectionListSectionsTest.tsx: 16/16 passed. With the source fix reverted, the new file fails 2 of 3 cases withTypeError: Cannot use 'in' operator to search for 'flatListKey' in undefined.npm run typecheck: passed.npm run linton the changed files: clean.npm run react-compiler-compliance-check checkonBaseSelectionListWithSections.tsx: no errors.npm run spell-changed: 0 issues.npm run fmt(oxfmt): no changes.Not run locally: the full Jest suite and the Storybook smoke test. CI covers them.