perf: Migrate to Flashlist in MoneyRequestReportTransactionList#91422
perf: Migrate to Flashlist in MoneyRequestReportTransactionList#91422TMisiukiewicz wants to merge 57 commits into
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@aimane-chnaif Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5c9bef51
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
@aimane-chnaif kind bump |
…asts The unified list ref was bridged from the shared ActionList context's FlatList-typed slot to the FlashList via `as unknown as` casts, tripping the no-unsafe-type-assertion seatbelt (3 > 1 allowed). Mirror the InvertedFlashList pattern: type the ref prop as FlatListRefType and forward it through @components/FlashList as a spread prop, so no cast is needed at the call site. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The type's only external consumers were the ref-cast imports in MoneyRequestReportActionsList and MoneyRequestReportTransactionList, both removed in the previous commit. It is still used internally, so keep the type and drop only the export. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
When we have task report action in the middle, there's issue. And the content is not rendered immediately. production.mp4this.branch.mp4 |
|
Updating the PR now and I'll look into this issue. |
…-list # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx # src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx # src/components/TransactionItemRow/index.tsx
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This looks broken in both scenarios but I cannot reproduce this case locally yet. It's potentially recycling working on |
|
Please create 40+ expenses (easy to create using duplicate expense feature) to build long list. |
|
Merged latest main and couldn't reproduce. @aimane-chnaif could you check again? Maybe it just resolved after resolving conflicts Simulator.Screen.Recording.-.iPhone.17.Pro.-.2026-06-16.at.12.28.48.mov |
|
@aimane-chnaif please let us know in Slack |
|
Can you please run new build? |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@aimane-chnaif Making freshest build |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@aimane-chnaif can you prioritize this please? @TMisiukiewicz @adhorodyski conflicts |
…rt-list # Conflicts: # src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx
|
@mountiny resolved conflicts |
|
job 2 failing but seems not related |
|
Hopefully no conflict but let's wait until #93403 is deployed and then retest since it refactored FlashList scrolling behavior. |
…rt-list # Conflicts: # src/CONST/index.ts
|
updated with latest main, @mountiny can you run a fresh adhoc? |
Explanation of Change
Problem
On heavy money-request reports the RHP list was effectively non-virtualised. The inner transactions section was rendered via plain
.map()inside the parent FlatList'sListHeaderComponent, which renders eagerly. Every transaction row mounted synchronously when the report opened, pushing report open time to ~20–30s for a report with 1600 expenses and producing a noticeable jank window on every navigation.Solution
Unified list — merged the inner transactions section and the outer report-actions section into a single virtualised list. A discriminated-union
UnifiedListItem(section-header|transaction|transactions-footer|report-action) flows through onerenderItemdispatcher, so transactions and chat messages now share the same recycler window instead of one being eagerly rendered inside the other's header.FlashList — migrated the list from
react-native'sFlatList(FlatListWithScrollKey).initialScrollIndexreplaces the FlatList-specificuseFlatListScrollKeywrapperController + sub-component —
MoneyRequestReportTransactionListcontinues to own the transaction-domain state (sort, group-by, selection, columns, totals, etc.) and exposes it to the parent via a render-prop controller. The parent renders a smallMoneyRequestReportUnifiedListsub-component that assembles the FlashList from controller data plus the report-actions listFixed Issues
$ #91425
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
web.mov