perf: feed stable report projection to transaction list to avoid read-state re-renders#92485
Conversation
…-state re-renders
|
Do we need another C+ on this PR? |
|
I'll be reviewing as part of #91422 |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
There was a problem hiding this comment.
Pull request overview
This PR reduces unnecessary re-renders in the money request report view by passing a stable, read-heartbeat-free projection of the Report object into the transaction list subtree, preventing chat read-state churn (e.g. lastReadTime, lastMessageText) from re-rendering the transaction list.
Changes:
- Exported the
StableReporttype fromsrc/selectors/Report.tsso it can be consumed externally. - Updated
MoneyRequestReportTransactionListto accept aStableReportinstead of the fullReport. - Updated
MoneyRequestReportActionsListto passreportStableintoMoneyRequestReportTransactionList(with an explicit truthy guard).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/selectors/Report.ts | Exports StableReport type to support typed consumption of the stable report projection. |
| src/components/MoneyRequestReportView/MoneyRequestReportTransactionList.tsx | Narrows report prop type to StableReport to avoid re-renders caused by read-state heartbeat fields. |
| src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx | Passes reportStable into the transaction list (and keeps conditional rendering behavior via !!reportStable). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
|
job 4 failing |
|
looks like a timeout, not related to changes |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
| /> | ||
| {!!reportStable && ( | ||
| <MoneyRequestReportTransactionList | ||
| report={reportStable} |
There was a problem hiding this comment.
@MelvinBot find out all occurrences in MoneyRequestReportTransactionList and its child components where report properties that don't exist in reportStable are referenced.
Review — looks correct and well-scoped ✅The change swaps the full What I verified (the omitted fields are never read downstream)Omitted by the projection:
Worth confirming before merge (perf, not correctness)The parent still self-subscribes to the full Minor (non-blocking)The new 🤖 Generated with Claude Code |
|
Answer: zero. Inside The omitted fields ( The one occurrence — and why it's safe
So even though it reads omitted fields, it never touches the projection. No regression. How I verified it's exhaustive (grep methodology)Every component reachable from
Grepping both directories for all 9 field names returns only:
No other file in the subtree references any omitted field. One non-component exit point worth noting: 🤖 Generated with Claude Code |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.3.99-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 Help site review: no changes required. This PR is a pure render-performance optimization with no user-facing behavior change, so nothing in Why no docs changes are neededThe change feeds a stable report projection (
As stated in the PR description: "Behavior is identical to production; only the re-render trigger surface shrinks." Help articles document user-facing features, flows, and settings — none of which change here. There are no new buttons, tabs, labels, settings, or workflows to document. No draft help site PR was created since no documentation updates apply. |
|
Hi @TMisiukiewicz. We have more than 30 expenses in the report, but the "Latest Messages" section does not appear on Web. Recording.805.mp4 |
Send any message in the report and try again |
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.3.99-9 🚀
|
Explanation of Change
getStableReportSelector/reportStablealready exist inMoneyRequestReportActionsListto produce a stable projection of the report object with read-state heartbeat fields (lastReadTime,lastVisibleActionCreated,lastMessageText, etc.) omitted. However, the fullreport— which re-renders on every chat heartbeat update — was still being passed down toMoneyRequestReportTransactionList. This meant everylast*field mutation (e.g. marking a report as read, receiving a message) re-rendered the entire transaction-list subtree even though nothing inside it reads those fields.This PR feeds
reportStable(typed asStableReport) toMoneyRequestReportTransactionListinstead.StableReportis structurally assignable toReport(the omitted fields are all optional), so all downstream consumers — row components, utility calls — need no changes. The!!reportStableguard preserves the existing conditional render. The net effect is that read-state churn no longer triggers re-renders of the transaction list. Behavior is identical to production; only the re-render trigger surface shrinks.export type {StableReport}is added tosrc/selectors/Report.tsso the type is importable from the selector module.Fixed Issues
$ #92491
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: mWeb Safari
MacOS: Chrome / Safari