Repository navigation
Conversation
e14c097 to
943ffd8
Compare
|
🚧 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! 🧪🧪
|
|
@MelvinBot review |
Review: LGTM ✅Clean, well-justified perf change. Swapping the full-collection Correctness — the returned set is identical. The derived value groups transactions by Reference stability — preserved. React Compiler is enabled repo-wide ( Minor / optional
Testing note: I ran a smoke test of Search → Spend on web, but the test account had no seeded expenses/reports, so the row-rendering and Pay-button paths couldn't be exercised live. Navigation was clean with no console errors. Functional equivalence is otherwise well covered by the passing The only failing check is |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2026-08-06.at.21.55.41.mov |
|
@mountiny looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Not emergency, seems like the action was still running though the button was green |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.52-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR against the help site content under Why: This is a purely internal performance optimization with no user-facing behavior change. If you believe a specific article is affected by this change, let me know which one and I'll take another look. |
|
Hi @TMisiukiewicz. Can you please share QA steps? |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.52-11 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Found via CPU profiling while investigating blank rows when scrolling the Search Spends tab. No linked issue.
useReportTransactionspreviously connected to the fulltransactionsOnyx collection and did anObject.values(...).filter(...)scan over every transaction in the app to find the ones belonging to a single report, for every consumer of the hook. This change makes it reuseuseReportTransactionsCollection, which reads from thereportTransactionsAndViolationsderived value already indexed byreportID, so lookups are O(1) per report instead of a full linear scan of the collection.The set of transactions returned is identical — the derived value groups transactions by
transaction.reportID. React Compiler memoizes theObject.values().filter(), so the returned array stays reference-stable.Profiling evidence
Chrome DevTools trace of scrolling the Spends tab (10.4s total, 6.56s idle). Top self-time frames:
getTransactionsForReport(pre-existing, separate)selector←BaseKYCWall→useReportTransactionsisTransactionEntrygetViolationsFromSearchDataCall stack:
BaseKYCWall → useReportTransactions → useOnyx → updateSyncExternalStore → getSnapshot → selector.Every Spends row renders a KYC wall via
PayActionCell → SettlementButton → KYCWall, so each row scrolling into view triggered a full scan of thetransactionscollection. That selector accounted for ~8% of all busy JS in the trace.Call sites that benefit
Fixed in the shared hook, so all three consumers improve:
src/components/KYCWall/BaseKYCWall.tsxsrc/components/ReportActionItem/MoneyReportView.tsxsrc/components/ReportActionItem/MoneyRequestView.tsxFixed Issues
$ #97977
PROPOSAL:
Tests
MoneyReportView) and verify the transaction list and totals are unchanged.MoneyRequestView) and verify the fields render and the report's transactions resolve as before.Offline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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