Skip to content

[No QA] Perf: lazily compute B2B invoice report lookup on payment instead of every render #85813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TMisiukiewicz wants to merge 2 commits into
base: main
Choose a base branch
from
+26 −16
Open

[No QA] Perf: lazily compute B2B invoice report lookup on payment instead of every render #85813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0985122
lazy B2B invoice report lookup in getPayMoneyRequestParams
TMisiukiewicz Mar 19, 2026
88a5424
Merge remote-tracking branch 'origin/main' into tomasz/lazy-b2b-invoi…
TMisiukiewicz Mar 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import useLocalize from '@hooks/useLocalize';
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
import useNetwork from '@hooks/useNetwork';
import useOnyx from '@hooks/useOnyx';
import useParticipantsInvoiceReport from '@hooks/useParticipantsInvoiceReport';
import usePaymentAnimations from '@hooks/usePaymentAnimations';
import usePaymentOptions from '@hooks/usePaymentOptions';
import usePermissions from '@hooks/usePermissions';
Expand Down Expand Up @@ -673,7 +672,6 @@ function MoneyReportHeader({
const isReportSubmitter = isCurrentUserSubmitter(chatIOUReport);
const isChatReportDM = isDM(chatReport);

const existingB2BInvoiceReport = useParticipantsInvoiceReport(activePolicyID, CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS, chatReport?.policyID);
const isSelectionModePaymentRef = useRef(false);
const confirmPayment = useCallback(
({paymentType: type, payAsBusiness, methodID, paymentMethod}: PaymentActionParams) => {
Expand Down Expand Up @@ -706,7 +704,6 @@ function MoneyReportHeader({
currentUserAccountIDParam: accountID,
currentUserEmailParam: email ?? '',
payAsBusiness,
existingB2BInvoiceReport,
methodID,
paymentMethod,
activePolicy,
Expand Down Expand Up @@ -768,7 +765,6 @@ function MoneyReportHeader({
introSelected,
accountID,
email,
existingB2BInvoiceReport,
activePolicy,
policy,
currentSearchQueryJSON,
Expand Down
5 changes: 0 additions & 5 deletions ...omponents/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useOnyx from '@hooks/useOnyx';
import useParticipantsInvoiceReport from '@hooks/useParticipantsInvoiceReport';
import usePaymentAnimations from '@hooks/usePaymentAnimations';
import usePermissions from '@hooks/usePermissions';
import usePolicy from '@hooks/usePolicy';
Expand Down Expand Up @@ -254,8 +253,6 @@ function MoneyRequestReportPreviewContent({

const hasReceipts = transactionsWithReceipts.length > 0;
const isScanning = hasReceipts && areAllRequestsBeingSmartScanned;
const existingB2BInvoiceReport = useParticipantsInvoiceReport(activePolicyID, CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS, chatReport?.policyID);

const {isDelegateAccessRestricted} = useDelegateNoAccessState();
const {showDelegateNoAccessModal} = useDelegateNoAccessActions();
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportID}`);
Expand Down Expand Up @@ -290,7 +287,6 @@ function MoneyRequestReportPreviewContent({
currentUserAccountIDParam: currentUserAccountID,
currentUserEmailParam: currentUserEmail,
payAsBusiness,
existingB2BInvoiceReport,
methodID,
paymentMethod,
activePolicy,
Expand Down Expand Up @@ -326,7 +322,6 @@ function MoneyRequestReportPreviewContent({
introSelected,
currentUserAccountID,
currentUserEmail,
existingB2BInvoiceReport,
activePolicy,
policy,
betas,
Expand Down
33 changes: 26 additions & 7 deletions src/libs/actions/IOU/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@
hasNonReimbursableTransactions as hasNonReimbursableTransactionsReportUtils,
hasOutstandingChildRequest,
hasViolations as hasViolationsReportUtils,
isArchivedNonExpenseReport,
isArchivedReport,
isClosedReport as isClosedReportUtil,
isDeprecatedGroupDM,
Expand Down Expand Up @@ -326,7 +327,6 @@
currentUserAccountIDParam: number;
currentUserEmailParam: string;
payAsBusiness?: boolean;
existingB2BInvoiceReport?: OnyxEntry<OnyxTypes.Report>;
methodID?: number;
paymentMethod?: PaymentMethod;
activePolicy?: OnyxTypes.Policy;
Expand Down Expand Up @@ -771,7 +771,7 @@
};

let allPersonalDetails: OnyxTypes.PersonalDetailsList = {};
Onyx.connect({

Check warning on line 774 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
allPersonalDetails = value ?? {};
Expand Down Expand Up @@ -900,7 +900,7 @@
};

let allTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 903 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -914,7 +914,7 @@
});

let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({

Check warning on line 917 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -923,7 +923,7 @@
});

let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {};
Onyx.connect({

Check warning on line 926 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -937,7 +937,7 @@
});

let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {};
Onyx.connect({

Check warning on line 940 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY_TAGS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -950,7 +950,7 @@
});

let allReports: OnyxCollection<OnyxTypes.Report>;
Onyx.connect({

Check warning on line 953 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -959,7 +959,7 @@
});

let allReportNameValuePairs: OnyxCollection<OnyxTypes.ReportNameValuePairs>;
Onyx.connect({

Check warning on line 962 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -969,7 +969,7 @@

let userAccountID = -1;
let currentUserEmail = '';
Onyx.connect({

Check warning on line 972 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserEmail = value?.email ?? '';
Expand All @@ -978,7 +978,7 @@
});

let deprecatedCurrentUserPersonalDetails: OnyxEntry<OnyxTypes.PersonalDetails>;
Onyx.connect({

Check warning on line 981 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
deprecatedCurrentUserPersonalDetails = value?.[userAccountID] ?? undefined;
Expand All @@ -986,7 +986,7 @@
});

let allReportActions: OnyxCollection<OnyxTypes.ReportActions>;
Onyx.connect({

Check warning on line 989 in src/libs/actions/IOU/index.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand Down Expand Up @@ -1026,6 +1026,25 @@
return allReports;
}

function getExistingB2BInvoiceReport(receiverID: string | number | undefined, policyID?: string): OnyxEntry<OnyxTypes.Report> {
return Object.values(allReports ?? {}).find((report) => {
if (!report || !isInvoiceRoom(report)) {
return false;
}
const rnvp = allReportNameValuePairs?.[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report.reportID}`];
Copy link
Contributor

@rlinoz rlinoz Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid that refactors like this goes in the opposite direction of removing Onyx.connect.

Do we have some other alternative?

cc: @tgolen

Copy link
Contributor Author

@TMisiukiewicz TMisiukiewicz Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative is a solution that I just opened a draft PR for: #85949. It isolates logic and Onyx calls, giving similar results (unless user has tens of rendered Pay buttons). Should we move forward with decomposition PR only or ship both just in case to avoid the perf edge cases?

Copy link
Contributor

@rlinoz rlinoz Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that is a nice PR!

Do you mean the changes here are temporary or do you plan to use this in the decomposition PR as well?

Copy link
Contributor Author

@TMisiukiewicz TMisiukiewicz Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah If we decide to ship this PR as-is, I'll also include this change in #85949. This PR removes the optimistic computation of a value that is only needed when Pay button is pressed - without it, we'd still compute it for every Pay button even if user does not plan to press it

Copy link
Contributor

@rlinoz rlinoz Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I will wait for Tim's thoughts, but I think once we get to allReportNameValuePairs and allReports in the Onyx.connect issues we will end up reverting back to the previous way without realizing it.

Copy link
Contributor

@tgolen tgolen Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to avoid shipping this PR as-is since it will just create more work that needs to be refactored later.

if (isArchivedNonExpenseReport(report, isArchivedReport(rnvp))) {
return false;
}
const isSameReceiver =
report.invoiceReceiver &&
report.invoiceReceiver.type === CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS &&
'policyID' in report.invoiceReceiver &&
report.invoiceReceiver.policyID === receiverID;

return report.policyID === policyID && isSameReceiver;
});
}

function getAllReportActionsFromIOU(): OnyxCollection<OnyxTypes.ReportActions> {
return allReportActions;
}
Expand Down Expand Up @@ -9296,7 +9315,6 @@
introSelected,
paymentPolicyID,
lastUsedPaymentMethod,
existingB2BInvoiceReport,
activePolicy,
iouReportCurrentNextStepDeprecated,
betas,
Expand All @@ -9312,7 +9330,6 @@
bankAccountID?: number;
paymentPolicyID?: string | undefined;
lastUsedPaymentMethod?: OnyxTypes.LastPaymentMethodType;
existingB2BInvoiceReport?: OnyxEntry<OnyxTypes.Report>;
activePolicy?: OnyxEntry<OnyxTypes.Policy>;
currentUserAccountIDParam: number;
currentUserEmailParam?: string;
Expand Down Expand Up @@ -9379,8 +9396,12 @@
onyxData.failureData?.push(...(policyFailureData ?? []), {onyxMethod: Onyx.METHOD.MERGE, key: ONYXKEYS.NVP_ACTIVE_POLICY_ID, value: activePolicy?.id ?? null});
}

if (isIndividualInvoiceRoom(chatReport) && payAsBusiness && existingB2BInvoiceReport) {
chatReport = existingB2BInvoiceReport;
if (isIndividualInvoiceRoom(chatReport) && payAsBusiness) {
const existingB2B = getExistingB2BInvoiceReport(activePolicy?.id, chatReport.policyID);

if (existingB2B) {
chatReport = existingB2B;
}
}

let total = (iouReport?.total ?? 0) - (iouReport?.nonReimbursableTotal ?? 0);
Expand Down Expand Up @@ -11348,7 +11369,6 @@
currentUserAccountIDParam,
currentUserEmailParam,
payAsBusiness = false,
existingB2BInvoiceReport,
methodID,
paymentMethod,
activePolicy,
Expand Down Expand Up @@ -11380,7 +11400,6 @@
full: true,
payAsBusiness,
bankAccountID: methodID,
existingB2BInvoiceReport,
activePolicy,
currentUserAccountIDParam,
currentUserEmailParam,
Expand Down
Loading