Hoist the active policy subscription out of Search rows - #98299
Conversation
|
|
|
🚧 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.
|
@cretadn22 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] |
|
@Eskalifer1 @mountiny One of you needs to 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] |
|
Let me know if you need my help here! If not please unassign me, thank you! |
|
@cretadn22 can you review please |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-08-12.at.18.47.11.movAndroid: mWeb ChromeScreen.Recording.2026-08-12.at.18.43.43.moviOS: HybridAppScreen.Recording.2026-08-12.at.18.46.33.moviOS: mWeb SafariScreen.Recording.2026-08-12.at.18.44.38.movMacOS: Chrome / SafariScreen.Recording.2026-08-12.at.18.37.51.mov |
|
@TMisiukiewicz Could you please take a look at fixing the failing tests? |
|
@cretadn22 updated! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 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! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.53-0 🚀
|
|
🤖 I reviewed the changes in this PR against Expensify's help site content under No help site changes are required. This is a pure internal performance refactor with no user-facing behavior change:
The change is entirely about avoiding redundant Since no documentation changes are needed, no draft help site PR was created. @TMisiukiewicz, if you believe a user-facing behavior did change here that I missed, let me know and I'll draft the corresponding help site update. |
Explanation of Change
usePolicyForMovingExpensessubscribed to the active policy itself, withactivePolicySelector— a selector that returns the whole policy object.useOnyxdeep-compares a selector's output to keep its reference stable, so each instance of the hook deep-compares an entire workspace (employeeList,customUnits,rules) on every Onyx merge touching its key.On Search that is multiplied twice over:
useOnyxre-pointspolicy_*at thesnapshot_<hash>key, so every snapshot merge notifies it, and each visible transaction row reaches the hook ~4× (TransactionListItemWide,TransactionListItemInner,TransactionItemRowWide, andusePolicyForTransaction←useTransactionInlineEdit). Profiled in dev, each compare cost 3.2–6.6 ms and every one concluded "equal" — pure waste, on the JS thread insidekeyChanged.This PR moves that subscription into
ActivePolicyProvider, mounted inOnyxListItemProvider(whose stated purpose is shared subscriptions for list items).usePolicyForMovingExpensesnow readsuseActivePolicy().Performance improvement: On a large customer account with a policy containing 11k members,
deepEqualchecks on reports from that policy previously took 100–300ms. This has been reduced to ~30ms.Fixed Issues
$ #98311
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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