Skip to content

perf: optimize ManualOpenCreateExpense metric with policy selector and reduced subscriptions #83803

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

Draft
OlimpiaZurek wants to merge 1 commit into
base: main
Choose a base branch
from
+54 −16
Draft

perf: optimize ManualOpenCreateExpense metric with policy selector and reduced subscriptions #83803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/libs/actions/IOU/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@

let allPersonalDetails: OnyxTypes.PersonalDetailsList = {};
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,

Check warning on line 812 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
callback: (value) => {
allPersonalDetails = value ?? {};
},
Expand Down Expand Up @@ -902,7 +902,7 @@

let allTransactions: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION,

Check warning on line 905 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
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
Expand All @@ -916,7 +916,7 @@

let allTransactionDrafts: NonNullable<OnyxCollection<OnyxTypes.Transaction>> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION_DRAFT,

Check warning on line 919 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
waitForCollectionCallback: true,
callback: (value) => {
allTransactionDrafts = value ?? {};
Expand All @@ -925,7 +925,7 @@

let allTransactionViolations: NonNullable<OnyxCollection<OnyxTypes.TransactionViolations>> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,

Check warning on line 928 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
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
Expand All @@ -939,7 +939,7 @@

let allPolicyTags: OnyxCollection<OnyxTypes.PolicyTagLists> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY_TAGS,

Check warning on line 942 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
waitForCollectionCallback: true,
callback: (value) => {
if (!value) {
Expand All @@ -952,7 +952,7 @@

let allReports: OnyxCollection<OnyxTypes.Report>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,

Check warning on line 955 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
waitForCollectionCallback: true,
callback: (value) => {
allReports = value;
Expand All @@ -961,7 +961,7 @@

let allReportNameValuePairs: OnyxCollection<OnyxTypes.ReportNameValuePairs>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS,

Check warning on line 964 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
waitForCollectionCallback: true,
callback: (value) => {
allReportNameValuePairs = value;
Expand All @@ -971,7 +971,7 @@
let userAccountID = -1;
let currentUserEmail = '';
Onyx.connect({
key: ONYXKEYS.SESSION,

Check warning on line 974 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
callback: (value) => {
currentUserEmail = value?.email ?? '';
userAccountID = value?.accountID ?? CONST.DEFAULT_NUMBER_ID;
Expand All @@ -980,7 +980,7 @@

let deprecatedCurrentUserPersonalDetails: OnyxEntry<OnyxTypes.PersonalDetails>;
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,

Check warning on line 983 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
callback: (value) => {
deprecatedCurrentUserPersonalDetails = value?.[userAccountID] ?? undefined;
},
Expand All @@ -988,7 +988,7 @@

let allReportActions: OnyxCollection<OnyxTypes.ReportActions>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,

Check warning on line 991 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
waitForCollectionCallback: true,
callback: (actions) => {
if (!actions) {
Expand Down Expand Up @@ -1219,7 +1219,7 @@
newIouRequestType,
report,
parentReport,
currentDate = '',
currentDate,
lastSelectedDistanceRates,
currentUserPersonalDetails,
hasOnlyPersonalPolicies,
Expand Down
63 changes: 51 additions & 12 deletions src/pages/iou/request/IOURequestStartPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {useFocusEffect} from '@react-navigation/native';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {Keyboard, View} from 'react-native';
import type {OnyxCollection} from 'react-native-onyx';
import DragAndDropProvider from '@components/DragAndDrop/Provider';
import FocusTrapContainerElement from '@components/FocusTrap/FocusTrapContainerElement';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
Expand Down Expand Up @@ -38,10 +39,11 @@ import {endSpan} from '@libs/telemetry/activeSpans';
import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
import type {IOURequestType} from '@userActions/IOU';
import {initMoneyRequest} from '@userActions/IOU';
import Tab from '@userActions/Tab';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type SCREENS from '@src/SCREENS';
import type {SelectedTabRequest} from '@src/types/onyx';
import type {Policy, SelectedTabRequest} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import IOURequestStepAmount from './step/IOURequestStepAmount';
Expand All @@ -60,6 +62,45 @@ type IOURequestStartPageProps = WithWritableReportOrNotFoundProps<typeof SCREENS
const platform = getPlatform(true);
const isWeb = ([CONST.PLATFORM.WEB, CONST.PLATFORM.MOBILE_WEB] as Platform[]).includes(platform);

// Tab indices for IOURequestStartPage
const PER_DIEM_TAB_INDEX = 2;

// Selector to optimize policy collection subscriptions - only fetch needed fields
const policySelector = (policies: OnyxCollection<Policy>): OnyxCollection<Policy> => {
if (!policies) {
return {};
}

const result: Record<string, Policy> = {};

for (const [id, policyItem] of Object.entries(policies)) {
if (!policyItem) {
continue;
}

result[id] = {
id: policyItem.id,
type: policyItem.type,
name: policyItem.name,
pendingAction: policyItem.pendingAction,
isPolicyExpenseChatEnabled: policyItem.isPolicyExpenseChatEnabled,
role: policyItem.role,
chatReportIDAdmins: policyItem.chatReportIDAdmins,
employeeList: policyItem.employeeList,
arePerDiemRatesEnabled: policyItem.arePerDiemRatesEnabled,
customUnits: policyItem.customUnits,
units: policyItem.units,
// Additional fields required by policy utilities
isJoinRequestPending: policyItem.isJoinRequestPending,
errors: policyItem.errors,
owner: policyItem.owner,
areInvoicesEnabled: policyItem.areInvoicesEnabled,
} as Policy;
}

return result;
};

function IOURequestStartPage({
route,
route: {
Expand All @@ -77,12 +118,16 @@ function IOURequestStartPage({
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID}`);
const policy = usePolicy(report?.policyID);
const [lastSelectedTab, selectedTabResult] = useOnyx(`${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.IOU_REQUEST_TYPE}`);
const [selectedTab, setSelectedTab] = useState(lastSelectedTab);
// Derive selectedTab directly instead of using state
const selectedTab = lastSelectedTab;

const isLoadingSelectedTab = shouldUseTab ? isLoadingOnyxValue(selectedTabResult) : false;
const [transaction, transactionResult] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${getNonEmptyStringOnyxID(route?.params.transactionID)}`);
const isLoadingTransaction = isLoadingOnyxValue(transactionResult);
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {
selector: policySelector,
});

const [lastSelectedDistanceRates] = useOnyx(ONYXKEYS.NVP_LAST_SELECTED_DISTANCE_RATES);
const [draftTransactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_DRAFT);
const [isMultiScanEnabled, setIsMultiScanEnabled] = useState(false);
Expand All @@ -109,8 +154,7 @@ function IOURequestStartPage({
// We requestAnimationFrame since the function is called in the animate block in the web implementation
// which fixes a locked animation glitch when swiping between tabs, and aligns with the native implementation internal delay
requestAnimationFrame(() => {
// 2 - PerDiem
if (index !== 2) {
if (index !== PER_DIEM_TAB_INDEX) {
return;
}
perDiemInputRef.current?.focus?.();
Expand Down Expand Up @@ -164,12 +208,7 @@ function IOURequestStartPage({
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

useEffect(() => {
if (isLoadingSelectedTab || selectedTab) {
return;
}
setSelectedTab(lastSelectedTab);
}, [isLoadingSelectedTab, selectedTab, lastSelectedTab]);
// Removed useEffect that was syncing selectedTab state - now derived directly

const navigateBack = () => {
Navigation.closeRHPFlow();
Expand Down Expand Up @@ -219,7 +258,7 @@ function IOURequestStartPage({

const onTabSelected = useCallback(
(newIouType: IOURequestType) => {
setSelectedTab(newIouType);
Tab.setSelectedTab(CONST.TAB.IOU_REQUEST_TYPE, newIouType);
resetIOUTypeIfChanged(newIouType);
},
[resetIOUTypeIfChanged],
Expand Down
5 changes: 2 additions & 3 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,6 @@ function IOURequestStepConfirmation({
const draftPolicyID = getIOURequestPolicyID(initialTransaction, reportDraft);
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${draftPolicyID}`);
const [policyReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${realPolicyID}`);
const [reportDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT);
const [gpsDraftDetails] = useOnyx(ONYXKEYS.GPS_DRAFT_DETAILS);
const [betas] = useOnyx(ONYXKEYS.BETAS);

Expand Down Expand Up @@ -310,9 +309,9 @@ function IOURequestStepConfirmation({
const privateIsArchived = privateIsArchivedMap[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${participant.reportID}`];
return participant.accountID
? getParticipantsOption(participant, personalDetails)
: getReportOption(participant, privateIsArchived, policy, currentUserPersonalDetails.accountID, personalDetails, reportAttributesDerived, reportDrafts);
: getReportOption(participant, privateIsArchived, policy, currentUserPersonalDetails.accountID, personalDetails, reportAttributesDerived);
}) ?? [],
[transaction?.participants, iouType, personalDetails, reportAttributesDerived, reportDrafts, privateIsArchivedMap, policy, currentUserPersonalDetails.accountID],
[transaction?.participants, iouType, personalDetails, reportAttributesDerived, privateIsArchivedMap, policy, currentUserPersonalDetails.accountID],
);
const isPolicyExpenseChat = useMemo(() => participants?.some((participant) => participant.isPolicyExpenseChat), [participants]);
const shouldGenerateTransactionThreadReport = !isBetaEnabled(CONST.BETAS.NO_OPTIMISTIC_TRANSACTION_THREADS);
Expand Down
Loading