Conversation
…selectors-getters # Conflicts: # src/libs/actions/IOU/index.ts
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
|
||
| // Selector returns only the qualifying policy ID — stable value, prevents re-renders | ||
| const [singleGroupPolicyID] = useOnyx(ONYXKEYS.COLLECTION.POLICY, { | ||
| selector: (policies) => getSingleGroupPolicyID(policies, login), |
There was a problem hiding this comment.
Can we return the policy here instead of the ID, similar to what we did with useReportAttributesByID?
There was a problem hiding this comment.
Policy objects are large, so returning the full object from a collection selector would require Onyx to deep compare it on every collection change, it might defeat the performance a bit
useReportAttributesByID works differently because it selects from a derived value where each entry is a small object, making deep comparison cheap. Here we're selecting from the full COLLECTION.POLICY, so returning only the stable ID and doing a per-key lookup is more efficient
what do you think?
There was a problem hiding this comment.
Deep comparison is cheap (single small object)
selects from a derived value where each entry is a small object
Ah, you're right. I've been thinking how it can be cheap if it's a report object. I forgot that it's a report attributes 🤦. All good!
| // Returns only IDs + flags (stable output) to prevent re-renders when unrelated policies change. | ||
| const policyQualificationSelector = (policies: OnyxCollection<Policy>) => getPolicyQualificationResult(policies, login, isPerDiemRequest, isTimeRequest, expensePolicyID); | ||
| const [qualificationResult] = useOnyx(ONYXKEYS.COLLECTION.POLICY, { | ||
| selector: policyQualificationSelector, |
|
Thanks for the review! I addressed the comments |
|
@bernhardoj 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb.mp4 |
|
No product review needed |
|
@rinej could you please resolve the conflicts? |
Explanation of Change
This is PR 1 of 5 in the IOURequestStepScan decomposition series (see #85571 for the full split plan).
Adds Onyx selectors to derived hooks to prevent full-collection re-renders:
useReportIsArchived: uses isArchivedReport as selector for stable boolean output
useDefaultExpensePolicy: extracts getSingleGroupPolicyID selector, replaces collection iteration with selector + per-key lookup
usePolicyForMovingExpenses: extracts getPolicyQualificationResult selector returning only IDs and flags
useReportAttributes: adds useReportAttributesByID for single-report lookups with cheap deep comparison
Also exports two getter functions from IOU actions (getAllTransactionDrafts, getAllReportNameValuePairs) needed by downstream variant components.
No user-visible behavior changes - return types and values are preserved.
Fixed Issues
$ #85583
PROPOSAL:
Tests
Offline tests
QA Steps
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