fix: Expense-Keep this one option doesn't resolve duplicates in trackmap expense #79871
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
melvin-bot
bot
requested review from
joekaufmanexpensify and
shubham1206agra
and removed request for
a team
|
@shubham1206agra 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] |
❌ PERF-11 (docs)File: src/hooks/useTransactionsByID.ts:7 This refactoring causes the component to subscribe to ALL transactions in the collection, which can lead to unnecessary re-renders whenever ANY transaction changes, not just the specific transactions needed by Problem: The old selector pattern only caused re-renders when the specific transactions in Suggested fix: Keep the selector pattern for optimal performance: import {useCallback} from 'react';
import type {OnyxCollection} from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Transaction} from '@src/types/onyx';
import useOnyx from './useOnyx';
function useTransactionsByID(transactionIDs: string[] | undefined) {
const transactionsSelector = useCallback(
(transactions: OnyxCollection<Transaction>) =>
transactionIDs?.map((id) => transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`]).filter(Boolean) as Transaction[] ?? [],
[transactionIDs],
);
const [transactions] = useOnyx(
ONYXKEYS.COLLECTION.TRANSACTION,
{
selector: transactionsSelector,
canBeMissing: true,
},
[transactionsSelector],
);
return [transactions];
}
export default useTransactionsByID;This ensures that the component only re-renders when the specific transactions in |
Code Review FindingsPERF-11 Violation in
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 341ff67a0c
ℹ️ 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".
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Fix makes sense
src/hooks/useTransactionsByID.ts
Outdated
| (transactions: OnyxCollection<Transaction>) => transactionIDs?.map((id) => transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`]) ?? [], | ||
| [transactionIDs], | ||
| ); | ||
| const [allTransactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, { |
There was a problem hiding this comment.
@nkdengineer Is there a reason for this change?
There was a problem hiding this comment.
@shubham1206agra When I test with my proposal, we have another case when refreshing on the confirmation page, the duplicates is also an empty array.
There was a problem hiding this comment.
After install lib again and reverting this change, this case works well now.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2026-01-30.at.5.05.12.PM.moviOS: HybridAppScreen.Recording.2026-01-30.at.5.15.36.PM.moviOS: mWeb SafariScreen.Recording.2026-01-30.at.4.34.52.PM.movMacOS: Chrome / SafariScreen.Recording.2026-01-30.at.4.32.00.PM.mov |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/amyevans in version: 9.3.11-16 🚀
|
5 participants
Explanation of Change
fix: Expense-Keep this one option doesn't resolve duplicates in trackmap expense
Fixed Issues
$ #79179
PROPOSAL: #79179 (comment)
Tests
Offline tests
None
QA Steps
Same as test
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
Screen.Recording.2026-01-19.at.15.19.33.mov
Android: mWeb Chrome
Screen.Recording.2026-01-19.at.15.23.15.mov
iOS: Native
Screen.Recording.2026-01-19.at.15.19.06.mov
iOS: mWeb Safari
Screen.Recording.2026-01-19.at.15.18.10.mov
MacOS: Chrome / Safari
Screen.Recording.2026-01-19.at.15.16.40.mov