From 2333659a67c247e59e8036377cddc069fdf09221 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 10:30:15 +0200 Subject: [PATCH 01/11] Migrate six big-object Onyx keys from useOnyx to Onyx.get --- src/components/AddressSearch/types.ts | 2 +- src/components/CurrencySelectionList/types.ts | 2 +- src/components/Form/types.ts | 2 +- src/hooks/useSubPage/types.ts | 2 +- src/libs/PersonalDetailsUtils.ts | 4 +-- src/libs/WorkflowUtils.ts | 4 +-- src/libs/actions/Transaction.ts | 6 ++-- src/libs/actions/TwoFactorAuthActions.ts | 5 +-- .../AddPersonalBankAccountPage/index.tsx | 5 +-- .../step/DynamicIOURequestStepWaypoint.tsx | 8 +++-- .../subPages/Confirmation.tsx | 5 +-- .../settings/Wallet/PaymentMethodList.tsx | 2 +- .../Wallet/UpdatePersonalBankAccountPage.tsx | 10 ++++-- ...ConnectExistingBusinessBankAccountPage.tsx | 5 +-- .../WorkspaceOverviewCurrencyPage.tsx | 6 ++-- .../actions/IOUTest/UpdateMoneyRequestTest.ts | 8 ++--- tests/actions/ReportTest.ts | 12 ++----- tests/actions/connections/SageIntacct.ts | 3 +- tests/unit/ViolationUtilsTest.ts | 32 +++++++++---------- 19 files changed, 64 insertions(+), 59 deletions(-) diff --git a/src/components/AddressSearch/types.ts b/src/components/AddressSearch/types.ts index 6cab85be7192..d76e2fc93e72 100644 --- a/src/components/AddressSearch/types.ts +++ b/src/components/AddressSearch/types.ts @@ -62,7 +62,7 @@ type AddressSearchProps = ForwardedFSClassProps & { onInputChange?: (value: string | number | Address | StreetValue, key?: string) => void; /** A callback function when an address has been auto-selected */ - onPress?: (props: OnPressProps) => void; + onPress?: (props: OnPressProps) => void | Promise; /** Customize the TextInput container */ containerStyles?: StyleProp; diff --git a/src/components/CurrencySelectionList/types.ts b/src/components/CurrencySelectionList/types.ts index d714f2d1645a..12e6ae67dc48 100644 --- a/src/components/CurrencySelectionList/types.ts +++ b/src/components/CurrencySelectionList/types.ts @@ -16,7 +16,7 @@ type CurrencySelectionListProps = Partial void; + onSelect: (item: CurrencyListItem) => void | Promise; /** The array of selected currencies. This prop should be used when multiple currencies can be selected */ selectedCurrencies?: string[]; diff --git a/src/components/Form/types.ts b/src/components/Form/types.ts index a77ad1536641..23118e0ae550 100644 --- a/src/components/Form/types.ts +++ b/src/components/Form/types.ts @@ -153,7 +153,7 @@ type FormProps = ForwardedFSClassProp isSubmitButtonVisible?: boolean; /** Callback to submit the form */ - onSubmit: (values: FormOnyxValues) => void; + onSubmit: (values: FormOnyxValues) => void | Promise; /** Should the button be enabled when offline */ enabledWhenOffline?: boolean; diff --git a/src/hooks/useSubPage/types.ts b/src/hooks/useSubPage/types.ts index bd233757dfa8..c17c27602240 100644 --- a/src/hooks/useSubPage/types.ts +++ b/src/hooks/useSubPage/types.ts @@ -38,7 +38,7 @@ type UseSubPageProps void; /** called on last sub page */ - onFinished: (data?: unknown) => void; + onFinished: (data?: unknown) => void | Promise; /** index of the page to start from (used when no subPage param in URL) */ startFrom?: number; diff --git a/src/libs/PersonalDetailsUtils.ts b/src/libs/PersonalDetailsUtils.ts index 0c61943ecc1c..b067e601b4c3 100644 --- a/src/libs/PersonalDetailsUtils.ts +++ b/src/libs/PersonalDetailsUtils.ts @@ -10,7 +10,7 @@ import type {OnyxData} from '@src/types/onyx/Request'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; -import type {SetNonNullable} from 'type-fest'; +import type {ReadonlyDeep, SetNonNullable} from 'type-fest'; import {Str} from 'expensify-common'; import Onyx from 'react-native-onyx'; @@ -424,7 +424,7 @@ function getStreetLines(street = '') { * @param privatePersonalDetails - details object * @returns - current address object */ -function getCurrentAddress(privatePersonalDetails: OnyxEntry): Address | undefined { +function getCurrentAddress(privatePersonalDetails: ReadonlyDeep>): ReadonlyDeep
| undefined { const {addresses} = privatePersonalDetails ?? {}; const currentAddress = addresses?.find((address) => address.current); return currentAddress ?? addresses?.[addresses.length - 1]; diff --git a/src/libs/WorkflowUtils.ts b/src/libs/WorkflowUtils.ts index 8b6747b2746a..864b2417283a 100644 --- a/src/libs/WorkflowUtils.ts +++ b/src/libs/WorkflowUtils.ts @@ -23,7 +23,7 @@ import type {PolicyEmployeeList} from '@src/types/onyx/PolicyEmployee'; import type Rule from '@src/types/onyx/Rule'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {Str} from 'expensify-common'; @@ -664,7 +664,7 @@ function updateWorkflowDataOnApproverRemoval({approvalWorkflows, removedApprover * Get eligible business bank accounts for the workspace reimbursement workflow */ function getEligibleExistingBusinessBankAccounts( - bankAccountList: BankAccountList | undefined, + bankAccountList: ReadonlyDeep | undefined, policyCurrency: string | undefined, shouldIncludePartiallySetup?: boolean, excludeBankAccountID?: number, diff --git a/src/libs/actions/Transaction.ts b/src/libs/actions/Transaction.ts index 085a12a4140d..0add8b403b60 100644 --- a/src/libs/actions/Transaction.ts +++ b/src/libs/actions/Transaction.ts @@ -89,10 +89,10 @@ import type {Waypoint, WaypointCollection} from '@src/types/onyx/Transaction'; import type TransactionState from '@src/types/utils/TransactionStateType'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxKey, OnyxUpdate} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; import {originalTransactionIDSelector} from '@selectors/Transaction'; import {getUnixTime} from 'date-fns'; -import lodashClone from 'lodash/clone'; import Onyx from 'react-native-onyx'; import {getAllTransactions} from './IOU'; @@ -102,7 +102,7 @@ type SaveWaypointProps = { index: string; waypoint: RecentWaypoint | null; isDraft?: boolean; - recentWaypointsList?: RecentWaypoint[]; + recentWaypointsList?: ReadonlyDeep; isSplitDraftTransaction?: boolean; }; @@ -159,7 +159,7 @@ function saveWaypoint({transactionID, index, waypoint, isDraft = false, recentWa } const recentWaypointAlreadyExists = recentWaypointsList.find((recentWaypoint) => recentWaypoint?.address === waypoint?.address); if (!recentWaypointAlreadyExists && waypoint !== null) { - const clonedWaypoints = lodashClone(recentWaypointsList); + const clonedWaypoints = [...recentWaypointsList]; const updatedWaypoint = {...waypoint, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; clonedWaypoints.unshift(updatedWaypoint); Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, CONST.RECENT_WAYPOINTS_NUMBER)); diff --git a/src/libs/actions/TwoFactorAuthActions.ts b/src/libs/actions/TwoFactorAuthActions.ts index 156b85bcb329..c357dc471cc6 100644 --- a/src/libs/actions/TwoFactorAuthActions.ts +++ b/src/libs/actions/TwoFactorAuthActions.ts @@ -2,8 +2,9 @@ import Navigation from '@libs/Navigation/Navigation'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Route} from '@src/ROUTES'; +import type {Account} from '@src/types/onyx'; -import type {OnyxMergeInput} from 'react-native-onyx'; +import type {NullishDeep} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; @@ -11,7 +12,7 @@ import Onyx from 'react-native-onyx'; * Clear 2FA data if the flow is interrupted without finishing */ function clearTwoFactorAuthData(clearProgress = false) { - const data: OnyxMergeInput = {recoveryCodes: null, twoFactorAuthSecretKey: null, codesAreCopied: false}; + const data: NullishDeep = {recoveryCodes: null, twoFactorAuthSecretKey: null, codesAreCopied: false}; if (clearProgress) { data.twoFactorAuthSetupInProgress = null; diff --git a/src/pages/AddPersonalBankAccountPage/index.tsx b/src/pages/AddPersonalBankAccountPage/index.tsx index a326c40bc4c7..42eaa3ebc380 100644 --- a/src/pages/AddPersonalBankAccountPage/index.tsx +++ b/src/pages/AddPersonalBankAccountPage/index.tsx @@ -27,6 +27,7 @@ import SCREENS from '@src/SCREENS'; import {useRoute} from '@react-navigation/native'; import React, {useContext, useEffect, useRef} from 'react'; +import Onyx from 'react-native-onyx'; import Address from './substeps/AddressStep'; import Confirmation from './substeps/ConfirmationStep'; @@ -70,7 +71,6 @@ function AddPersonalBankAccountPage() { const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE); const [personalPolicyID] = useOnyx(ONYXKEYS.PERSONAL_POLICY_ID); - const [plaidData] = useOnyx(ONYXKEYS.PLAID_DATA); const shouldCollectInternationalDepositDetails = useShouldCollectInternationalDepositDetails(CONST.COUNTRY.US); const kycWallRef = useContext(KYCWallContext); @@ -108,7 +108,8 @@ function AddPersonalBankAccountPage() { clearPersonalBankAccount(); }; - const submitBankAccountForm = () => { + const submitBankAccountForm = async () => { + const plaidData = await Onyx.get(ONYXKEYS.PLAID_DATA); const bankAccounts = plaidData?.bankAccounts ?? []; const selectedPlaidBankAccount = bankAccounts.find((bankAccount) => bankAccount.plaidAccountID === personalBankAccount?.selectedPlaidAccountID); diff --git a/src/pages/iou/request/step/DynamicIOURequestStepWaypoint.tsx b/src/pages/iou/request/step/DynamicIOURequestStepWaypoint.tsx index f0d9d3c0a91b..70606b9da054 100644 --- a/src/pages/iou/request/step/DynamicIOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/DynamicIOURequestStepWaypoint.tsx @@ -41,6 +41,7 @@ import {useNavigation} from '@react-navigation/native'; import lodashIsEmpty from 'lodash/isEmpty'; import React, {useCallback, useMemo, useRef, useState} from 'react'; import {View} from 'react-native'; +import Onyx from 'react-native-onyx'; import type {WithWritableReportOrNotFoundProps} from './withWritableReportOrNotFound'; @@ -101,7 +102,6 @@ function DynamicIOURequestStepWaypoint({ const [userLocation] = useOnyx(ONYXKEYS.USER_LOCATION); const [recentWaypoints] = useOnyx(ONYXKEYS.NVP_RECENT_WAYPOINTS, {selector: recentWaypointsSelector}); - const [allRecentWaypoints] = useOnyx(ONYXKEYS.NVP_RECENT_WAYPOINTS); const waypointDescriptionKey = useMemo(() => { switch (parsedWaypointIndex) { @@ -138,7 +138,8 @@ function DynamicIOURequestStepWaypoint({ return errors; }; - const save = (waypoint: FormOnyxValues<'waypointForm'>) => { + const save = async (waypoint: FormOnyxValues<'waypointForm'>) => { + const allRecentWaypoints = await Onyx.get(ONYXKEYS.NVP_RECENT_WAYPOINTS); saveWaypoint({ transactionID, index: pageIndex, @@ -173,7 +174,8 @@ function DynamicIOURequestStepWaypoint({ goBack(); }; - const selectWaypoint = (values: Waypoint) => { + const selectWaypoint = async (values: Waypoint) => { + const allRecentWaypoints = await Onyx.get(ONYXKEYS.NVP_RECENT_WAYPOINTS); const waypoint = { lat: values.lat, lng: values.lng, diff --git a/src/pages/settings/Wallet/InternationalDepositAccount/subPages/Confirmation.tsx b/src/pages/settings/Wallet/InternationalDepositAccount/subPages/Confirmation.tsx index be15e75f81ba..23b96bf15c44 100644 --- a/src/pages/settings/Wallet/InternationalDepositAccount/subPages/Confirmation.tsx +++ b/src/pages/settings/Wallet/InternationalDepositAccount/subPages/Confirmation.tsx @@ -29,6 +29,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {CorpayFormField} from '@src/types/onyx'; import React, {useCallback, useEffect} from 'react'; +import Onyx from 'react-native-onyx'; const STEP_INDEXES = CONST.CORPAY_FIELDS.INDEXES.MAPPING; @@ -50,7 +51,6 @@ function TermsAndConditionsLabel() { function Confirmation({onNext, onMove, formValues, fieldsMap}: CustomSubPageProps) { const {translate} = useLocalize(); const styles = useThemeStyles(); - const [corpayFields] = useOnyx(ONYXKEYS.CORPAY_FIELDS); const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); const {isOffline} = useNetwork(); const {getCurrencySymbol} = useCurrencyListActions(); @@ -69,7 +69,8 @@ function Confirmation({onNext, onMove, formValues, fieldsMap}: CustomSubPageProp return formValues[fieldName]; }; - const getDataAndGoToNextStep = (values: FormOnyxValues) => { + const getDataAndGoToNextStep = async (values: FormOnyxValues) => { + const corpayFields = await Onyx.get(ONYXKEYS.CORPAY_FIELDS); createCorpayBankAccountForWalletFlow({...formValues, ...values}, corpayFields?.classification ?? '', corpayFields?.destinationCountry ?? '', corpayFields?.preferredMethod ?? ''); }; diff --git a/src/pages/settings/Wallet/PaymentMethodList.tsx b/src/pages/settings/Wallet/PaymentMethodList.tsx index 6535eaabef22..20149022003b 100644 --- a/src/pages/settings/Wallet/PaymentMethodList.tsx +++ b/src/pages/settings/Wallet/PaymentMethodList.tsx @@ -130,7 +130,7 @@ type PaymentMethodListProps = { policyID?: string; /** Function to be called when the user presses the add bank account button */ - onAddBankAccountPress?: () => void; + onAddBankAccountPress?: () => void | Promise; /** The icon to be displayed in the right side of the payment method item */ itemIconRight?: IconAsset; diff --git a/src/pages/settings/Wallet/UpdatePersonalBankAccountPage.tsx b/src/pages/settings/Wallet/UpdatePersonalBankAccountPage.tsx index 8438a0268ac3..23ffcf1d41b3 100644 --- a/src/pages/settings/Wallet/UpdatePersonalBankAccountPage.tsx +++ b/src/pages/settings/Wallet/UpdatePersonalBankAccountPage.tsx @@ -32,6 +32,7 @@ import type {BankAccountList} from '@src/types/onyx'; import type {OnyxEntry} from 'react-native-onyx'; import React, {useEffect} from 'react'; +import Onyx from 'react-native-onyx'; const PAGE_NAME = CONST.UPDATE_PERSONAL_BANK_ACCOUNT.PAGE_NAME; @@ -106,8 +107,6 @@ function UpdatePersonalBankAccountPage() { const {translate} = useLocalize(); const styles = useThemeStyles(); - const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS); - const [personalBankAccountDraft] = useOnyx(ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT_FORM_DRAFT); const [homeAddressDraft] = useOnyx(ONYXKEYS.FORMS.HOME_ADDRESS_FORM_DRAFT); const [personalBankAccount, personalBankAccountResult] = useOnyx(ONYXKEYS.PERSONAL_BANK_ACCOUNT); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); @@ -142,11 +141,16 @@ function UpdatePersonalBankAccountPage() { Navigation.goBack(ROUTES.SETTINGS_WALLET); }; - const submitPersonalInfo = (finishData?: unknown) => { + const submitPersonalInfo = async (finishData?: unknown) => { if (!personalBankAccount?.bankAccountID || personalBankAccount?.isLoading) { return; } + const [privatePersonalDetails, personalBankAccountDraft] = await Promise.all([ + Onyx.get(ONYXKEYS.PRIVATE_PERSONAL_DETAILS), + Onyx.get(ONYXKEYS.FORMS.PERSONAL_BANK_ACCOUNT_FORM_DRAFT), + ]); + // AddressStep forwards its form values via finishData so prefilled fields (e.g. auto-filled zip) aren't lost. const submittedAddress = finishData as SubmittedAddress | undefined; diff --git a/src/pages/workspace/ConnectExistingBusinessBankAccountPage.tsx b/src/pages/workspace/ConnectExistingBusinessBankAccountPage.tsx index 799b3b6e4c1a..49e49cb79855 100644 --- a/src/pages/workspace/ConnectExistingBusinessBankAccountPage.tsx +++ b/src/pages/workspace/ConnectExistingBusinessBankAccountPage.tsx @@ -32,6 +32,7 @@ import type SCREENS from '@src/SCREENS'; import React, {useState} from 'react'; import {View} from 'react-native'; +import Onyx from 'react-native-onyx'; type ConnectExistingBusinessBankAccountPageProps = PlatformStackScreenProps; @@ -42,7 +43,6 @@ function ConnectExistingBusinessBankAccountPage({route}: ConnectExistingBusiness const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); const [lastPaymentMethod] = useOnyx(ONYXKEYS.NVP_LAST_PAYMENT_METHOD); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); - const [reimbursementAccount] = useOnyx(ONYXKEYS.REIMBURSEMENT_ACCOUNT); const policyName = policy?.name ?? ''; const policyCurrency = policy?.outputCurrency ?? ''; const {shouldUseNarrowLayout} = useResponsiveLayout(); @@ -56,11 +56,12 @@ function ConnectExistingBusinessBankAccountPage({route}: ConnectExistingBusiness const [isSelectingBankAccount, setIsSelectingBankAccount] = useState(false); - const handleAddBankAccountPress = () => { + const handleAddBankAccountPress = async () => { // When changing the bank account we prepare a fresh setup (clearing the current account without disconnecting it). // We keep the policyID in the route so the newly connected account still links to the workspace's Workflows > Payments. // We must not set the loading flag here: during the change flow the page skips fetchData, so nothing would reset it. if (isChangingBankAccount) { + const reimbursementAccount = await Onyx.get(ONYXKEYS.REIMBURSEMENT_ACCOUNT); prepareNewBankAccountSetup(policyCurrency, reimbursementAccount); } else { setReimbursementAccountLoading(true); diff --git a/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx b/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx index 27b60a1a6efe..63f2bb975a37 100644 --- a/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx +++ b/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx @@ -4,7 +4,6 @@ import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; import useLocalize from '@hooks/useLocalize'; -import useOnyx from '@hooks/useOnyx'; import useReviewWorkspaceSettingsTaskCompletion from '@hooks/useReviewWorkspaceSettingsTaskCompletion'; import useShouldBlockCurrencyChange from '@hooks/useShouldBlockCurrencyChange'; @@ -29,6 +28,7 @@ import type {TupleToUnion} from 'type-fest'; import {useRoute} from '@react-navigation/native'; import React from 'react'; +import Onyx from 'react-native-onyx'; import type {WithPolicyAndFullscreenLoadingProps} from './withPolicyAndFullscreenLoading'; @@ -42,11 +42,10 @@ function WorkspaceOverviewCurrencyPage({policy}: WorkspaceOverviewCurrencyPagePr const route = useRoute>(); const {translate} = useLocalize(); const isForcedToChangeCurrency = !!route.params?.isForcedToChangeCurrency; - const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); const shouldBlockCurrencyChange = useShouldBlockCurrencyChange(policy?.id); const getReviewWorkspaceSettingsTaskCompletion = useReviewWorkspaceSettingsTaskCompletion(); - const onSelectCurrency = (item: CurrencyListItem) => { + const onSelectCurrency = async (item: CurrencyListItem) => { if (!policy) { return; } @@ -56,6 +55,7 @@ function WorkspaceOverviewCurrencyPage({policy}: WorkspaceOverviewCurrencyPagePr if (isForcedToChangeCurrency) { if (isCurrencySupportedForGlobalReimbursement(item.currencyCode as CurrencyType)) { + const bankAccountList = await Onyx.get(ONYXKEYS.BANK_ACCOUNT_LIST); const hasValidExistingAccounts = getEligibleExistingBusinessBankAccounts(bankAccountList, item.currencyCode, true).length > 0; if (hasValidExistingAccounts) { Navigation.navigate(ROUTES.BANK_ACCOUNT_CONNECT_EXISTING_BUSINESS_BANK_ACCOUNT.getRoute(policy.id, ROUTES.WORKSPACE_WORKFLOWS.getRoute(policy.id))); diff --git a/tests/actions/IOUTest/UpdateMoneyRequestTest.ts b/tests/actions/IOUTest/UpdateMoneyRequestTest.ts index 1b67da776c2d..ce9c865bba2b 100644 --- a/tests/actions/IOUTest/UpdateMoneyRequestTest.ts +++ b/tests/actions/IOUTest/UpdateMoneyRequestTest.ts @@ -29,7 +29,7 @@ import type {CurrentUserPersonalDetails} from '@src/types/onyx/PersonalDetails'; import type {Routes} from '@src/types/onyx/Transaction'; import type Transaction from '@src/types/onyx/Transaction'; -import type {NullishDeep, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; +import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; /* eslint-disable @typescript-eslint/no-unsafe-assignment */ import {format} from 'date-fns'; @@ -594,7 +594,7 @@ describe('actions/IOU/UpdateMoneyRequest', () => { const optimisticSnapshot = onyxData.optimisticData?.find( (update): update is SnapshotUpdate => update.key === snapshotKey && (update.onyxMethod === Onyx.METHOD.SET || update.onyxMethod === Onyx.METHOD.MERGE), ); - const optimisticSnapshotData: NullishDeep | null | undefined = optimisticSnapshot?.value?.data; + const optimisticSnapshotData = optimisticSnapshot?.value?.data; const optimisticTransaction = optimisticSnapshotData?.[transactionKey]; expect(optimisticTransaction).toMatchObject({ modifiedAmount: -20000, @@ -604,14 +604,14 @@ describe('actions/IOU/UpdateMoneyRequest', () => { const successSnapshot = onyxData.successData?.find( (update): update is SnapshotUpdate => update.key === snapshotKey && (update.onyxMethod === Onyx.METHOD.SET || update.onyxMethod === Onyx.METHOD.MERGE), ); - const successSnapshotData: NullishDeep | null | undefined = successSnapshot?.value?.data; + const successSnapshotData = successSnapshot?.value?.data; const successTransaction = successSnapshotData?.[transactionKey]; expect(successTransaction).toEqual({pendingFields: {amount: null}}); const failureSnapshot = onyxData.failureData?.find( (update): update is SnapshotUpdate => update.key === snapshotKey && (update.onyxMethod === Onyx.METHOD.SET || update.onyxMethod === Onyx.METHOD.MERGE), ); - const failureSnapshotData: NullishDeep | null | undefined = failureSnapshot?.value?.data; + const failureSnapshotData = failureSnapshot?.value?.data; const failureTransaction = failureSnapshotData?.[transactionKey]; expect(failureTransaction).toMatchObject({ transactionID, diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 9d84dab70431..122321695902 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -745,6 +745,8 @@ describe('actions/Report', () => { reportID: REPORT_ID, }; + reportActionCreatedDate = DateUtils.getDBTime(); + const optimisticReportActions: OnyxUpdate = { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, @@ -766,20 +768,12 @@ describe('actions/Report', () => { 400: { ...USER_1_BASE_ACTION, message: [{type: 'COMMENT', html: 'Current User Comment 3', text: 'Current User Comment 3'}], - created: DateUtils.getDBTime(), + created: reportActionCreatedDate, reportActionID: '400', }, }, }; - reportActionCreatedDate = DateUtils.getDBTime(); - - const optimisticReportActionsValue = optimisticReportActions.value; - - if (optimisticReportActionsValue?.[400]) { - optimisticReportActionsValue[400].created = reportActionCreatedDate; - } - // When we emit the events for these pending created actions to update them to not pending PusherHelper.emitOnyxUpdate([ { diff --git a/tests/actions/connections/SageIntacct.ts b/tests/actions/connections/SageIntacct.ts index d075664be702..0f6bb72c4f0b 100644 --- a/tests/actions/connections/SageIntacct.ts +++ b/tests/actions/connections/SageIntacct.ts @@ -10,6 +10,7 @@ import type {SageIntacctConnectionsConfig} from '@src/types/onyx/Policy'; import type {AnyOnyxData} from '@src/types/onyx/Request'; import type {NullishDeep, OnyxUpdate} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; import Onyx from 'react-native-onyx'; @@ -23,7 +24,7 @@ const writeSpy = jest.spyOn(API, 'write'); const MOCK_POLICY_ID = 'MOCK_POLICY_ID'; const MOCK_ONYX_ERROR = {key: 'error'}; -type SageIntacctConfigMerge = NullishDeep; +type SageIntacctConfigMerge = ReadonlyDeep>; function getSageIntacctConfig(update?: OnyxUpdate): SageIntacctConfigMerge | undefined { if (!update || !update.value || typeof update.value !== 'object' || !('connections' in update.value)) { diff --git a/tests/unit/ViolationUtilsTest.ts b/tests/unit/ViolationUtilsTest.ts index 8429995f05cb..f179c373cbe8 100644 --- a/tests/unit/ViolationUtilsTest.ts +++ b/tests/unit/ViolationUtilsTest.ts @@ -917,7 +917,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeDefined(); expect(itemizedReceiptViolation?.type).toBe(CONST.VIOLATION_TYPES.VIOLATION); }); @@ -939,7 +939,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const foundReceiptRequiredViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); + const foundReceiptRequiredViolation = violations.find((v) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); expect(foundReceiptRequiredViolation).toBeUndefined(); }); @@ -960,8 +960,8 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const receiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const receiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); // Should have itemized receipt violation but NOT regular receipt violation expect(itemizedReceiptViolation).toBeDefined(); expect(receiptViolation).toBeUndefined(); @@ -984,7 +984,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeUndefined(); }); @@ -1005,7 +1005,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeUndefined(); }); }); @@ -1057,7 +1057,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeDefined(); expect(itemizedReceiptViolation?.data).toBeUndefined(); // Category-level violations don't have data }); @@ -1077,7 +1077,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeUndefined(); // Category "Never" should override policy }); @@ -1096,7 +1096,7 @@ describe('getViolationsOnyxData', () => { isInvoiceTransaction: false, }); const violations = getTransactionViolationsFromResult(result); - const itemizedReceiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedReceiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedReceiptViolation).toBeDefined(); // Should follow policy threshold }); @@ -1123,8 +1123,8 @@ describe('getViolationsOnyxData', () => { const violations = getTransactionViolationsFromResult(result); // Then the itemized violation should be removed and replaced with receiptRequired because the policy still requires receipts - const itemizedViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); - const receiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); + const itemizedViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const receiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); expect(itemizedViolation).toBeUndefined(); expect(receiptViolation).toBeDefined(); }); @@ -1151,7 +1151,7 @@ describe('getViolationsOnyxData', () => { const violations = getTransactionViolationsFromResult(result); // Then the violation should have updated threshold data to reflect the current policy settings - const itemizedViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const itemizedViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(itemizedViolation).toBeDefined(); expect(itemizedViolation?.data?.amount).toBe(7500); expect(itemizedViolation?.data?.currency).toBe(CONST.CURRENCY.USD); @@ -1180,8 +1180,8 @@ describe('getViolationsOnyxData', () => { const violations = getTransactionViolationsFromResult(result); // Then itemized should supersede receipt because itemized is more restrictive - const receiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); - const itemizedViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const receiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); + const itemizedViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); expect(receiptViolation).toBeUndefined(); expect(itemizedViolation).toBeDefined(); }); @@ -1209,8 +1209,8 @@ describe('getViolationsOnyxData', () => { const violations = getTransactionViolationsFromResult(result); // Then no receipt violations should exist because category overrides take precedence over policy settings - const itemizedViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); - const receiptViolation = violations.find((v: TransactionViolation) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); + const itemizedViolation = violations.find((v) => v.name === CONST.VIOLATIONS.ITEMIZED_RECEIPT_REQUIRED); + const receiptViolation = violations.find((v) => v.name === CONST.VIOLATIONS.RECEIPT_REQUIRED); expect(itemizedViolation).toBeUndefined(); expect(receiptViolation).toBeUndefined(); }); From 6610dd5ee99c278ac1217f0d20ea89d3e5152c1e Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 10:37:28 +0200 Subject: [PATCH 02/11] Type Onyx write inputs as ReadonlyDeep via patch-package --- patches/react-native-onyx/details.md | 17 +++++++++++++++++ ...yx+3.0.110+001+readonly-write-inputs.patch | 19 +++++++++++++++++++ src/libs/actions/IOU/BulkEdit.ts | 12 ++++++++++-- .../actions/ReimbursementAccount/index.ts | 4 ++-- 4 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 patches/react-native-onyx/details.md create mode 100644 patches/react-native-onyx/react-native-onyx+3.0.110+001+readonly-write-inputs.patch diff --git a/patches/react-native-onyx/details.md b/patches/react-native-onyx/details.md new file mode 100644 index 000000000000..774f84f55173 --- /dev/null +++ b/patches/react-native-onyx/details.md @@ -0,0 +1,17 @@ +# `react-native-onyx` patches + +### [react-native-onyx+3.0.110+001+readonly-write-inputs.patch](react-native-onyx+3.0.110+001+readonly-write-inputs.patch) + +- Reason: + + ``` + Experiment only, not for merge. Types every Onyx write input as `ReadonlyDeep`, so a value read out of the + cache can be passed straight to `set`/`merge`/`multiSet`/`mergeCollection`/`setCollection`/`update` without a + copy. This is the library half of https://github.com/Expensify/App/issues/71206 and is here so CI can measure + the cost across the repo. It is a widening: nothing that passes a value in can break, and Onyx does not mutate + write inputs at runtime. + ``` + +- Upstream PR/issue: https://github.com/Expensify/App/issues/71206 +- E/App issue: https://github.com/Expensify/App/issues/71206 +- PR introducing patch: experiment branch, no PR diff --git a/patches/react-native-onyx/react-native-onyx+3.0.110+001+readonly-write-inputs.patch b/patches/react-native-onyx/react-native-onyx+3.0.110+001+readonly-write-inputs.patch new file mode 100644 index 000000000000..10ecc07a314d --- /dev/null +++ b/patches/react-native-onyx/react-native-onyx+3.0.110+001+readonly-write-inputs.patch @@ -0,0 +1,19 @@ +diff --git a/node_modules/react-native-onyx/dist/types.d.ts b/node_modules/react-native-onyx/dist/types.d.ts +index cb91605..f974c46 100644 +--- a/node_modules/react-native-onyx/dist/types.d.ts ++++ b/node_modules/react-native-onyx/dist/types.d.ts +@@ -1,4 +1,4 @@ +-import type { Merge } from 'type-fest'; ++import type { Merge, ReadonlyDeep } from 'type-fest'; + import type OnyxUtils from './OnyxUtils'; + import type { OnyxMethod } from './OnyxUtils'; + import type { FastMergeReplaceNullPatch } from './utils'; +@@ -219,7 +219,7 @@ type OnyxCollectionInputValue = OnyxInputValue = OnyxInputValue>; ++type OnyxInput = ReadonlyDeep>>; + /** + * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. + * diff --git a/src/libs/actions/IOU/BulkEdit.ts b/src/libs/actions/IOU/BulkEdit.ts index 8361e72e4615..4123fe01f028 100644 --- a/src/libs/actions/IOU/BulkEdit.ts +++ b/src/libs/actions/IOU/BulkEdit.ts @@ -45,7 +45,7 @@ import type {SearchResultDataType} from '@src/types/onyx/SearchResults'; import type {TransactionChanges} from '@src/types/onyx/Transaction'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {deepEqual} from 'fast-equals'; // lodashUnionBy de-dupes recent attendees by email/displayName in one pass; no lodash-free equivalent is used here @@ -56,6 +56,14 @@ import Onyx from 'react-native-onyx'; import {getRecentAttendees} from '.'; import {getUpdatedMoneyRequestReportData} from './MoneyRequestBuilder'; +/** + * A snapshot `data` object under construction. The container is mutable so keys can be assigned one at a time, + * while each value keeps the readonly shape it has when it comes out of an Onyx write input (`OnyxUpdate['value']`). + */ +type SnapshotDataDraft = { + -readonly [TKey in keyof NullishDeep]: ReadonlyDeep[TKey]>; +}; + type BulkEditWriteOnyxData = { optimisticData: Array< OnyxUpdate< @@ -608,7 +616,7 @@ function updateMultipleMoneyRequests({ // result rendering and is not automatically updated by the TRANSACTION write above). if (hash) { // Initializing as an empty typed object to allow dynamic key assignment resolves TypeScript type inference issue - const optimisticSnapshotData: NullishDeep = {}; + const optimisticSnapshotData: SnapshotDataDraft = {}; optimisticSnapshotData[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] = {...genericUpdatedTransaction, pendingFields}; if (optimisticViolationsData && optimisticViolationsData.onyxMethod === Onyx.METHOD.SET) { optimisticSnapshotData[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`] = optimisticViolationsData.value; diff --git a/src/libs/actions/ReimbursementAccount/index.ts b/src/libs/actions/ReimbursementAccount/index.ts index fccba12680b2..cbd43666a256 100644 --- a/src/libs/actions/ReimbursementAccount/index.ts +++ b/src/libs/actions/ReimbursementAccount/index.ts @@ -7,7 +7,7 @@ import type ReimbursementAccount from '@src/types/onyx/ReimbursementAccount'; import type {ACHData, ReimbursementAccountSubStep} from '@src/types/onyx/ReimbursementAccount'; import type {OnyxEntry} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import Onyx from 'react-native-onyx'; @@ -68,7 +68,7 @@ function clearReimbursementAccountBackup() { * and initializing draft with country and currency. * We need to temporarily clear this data to set up new account without disconnecting existing one */ -function prepareNewBankAccountSetup(currency: string, reimbursementAccountToBackup?: OnyxEntry) { +function prepareNewBankAccountSetup(currency: string, reimbursementAccountToBackup?: ReadonlyDeep>) { // Snapshot the account before clearing it, so it can be restored if the user abandons the change flow. Any real // account has achData.bankAccountID; the default/empty account has none, so skip it. if (reimbursementAccountToBackup?.achData?.bankAccountID) { From c0bd9229ce9f7050c69e2e3213fe091e3b2bd7ee Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 11:31:31 +0200 Subject: [PATCH 03/11] WIP readonly Onyx.get signature chain --- src/libs/ReportUtils.ts | 28 +++++++++---------- .../TransactionUtils/getDistanceInMeters.ts | 4 ++- .../TransactionUtils/getSelectedRouteKey.ts | 4 ++- src/libs/TransactionUtils/index.ts | 4 +-- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 39dc37d6f666..a1d9477f4d3a 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -80,7 +80,7 @@ import type {Locale as DateFnsLocale} from 'date-fns'; import type {ColorValue} from 'react-native'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; import type {SvgProps} from 'react-native-svg'; -import type {SetRequired, TupleToUnion, ValueOf} from 'type-fest'; +import type {ReadonlyDeep, SetRequired, TupleToUnion, ValueOf} from 'type-fest'; /* eslint-disable max-lines */ import {findFocusedRoute} from '@react-navigation/native'; @@ -1513,7 +1513,7 @@ function isReportIDApproved(reportID: string | undefined) { /** * Checks if a report is an Expense report. */ -function isExpenseReport(reportOrID: OnyxInputOrEntry | string): boolean { +function isExpenseReport(reportOrID: ReadonlyDeep> | string): boolean { const report = typeof reportOrID === 'string' ? (getReport(reportOrID, deprecatedAllReports) ?? null) : reportOrID; return report?.type === CONST.REPORT.TYPE.EXPENSE; } @@ -1605,7 +1605,7 @@ function isReportManuallyReimbursed(report: OnyxEntry): boolean { /** * Checks if the supplied report is an expense report in Open state and status. */ -function isOpenExpenseReport(report: OnyxInputOrEntry): boolean { +function isOpenExpenseReport(report: ReadonlyDeep>): boolean { return isExpenseReport(report) && report?.stateNum === CONST.REPORT.STATE_NUM.OPEN && report?.statusNum === CONST.REPORT.STATUS_NUM.OPEN; } @@ -1657,7 +1657,7 @@ function isSettled(reportOrID: OnyxInputOrEntry | string | undefined, re * Whether the current user is the submitter of the report */ // TODO: currentUserAccountID will be required eventually so this becomes a pure function. Subscribe the data via useOnyx and pass it from the component. Refactor issue: https://github.com/Expensify/App/issues/66412 -function isCurrentUserSubmitter(report: OnyxEntry, currentUserAccountID?: number): boolean { +function isCurrentUserSubmitter(report: ReadonlyDeep>, currentUserAccountID?: number): boolean { return !!report && report.ownerAccountID === (currentUserAccountID ?? deprecatedCurrentUserAccountID); } @@ -2030,7 +2030,7 @@ function shouldEnableNegative(report: OnyxEntry, policy?: OnyxEntry): boolean { +function isProcessingReport(report: ReadonlyDeep>): boolean { return report?.stateNum === CONST.REPORT.STATE_NUM.SUBMITTED && report?.statusNum === CONST.REPORT.STATUS_NUM.SUBMITTED; } @@ -2074,7 +2074,7 @@ function hasReportBeenForwardedSinceLastSubmit(report: OnyxEntry, report return reportActionsArray.some((action) => isForwardedAction(action) && action.created > lastSubmittedAt); } -function isAwaitingFirstLevelApproval(report: OnyxEntry): boolean { +function isAwaitingFirstLevelApproval(report: ReadonlyDeep>): boolean { if (!report) { return false; } @@ -2619,7 +2619,7 @@ function isArchivedNonExpenseReport(report: OnyxInputOrEntry, isReportAr * Whether the provided report is an archived report */ -function isArchivedReport(reportNameValuePairs?: OnyxInputOrEntry): boolean { +function isArchivedReport(reportNameValuePairs?: ReadonlyDeep>): boolean { return !!reportNameValuePairs?.private_isArchived; } @@ -2804,7 +2804,7 @@ function isMoneyRequest(reportOrID: OnyxEntry | string): boolean { /** * Checks if a report is an IOU or expense report. */ -function isMoneyRequestReport(reportOrID: OnyxInputOrEntry | string, reports?: Report[] | OnyxCollection): boolean { +function isMoneyRequestReport(reportOrID: ReadonlyDeep> | string, reports?: Report[] | OnyxCollection): boolean { const report = typeof reportOrID === 'string' ? (getReport(reportOrID, reports ?? deprecatedAllReports) ?? null) : reportOrID; return isIOUReport(report) || isExpenseReport(report); } @@ -3017,7 +3017,7 @@ function getChildReportNotificationPreference(reportAction: OnyxInputOrEntry, isReportArchived = false): boolean { +function canAddOrDeleteTransactions(moneyRequestReport: ReadonlyDeep>, isReportArchived = false): boolean { if (!(isMoneyRequestReport(moneyRequestReport) || isInvoiceReport(moneyRequestReport)) || isReportArchived) { return false; } @@ -3046,7 +3046,7 @@ function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry, isRep * Returns false if: * - if current user is not the submitter of an expense report */ -function canAddTransaction(moneyRequestReport: OnyxEntry, isReportArchived = false, isMovingTransaction = false): boolean { +function canAddTransaction(moneyRequestReport: ReadonlyDeep>, isReportArchived = false, isMovingTransaction = false): boolean { if (!isMoneyRequestReport(moneyRequestReport)) { return false; } @@ -5377,7 +5377,7 @@ function canEditFieldOfMoneyRequest({ fieldToEdit: ValueOf; isDeleteAction?: boolean; isChatReportArchived?: boolean; - outstandingReportsByPolicyID?: OutstandingReportsByPolicyIDDerivedValue; + outstandingReportsByPolicyID?: ReadonlyDeep; transaction: OnyxEntry; report?: OnyxInputOrEntry; policy?: OnyxEntry; @@ -12370,15 +12370,15 @@ function createDraftTransactionAndNavigateToParticipantSelector({ /** * Check if a report is forwarded or not */ -function isForwardedReport(report: OnyxEntry): boolean { +function isForwardedReport(report: ReadonlyDeep>): boolean { return isProcessingReport(report) && !isAwaitingFirstLevelApproval(report); } function isReportOutstanding( - iouReport: OnyxInputOrEntry, + iouReport: ReadonlyDeep>, policyID: string | undefined, // Temporarily optional while archived report checks are migrated in smaller PRs. Remove this fallback as part of https://github.com/Expensify/App/issues/66422. - reportNameValuePair?: OnyxInputOrEntry, + reportNameValuePair?: ReadonlyDeep>, allowSubmitted = true, ): boolean { if ( diff --git a/src/libs/TransactionUtils/getDistanceInMeters.ts b/src/libs/TransactionUtils/getDistanceInMeters.ts index f8ddd0f39b65..f8416b12e0f4 100644 --- a/src/libs/TransactionUtils/getDistanceInMeters.ts +++ b/src/libs/TransactionUtils/getDistanceInMeters.ts @@ -4,11 +4,13 @@ import CONST from '@src/CONST'; import type {OnyxInputOrEntry, Transaction} from '@src/types/onyx'; import type {Unit} from '@src/types/onyx/Policy'; +import type {ReadonlyDeep} from 'type-fest'; + import getSelectedRouteKey from './getSelectedRouteKey'; // Get the distance in meters from the transaction. // This function is placed in a separate file to avoid circular dependencies. -function getDistanceInMeters(transaction: OnyxInputOrEntry, unit: Unit | undefined) { +function getDistanceInMeters(transaction: ReadonlyDeep>, unit: Unit | undefined) { // If the request is completed, transaction.routes is cleared and comment.customUnit.quantity holds the new distance in the selected unit. // We need to convert it from the selected distance unit to meters. // This check takes priority because after a manual distance edit, routes.route0.distance may still diff --git a/src/libs/TransactionUtils/getSelectedRouteKey.ts b/src/libs/TransactionUtils/getSelectedRouteKey.ts index 0482b364329e..3a27dd3cf3a7 100644 --- a/src/libs/TransactionUtils/getSelectedRouteKey.ts +++ b/src/libs/TransactionUtils/getSelectedRouteKey.ts @@ -1,10 +1,12 @@ import CONST from '@src/CONST'; import type {OnyxInputOrEntry, Transaction} from '@src/types/onyx'; +import type {ReadonlyDeep} from 'type-fest'; + // Kept in its own file to avoid circular dependencies. `comment.selectedRouteKey` is frontend-only — // only the selected route's distance survives a save, echoed as `comment.customUnit.routeDistanceMeters` // (route distance, not displayed), so a saved pick is recovered by matching it. -function getSelectedRouteKey(transaction: OnyxInputOrEntry): string { +function getSelectedRouteKey(transaction: ReadonlyDeep>): string { const routes = transaction?.routes; // A local pick wins over the BE value: it is the user's pending, not-yet-saved intent. The existence check diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 0d76e0713807..ecaa63ff8b9e 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -95,7 +95,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {Locale as DateFnsLocale} from 'date-fns'; import type {NullishDeep, OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {format, isValid, parse} from 'date-fns'; import {SafeString, Str} from 'expensify-common'; @@ -3554,7 +3554,7 @@ function getSelectedRouteDistance(transaction: OnyxEntry): number | * picking an alternate route, that route's distance — so the comparison has to be against the *selected* route and not * the primary one, or every alternate route selection would look like an override. */ -function hasManualDistanceOverride(transaction: OnyxInputOrEntry): boolean { +function hasManualDistanceOverride(transaction: ReadonlyDeep>): boolean { const quantity = transaction?.comment?.customUnit?.quantity; const selectedRouteDistanceInMeters = transaction?.routes?.[getSelectedRouteKey(transaction)]?.distance; if (quantity == null || !selectedRouteDistanceInMeters) { From 1cf44b9dac3e4b1a7040ad1dc25e9936fcda9fcb Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 12:01:04 +0200 Subject: [PATCH 04/11] Add scan for useOnyx subscriptions convertible to Onyx.get --- scripts/onyxGetMigrationCandidates.ts | 1501 +++++++++++++++++++++++++ 1 file changed, 1501 insertions(+) create mode 100644 scripts/onyxGetMigrationCandidates.ts diff --git a/scripts/onyxGetMigrationCandidates.ts b/scripts/onyxGetMigrationCandidates.ts new file mode 100644 index 000000000000..1087b3ed30ea --- /dev/null +++ b/scripts/onyxGetMigrationCandidates.ts @@ -0,0 +1,1501 @@ +#!/usr/bin/env bun +/** + * Onyx.get() migration candidate finder. + * + * Scans `src/` for `useOnyx()` subscriptions whose value never reaches render, which are the only ones + * `Onyx.get()` can replace. For every subscription it resolves the ONYXKEYS path being read, follows every + * reference to the destructured value, and classifies the position each reference sits in with the same + * render/event/module-scope model as `rulesdir/no-unsafe-onyx-read`. + * + * The output is a work list, not a verdict. What the script can decide on its own is the mechanical part: + * whether the key is one of the Search snapshot keys the rule refuses outright, whether the key is even + * statically resolvable (the rule refuses the rest), and whether any reference to the value is reachable + * from render. What it cannot decide is stated per candidate as a caveat, and the hazards no static pass + * catches are listed as a manual checklist in the report. + * + * Usage: + * bun scripts/onyxGetMigrationCandidates.ts # writes the report and the PR map into .context/ + * bun scripts/onyxGetMigrationCandidates.ts --prs plan.md # write the PR map somewhere else + * bun scripts/onyxGetMigrationCandidates.ts --out report.md # write somewhere else + * bun scripts/onyxGetMigrationCandidates.ts --json data.json # also emit the raw findings + * bun scripts/onyxGetMigrationCandidates.ts src/libs src/pages # restrict the scan + */ +import type {NodePath} from '@babel/traverse'; +import type {CallExpression, Function as FunctionNode, Identifier, Node, ObjectExpression} from '@babel/types'; + +import {parse} from '@babel/parser'; +import traverse from '@babel/traverse'; +import fs from 'fs'; +import path from 'path'; + +const DEFAULT_INPUTS = ['src']; +const DEFAULT_OUTPUT = path.join('.context', 'onyx-get-migration-candidates.md'); +const DEFAULT_PR_OUTPUT = path.join('.context', 'onyx-get-migration-prs.md'); +const FILE_EXTENSIONS = new Set(['.ts', '.tsx']); + +const HOOK_NAME = 'useOnyx'; +const ONYXKEYS_ROOT = 'ONYXKEYS'; + +/** Where the Search snapshot key list lives, and how to pull it out. Kept identical to the lint rule so the two cannot disagree. */ +const SNAPSHOT_KEYS_SOURCE = 'src/CONST/runtimeConfigured.ts'; +const SNAPSHOT_KEYS_DECLARATION = /SEARCH_SNAPSHOT_ONYX_KEYS:\s*\[([^\]]*)\]/; + +/** Array methods that call their callback synchronously, so a read inside one cannot be awaited without restructuring the caller. */ +const SYNCHRONOUS_CALLBACK_METHODS = new Set(['map', 'filter', 'reduce', 'reduceRight', 'forEach', 'find', 'findIndex', 'findLast', 'findLastIndex', 'flatMap', 'some', 'every', 'sort']); + +/** Hook arguments React runs while rendering, by argument index. */ +const RENDER_TIME_HOOK_ARGUMENTS = new Map([ + ['useMemo', new Set([0])], + ['useState', new Set([0])], + ['useReducer', new Set([2])], + ['useSyncExternalStore', new Set([1, 2])], +]); + +/** Names a callback carries when it is meant to be attached to an event rather than called by the receiver. */ +const EVENT_CALLBACK_NAME = /^(on|handle)[A-Z]/; + +const COMPONENT_WRAPPER_NAMES = new Set(['memo', 'forwardRef']); +const SYNCHRONOUS_EXECUTOR_NAMES = new Set(['Promise']); +const RENDER_TIME_OPTION_NAMES = new Set(['selector']); + +/** + * `useOnyx` options that change what the subscription returns compared to a plain read, so a conversion + * cannot be judged mechanically. `selector` is handled separately because reapplying it at the read site is + * a known, small fix; these are not. + */ +const SHAPE_CHANGING_OPTIONS = new Set(['initWithStoredValues', 'allowStaleData', 'initialValue']); + +/** Where a reference to a subscribed value sits, in the order of how much it rules out. */ +type ReferenceKind = 'render' | 'dependency-array' | 'synchronous' | 'event'; + +type ReferenceSite = { + kind: ReferenceKind; + line: number; + /** The name of the enclosing callback, or the prop/hook it is passed to, so a reviewer can find it. */ + context: string; +}; + +type Caveat = + | 'has-selector' + | 'has-default-value' + | 'derived-key' + | 'collection-key' + | 'synchronous-reference' + | 'non-event-prop' + | 'parked-in-state' + | 'escapes-via-return' + | 'shape-changing-options' + | 'uses-metadata' + | 'in-dependency-array' + | 'trigger-chain' + | 'not-array-destructured'; + +/** The verdicts that are work items: the three the report lists, and the only ones the JSON keeps. */ +const CONVERTIBLE_VERDICTS = new Set(['drop-in', 'needs-fix', 'needs-decision']); + +/** Caveats that a reviewer has to resolve by reading the code, rather than by applying a known fix. */ +const BLOCKING_CAVEATS = new Set(['uses-metadata', 'in-dependency-array', 'trigger-chain', 'not-array-destructured', 'shape-changing-options']); + +type Verdict = 'drop-in' | 'needs-fix' | 'needs-decision' | 'render-bound' | 'unreferenced' | 'blocked-snapshot-key' | 'blocked-unresolvable-key'; + +type Finding = { + file: string; + line: number; + /** The variable the value is destructured into, or null when the call is not array-destructured. */ + valueName: string | null; + /** The key expression as written, e.g. `` `${ONYXKEYS.COLLECTION.POLICY}${policyID}` ``. */ + keySource: string; + /** The resolved `ONYXKEYS` access path, e.g. `COLLECTION.POLICY`, or null when it could not be resolved. */ + keyPath: string | null; + verdict: Verdict; + /** + * For a blocked subscription, the verdict it would carry if the key were readable. It answers what the + * Search snapshot ban costs, which is the number that changes when the snapshot redirect goes away. + */ + verdictIfUnblocked?: Verdict; + caveats: Caveat[]; + references: ReferenceSite[]; +}; + +/** Reads the Search snapshot key paths out of the runtime config, the same source the lint rule reads. */ +function resolveSnapshotKeyPaths(repoRoot: string): Set { + const source = fs.readFileSync(path.join(repoRoot, SNAPSHOT_KEYS_SOURCE), 'utf8'); + const declaration = SNAPSHOT_KEYS_DECLARATION.exec(source); + + if (!declaration) { + throw new Error(`Could not read SEARCH_SNAPSHOT_ONYX_KEYS from ${SNAPSHOT_KEYS_SOURCE}. Without it this scan would propose banned keys as candidates.`); + } + + return new Set([...declaration[1].matchAll(/ONYXKEYS\.([A-Z0-9_.]+)/g)].map((match) => match[1])); +} + +function collectFiles(inputs: readonly string[]): string[] { + const files: string[] = []; + + const visit = (target: string) => { + const stats = fs.statSync(target); + + if (stats.isFile()) { + if (FILE_EXTENSIONS.has(path.extname(target)) && !target.endsWith('.d.ts')) { + files.push(target); + } + return; + } + + for (const entry of fs.readdirSync(target, {withFileTypes: true})) { + visit(path.join(target, entry.name)); + } + }; + + for (const input of inputs) { + visit(input); + } + + return files.sort(); +} + +function getStaticName(node: Node | null | undefined, computed: boolean): string | null { + if (!computed && node?.type === 'Identifier') { + return node.name; + } + + return node?.type === 'StringLiteral' ? node.value : null; +} + +function getStaticPropertyName(node: Node): string | null { + return node.type === 'MemberExpression' ? getStaticName(node.property, node.computed) : null; +} + +function getCalleeName(callee: Node): string | null { + if (callee.type === 'Identifier') { + return callee.name; + } + + return callee.type === 'MemberExpression' ? getStaticPropertyName(callee) : null; +} + +/** Unwraps type-only wrappers and the leading interpolation of a collection member key template. */ +function unwrapKeyExpression(node: Node): Node { + // The TypeScript-only wrappers a key arrives in, all of which carry the real expression on `.expression`, + // e.g. `` `${ONYXKEYS.COLLECTION.REPORT}${id}` as const ``. + if ( + node.type === 'TSAsExpression' || + node.type === 'TSSatisfiesExpression' || + node.type === 'TSNonNullExpression' || + node.type === 'TSInstantiationExpression' || + node.type === 'TSTypeAssertion' + ) { + return unwrapKeyExpression(node.expression); + } + + if (node.type !== 'TemplateLiteral') { + return node; + } + + const leadingExpression = node.expressions.at(0); + + // cspell:disable-next-line -- quasis is the Babel name for the static chunks of a template literal + if (!leadingExpression || node.quasis.at(0)?.value.cooked !== '') { + return node; + } + + return unwrapKeyExpression(leadingExpression as Node); +} + +/** Resolves an identifier through a single `const` declaration, which is as far as a static read can be trusted. */ +function getConstInitializer(name: string, scopePath: NodePath): Node | null { + const binding = scopePath.scope.getBinding(name); + + if (!binding || binding.kind !== 'const' || binding.constantViolations.length > 0) { + return null; + } + + const declaration = binding.path; + + return declaration.node.type === 'VariableDeclarator' ? (declaration.node.init ?? null) : null; +} + +/** Turns `ONYXKEYS.COLLECTION.REPORT` into `COLLECTION.REPORT`, and returns null for anything not rooted at `ONYXKEYS`. */ +function getOnyxKeyPath(node: Node | null | undefined, scopePath: NodePath, seen = new Set()): string | null { + if (!node) { + return null; + } + + const segments: string[] = []; + let current: Node | null = unwrapKeyExpression(node); + + if (current.type === 'Identifier' && current.name !== ONYXKEYS_ROOT) { + if (seen.has(current.name)) { + return null; + } + + seen.add(current.name); + const initializer = getConstInitializer(current.name, scopePath); + + return initializer ? getOnyxKeyPath(initializer, scopePath, seen) : null; + } + + while (current?.type === 'MemberExpression') { + const propertyName = getStaticPropertyName(current); + + if (!propertyName) { + return null; + } + + segments.unshift(propertyName); + current = current.object; + } + + if (current?.type !== 'Identifier' || current.name !== ONYXKEYS_ROOT || segments.length === 0) { + return null; + } + + return segments.join('.'); +} + +function isHookName(name: string): boolean { + return /^use[A-Z0-9]/.test(name); +} + +function isComponentName(name: string): boolean { + return /^[A-Z]/.test(name); +} + +function getFunctionName(functionPath: NodePath): string | null { + const node = functionPath.node; + + if (node.type === 'FunctionDeclaration' && node.id) { + return node.id.name; + } + + const parent = functionPath.parent; + + if (parent.type === 'VariableDeclarator' && parent.id.type === 'Identifier') { + return parent.id.name; + } + + if (parent.type === 'ObjectProperty' && !parent.computed && parent.key.type === 'Identifier') { + return parent.key.name; + } + + return null; +} + +function returnsJSX(functionPath: NodePath): boolean { + const body = functionPath.node.body; + + if (body.type === 'JSXElement' || body.type === 'JSXFragment') { + return true; + } + + if (body.type !== 'BlockStatement') { + return false; + } + + return body.body.some((statement) => statement.type === 'ReturnStatement' && (statement.argument?.type === 'JSXElement' || statement.argument?.type === 'JSXFragment')); +} + +/** Whether an options object property sits in a call to something named like a hook. */ +function isHookOption(propertyPath: NodePath): boolean { + const objectPath = propertyPath.parentPath; + const callPath = objectPath?.parentPath; + + if (!callPath || callPath.node.type !== 'CallExpression' || !objectPath) { + return false; + } + + if (!callPath.node.arguments.some((argument) => argument === objectPath.node)) { + return false; + } + + const calleeName = getCalleeName(callPath.node.callee); + + return !!calleeName && isHookName(calleeName); +} + +/** + * How a function runs relative to render: `render` when React calls it while rendering, `synchronous` when + * the caller runs it in the same stretch and cannot await, `deferred` when it runs later on an event. + */ +function classifyFunctionBoundary(functionPath: NodePath): 'render' | 'synchronous' | 'deferred' { + const parentPath = functionPath.parentPath; + const parent = parentPath?.node; + + if ( + parent?.type === 'ObjectProperty' && + parent.value === functionPath.node && + RENDER_TIME_OPTION_NAMES.has(getStaticName(parent.key, parent.computed) ?? '') && + isHookOption(parentPath) + ) { + return 'render'; + } + + if (parent?.type === 'NewExpression' && parent.arguments.at(0) === functionPath.node && SYNCHRONOUS_EXECUTOR_NAMES.has(getCalleeName(parent.callee as Node) ?? '')) { + return 'synchronous'; + } + + if (parent?.type === 'CallExpression') { + if (parent.callee === functionPath.node) { + return 'synchronous'; + } + + const argumentIndex = parent.arguments.findIndex((argument) => argument === functionPath.node); + + if (argumentIndex >= 0) { + const calleeName = getCalleeName(parent.callee as Node); + + if (calleeName && COMPONENT_WRAPPER_NAMES.has(calleeName)) { + return 'render'; + } + + if (calleeName && RENDER_TIME_HOOK_ARGUMENTS.get(calleeName)?.has(argumentIndex)) { + return 'render'; + } + + if (parent.callee.type === 'MemberExpression' && calleeName && SYNCHRONOUS_CALLBACK_METHODS.has(calleeName)) { + return 'synchronous'; + } + + return 'deferred'; + } + } + + const functionName = getFunctionName(functionPath); + + if (functionName && (isHookName(functionName) || isComponentName(functionName))) { + return 'render'; + } + + return returnsJSX(functionPath) ? 'render' : 'deferred'; +} + +/** The name a reviewer would identify the reference site by: the prop, the hook, or the enclosing function. */ +function describeFunctionContext(functionPath: NodePath): string { + const parent = functionPath.parentPath?.node; + + if (parent?.type === 'JSXExpressionContainer' && functionPath.parentPath?.parentPath?.node.type === 'JSXAttribute') { + const attribute = functionPath.parentPath.parentPath.node; + return attribute.name.type === 'JSXIdentifier' ? `${attribute.name.name} prop` : 'JSX prop'; + } + + if (parent?.type === 'CallExpression') { + const calleeName = getCalleeName(parent.callee as Node); + + if (calleeName) { + return `${calleeName}()`; + } + } + + if (parent?.type === 'ObjectProperty' && parent.key.type === 'Identifier') { + return `${parent.key.name} prop`; + } + + return getFunctionName(functionPath) ?? 'anonymous callback'; +} + +/** Whether the array is the dependency list of a hook call, i.e. the last argument of a `use*` call. */ +function isDependencyArray(arrayPath: NodePath): boolean { + const callPath = arrayPath.parentPath; + + if (!callPath || callPath.node.type !== 'CallExpression') { + return false; + } + + const call = callPath.node; + const calleeName = getCalleeName(call.callee as Node); + + if (!calleeName || !isHookName(calleeName)) { + return false; + } + + return call.arguments.at(-1) === arrayPath.node; +} + +/** + * Classifies one reference to a subscribed value. Dependency-array membership is checked first: such a + * reference sits in the component body, so the position walk would call it render and hide the fact that the + * subscription is what re-runs the effect. + */ +function classifyReference(referencePath: NodePath): ReferenceSite { + const line = referencePath.node.loc?.start.line ?? 0; + let sawJSXExpression = false; + + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (current.node.type === 'ArrayExpression' && isDependencyArray(current)) { + const callee = current.parentPath?.node.type === 'CallExpression' ? getCalleeName(current.parentPath.node.callee as Node) : null; + return {kind: 'dependency-array', line, context: callee ? `${callee}() deps` : 'hook deps'}; + } + + if (current.node.type === 'JSXExpressionContainer') { + sawJSXExpression = true; + continue; + } + + if (!current.isFunction()) { + continue; + } + + const functionPath = current; + + if (sawJSXExpression) { + return {kind: 'render', line, context: 'JSX'}; + } + + const boundary = classifyFunctionBoundary(functionPath); + const context = describeFunctionContext(functionPath); + + if (boundary === 'deferred') { + return {kind: 'event', line, context}; + } + + return {kind: boundary, line, context}; + } + + return {kind: 'render', line, context: 'module scope'}; +} + +/** + * Reclassifies a reference by where the function holding it is actually called, one hop out. A value used + * inside a plain local helper looks like event-time code on its own, and stays event-time only while every + * call to that helper is. One call from render is enough to make the read a render read, and the position + * walk cannot see it because it stops at the helper's own boundary. + * + * One hop is deliberate: a helper called by a helper is left to the reviewer, and the checklist says so. + */ +function classifyByCallSites(referencePath: NodePath, site: ReferenceSite): ReferenceSite { + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (!current.isFunction()) { + continue; + } + + const functionPath = current; + const name = getFunctionName(functionPath); + const declaration = functionPath.node.type === 'FunctionDeclaration' ? functionPath : functionPath.parentPath; + + if (!name || declaration?.node.type === 'ObjectProperty') { + return site; + } + + const binding = functionPath.scope.getBinding(name); + + for (const usage of binding?.referencePaths ?? []) { + if (!usage.isIdentifier() || usage.parentPath?.node.type !== 'CallExpression' || usage.parentPath.node.callee !== usage.node) { + continue; + } + + const callSite = classifyReference(usage); + + if (callSite.kind === 'render' || callSite.kind === 'dependency-array') { + return {kind: callSite.kind, line: site.line, context: `${site.context} called from ${callSite.context}`}; + } + } + + return site; + } + + return site; +} + +/** + * Whether the callback holding the reference is handed to a component as a prop that is not named for an + * event. A child is free to call such a prop during its own render, which would put the read back on the + * render path from a different file. + */ +function isPassedAsNonEventProp(referencePath: NodePath): boolean { + const isEventPropName = (name: string) => EVENT_CALLBACK_NAME.test(name); + + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (!current.isFunction()) { + continue; + } + + const parent = current.parentPath; + + if (parent?.node.type === 'JSXExpressionContainer' && parent.parentPath?.node.type === 'JSXAttribute') { + const attributeName = parent.parentPath.node.name; + return attributeName.type === 'JSXIdentifier' && !isEventPropName(attributeName.name); + } + + if (parent?.node.type === 'VariableDeclarator' && parent.node.id.type === 'Identifier') { + const binding = parent.scope.getBinding(parent.node.id.name); + + return !!binding?.referencePaths.some((usage) => { + const container = usage.parentPath; + + if (container?.node.type !== 'JSXExpressionContainer' || container.parentPath?.node.type !== 'JSXAttribute') { + return false; + } + + const attributeName = container.parentPath.node.name; + return attributeName.type === 'JSXIdentifier' && !isEventPropName(attributeName.name); + }); + } + + return false; + } + + return false; +} + +/** + * Names that appear in a dependency array anywhere in the file. A memoized callback holding the read whose + * own name shows up here is a link in a trigger chain: dropping the subscription makes the callback stable + * and whatever depends on it stops re-running. + */ +function collectDependencyArrayNames(programPath: NodePath): Set { + const names = new Set(); + + programPath.traverse({ + ArrayExpression(arrayPath) { + if (!isDependencyArray(arrayPath)) { + return; + } + + for (const element of arrayPath.node.elements) { + if (element?.type === 'Identifier') { + names.add(element.name); + } + } + }, + }); + + return names; +} + +/** The variable a memoized callback holding the reference is bound to, if any. */ +function getEnclosingMemoizedName(referencePath: NodePath): string | null { + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (!current.isFunction()) { + continue; + } + + const declarator = current.parentPath?.node.type === 'CallExpression' ? current.parentPath.parentPath : current.parentPath; + + if (declarator?.node.type === 'VariableDeclarator' && declarator.node.id.type === 'Identifier') { + return declarator.node.id.name; + } + + return null; + } + + return null; +} + +/** + * Whether the value is handed to a state setter or written into a ref inside the callback. The value then + * reaches render through the store rather than directly, and the screen shows the moment of the read. + */ +function isParkedWhereRenderReads(referencePath: NodePath): boolean { + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (current.isFunction()) { + return false; + } + + if (current.node.type === 'AssignmentExpression' && current.node.left.type === 'MemberExpression' && getStaticPropertyName(current.node.left) === 'current') { + return true; + } + + if (current.node.type !== 'CallExpression') { + continue; + } + + const calleeName = getCalleeName(current.node.callee as Node); + + if (calleeName && /^set[A-Z]/.test(calleeName)) { + return true; + } + } + + return false; +} + +/** Whether the reference is the value of a prop or an object field named for an event, e.g. `onPress={handler}`. */ +function isEventPropValue(nodePath: NodePath): boolean { + const parent = nodePath.parentPath; + + if (parent?.node.type === 'ObjectProperty' && parent.node.key.type === 'Identifier') { + return EVENT_CALLBACK_NAME.test(parent.node.key.name); + } + + if (parent?.node.type !== 'JSXExpressionContainer' || parent.parentPath?.node.type !== 'JSXAttribute') { + return false; + } + + const attributeName = parent.parentPath.node.name; + + return attributeName.type === 'JSXIdentifier' && EVENT_CALLBACK_NAME.test(attributeName.name); +} + +/** Whether a `return` hands its value out of a component or a hook, rather than out of a local callback. */ +function isReturnFromComponentOrHook(returnPath: NodePath): boolean { + const owner = returnPath.getFunctionParent(); + + if (!owner) { + return false; + } + + const ownerName = getFunctionName(owner); + + return (!!ownerName && (isHookName(ownerName) || isComponentName(ownerName))) || returnsJSX(owner); +} + +/** Whether the path sits inside a value a component or a hook returns. */ +function isReturnedOutOfComponent(nodePath: NodePath): boolean { + for (let current: NodePath | null = nodePath; current; current = current.parentPath) { + if (current.node.type === 'ReturnStatement') { + return isReturnFromComponentOrHook(current); + } + } + + return false; +} + +/** + * Whether the callback holding the reference leaves the component or the hook through its return value, + * which puts the decision about when it runs in a file this scan cannot see. Every caller has to be checked + * by hand. A callback returned out of a local helper does not count: it has not left the component yet, and + * the walk keeps going until it reaches the component or hook boundary. + */ +function escapesViaReturn(referencePath: NodePath): boolean { + for (let current: NodePath | null = referencePath.parentPath; current; current = current.parentPath) { + if (!current.isFunction()) { + continue; + } + + const parent = current.parentPath; + const boundName = parent?.node.type === 'ObjectProperty' && parent.node.key.type === 'Identifier' ? parent.node.key.name : getFunctionName(current); + + // A callback named for an event is attached to one by whoever receives it, which is the shape the + // philosophy allows. Only a callback the receiver is free to call in its own body is worth flagging. + if (boundName && EVENT_CALLBACK_NAME.test(boundName)) { + return false; + } + + if (isReturnedOutOfComponent(current)) { + return true; + } + + const declarator = parent?.node.type === 'CallExpression' ? parent.parentPath : parent; + + if (declarator?.node.type !== 'VariableDeclarator' || declarator.node.id.type !== 'Identifier') { + return false; + } + + const binding = declarator.scope.getBinding(declarator.node.id.name); + + return !!binding?.referencePaths.some((usage) => !isEventPropValue(usage) && isReturnedOutOfComponent(usage)); + } + + return false; +} + +function getOptionsObject(call: CallExpression): ObjectExpression | null { + const optionsArgument = call.arguments.at(1); + + return optionsArgument?.type === 'ObjectExpression' ? optionsArgument : null; +} + +function getOptionNames(options: ObjectExpression | null): Set { + const names = new Set(); + + for (const property of options?.properties ?? []) { + if (property.type === 'ObjectProperty') { + const name = getStaticName(property.key, property.computed); + + if (name) { + names.add(name); + } + } + } + + return names; +} + +/** Where the value is used, independent of whether the key is readable at all. */ +function classifyByUse(finding: Omit): Verdict { + if (finding.references.some((reference) => reference.kind === 'render')) { + return 'render-bound'; + } + + if (finding.references.length === 0) { + return 'unreferenced'; + } + + if (finding.caveats.some((caveat) => BLOCKING_CAVEATS.has(caveat))) { + return 'needs-decision'; + } + + return finding.caveats.length === 0 ? 'drop-in' : 'needs-fix'; +} + +/** + * The verdict, plus what the verdict would have been without the ban. A blocked key is reported as blocked + * whatever its uses look like, since the lint rule refuses it either way. + */ +function decideVerdict(finding: Omit, isSnapshotKey: boolean): Pick { + const byUse = classifyByUse(finding); + + if (!finding.keyPath) { + return {verdict: 'blocked-unresolvable-key', verdictIfUnblocked: byUse}; + } + + return isSnapshotKey ? {verdict: 'blocked-snapshot-key', verdictIfUnblocked: byUse} : {verdict: byUse}; +} + +function analyzeFile(file: string, source: string, snapshotKeyPaths: Set): Finding[] { + const ast = parse(source, { + sourceType: 'module', + errorRecovery: true, + plugins: ['typescript', 'jsx', 'decorators-legacy', 'classProperties', 'optionalChaining', 'nullishCoalescingOperator', 'explicitResourceManagement'], + }); + + const findings: Finding[] = []; + let dependencyArrayNames: Set | null = null; + + traverse(ast, { + Program(programPath) { + dependencyArrayNames = collectDependencyArrayNames(programPath); + }, + CallExpression(callPath) { + const call = callPath.node; + + if (getCalleeName(call.callee as Node) !== HOOK_NAME) { + return; + } + + const keyArgument = call.arguments.at(0) as Node | undefined; + const keyPath = getOnyxKeyPath(keyArgument, callPath); + const keySource = keyArgument?.start != null && keyArgument.end != null ? source.slice(keyArgument.start, keyArgument.end) : ''; + const line = call.loc?.start.line ?? 0; + + const optionNames = getOptionNames(getOptionsObject(call)); + const caveats = new Set(); + + if (optionNames.has('selector')) { + caveats.add('has-selector'); + } + + for (const option of optionNames) { + if (SHAPE_CHANGING_OPTIONS.has(option)) { + caveats.add('shape-changing-options'); + } + } + + if (keyPath?.startsWith('DERIVED.')) { + caveats.add('derived-key'); + } + + if (keyPath?.startsWith('COLLECTION.')) { + caveats.add('collection-key'); + } + + // The ban is unconditional, so the uses are still read: they say what lifting it would be worth. + const isSnapshotKey = !!keyPath && snapshotKeyPaths.has(keyPath); + const pushFinding = (partial: Omit) => findings.push({...partial, ...decideVerdict(partial, isSnapshotKey)}); + + const declaratorPath = callPath.parentPath; + + if (declaratorPath?.node.type !== 'VariableDeclarator') { + caveats.add('not-array-destructured'); + pushFinding({file, line, valueName: null, keySource, keyPath, caveats: [...caveats], references: []}); + return; + } + + const pattern = declaratorPath.node.id; + + if (pattern.type !== 'ArrayPattern') { + caveats.add('not-array-destructured'); + pushFinding({file, line, valueName: null, keySource, keyPath, caveats: [...caveats], references: []}); + return; + } + + const valueElement = pattern.elements.at(0); + const metadataElement = pattern.elements.at(1); + const valueIdentifier = valueElement?.type === 'AssignmentPattern' ? valueElement.left : valueElement; + + if (valueElement?.type === 'AssignmentPattern') { + caveats.add('has-default-value'); + } + + if (metadataElement?.type === 'Identifier' && (declaratorPath.scope.getBinding(metadataElement.name)?.referencePaths.length ?? 0) > 0) { + caveats.add('uses-metadata'); + } + + if (valueIdentifier?.type !== 'Identifier') { + caveats.add('not-array-destructured'); + pushFinding({file, line, valueName: null, keySource, keyPath, caveats: [...caveats], references: []}); + return; + } + + const binding = declaratorPath.scope.getBinding(valueIdentifier.name); + const references: ReferenceSite[] = []; + + for (const referencePath of binding?.referencePaths ?? []) { + if (!referencePath.isIdentifier()) { + continue; + } + + const identifierPath = referencePath; + const directSite = classifyReference(identifierPath); + const site = directSite.kind === 'event' ? classifyByCallSites(identifierPath, directSite) : directSite; + references.push(site); + + if (site.kind === 'dependency-array') { + caveats.add('in-dependency-array'); + } + + if (site.kind === 'synchronous') { + caveats.add('synchronous-reference'); + } + + if (site.kind === 'event') { + if (isPassedAsNonEventProp(identifierPath)) { + caveats.add('non-event-prop'); + } + + if (isParkedWhereRenderReads(identifierPath)) { + caveats.add('parked-in-state'); + } + + if (escapesViaReturn(identifierPath)) { + caveats.add('escapes-via-return'); + } + + const memoizedName = getEnclosingMemoizedName(identifierPath); + + if (memoizedName && dependencyArrayNames?.has(memoizedName)) { + caveats.add('trigger-chain'); + } + } + } + + if (dependencyArrayNames?.has(valueIdentifier.name)) { + caveats.add('in-dependency-array'); + } + + pushFinding({file, line, valueName: valueIdentifier.name, keySource, keyPath, caveats: [...caveats], references}); + }, + }); + + return findings; +} + +const CAVEAT_EXPLANATIONS = new Map([ + ['has-selector', 'the subscription projects the value through a selector, which must be reapplied to the read result'], + ['has-default-value', 'the destructure supplies a default, which becomes a `??` at the read site'], + ['derived-key', 'a DERIVED key: it and its sources are a revision apart in a tick that wrote either'], + ['collection-key', 'a collection key: the read returns every member, and resolves frozen'], + ['synchronous-reference', 'the value is used in a callback the caller runs synchronously, so awaiting a read needs the caller restructured'], + ['non-event-prop', 'the callback holding the use is passed as a prop not named for an event, so a child may call it during render'], + ['parked-in-state', 'the value is handed to a state setter or written into a ref, so it reaches render indirectly'], + ['escapes-via-return', 'the callback holding the use is returned out of the component or hook, so its callers have to be checked by hand'], + ['shape-changing-options', 'the subscription passes options that change what it returns compared to a plain read'], + ['uses-metadata', 'the subscription result status is used, and a read has no status'], + ['in-dependency-array', 'the value is in a hook dependency array, so the subscription may exist to trigger work rather than to supply a value'], + ['trigger-chain', 'the value feeds a memoized callback that something else depends on, which is a trigger chain'], + ['not-array-destructured', 'the call result is not destructured into a value binding, so its uses could not be followed'], +]); + +const VERDICT_TITLES = new Map([ + ['drop-in', 'Drop-in'], + ['needs-fix', 'Needs a fix'], + ['needs-decision', 'Needs a decision'], + ['render-bound', 'Render-bound (not convertible)'], + ['unreferenced', 'Value never referenced'], + ['blocked-snapshot-key', 'Blocked: Search snapshot key'], + ['blocked-unresolvable-key', 'Blocked: key not statically resolvable'], +]); + +function formatReferences(references: readonly ReferenceSite[]): string { + if (references.length === 0) { + return 'no uses'; + } + + // The same line and context can appear more than once when a handler reads the value repeatedly, and the + // repetition says nothing a reviewer needs. + return [...new Set(references.map((reference) => `L${reference.line} ${reference.context}`))].join(', '); +} + +function groupByKey(findings: readonly Finding[]): Map { + const grouped = new Map(); + + for (const finding of findings) { + const key = finding.keyPath ? `${ONYXKEYS_ROOT}.${finding.keyPath}` : ''; + const group = grouped.get(key) ?? []; + group.push(finding); + grouped.set(key, group); + } + + return new Map([...grouped.entries()].sort((left, right) => right[1].length - left[1].length || left[0].localeCompare(right[0]))); +} + +function renderCandidateSection(title: string, description: string, findings: readonly Finding[]): string { + const lines = [`## ${title}`, '', description, '', `${findings.length} subscription${findings.length === 1 ? '' : 's'}.`, '']; + + if (findings.length === 0) { + return `${lines.join('\n')}\n`; + } + + for (const [key, group] of groupByKey(findings)) { + lines.push(`### \`${key}\` — ${group.length}`, ''); + lines.push('| Site | Value | Uses | Caveats |', '| --- | --- | --- | --- |'); + + for (const finding of group) { + const caveats = finding.caveats.length > 0 ? finding.caveats.map((caveat) => `\`${caveat}\``).join(' ') : '—'; + lines.push(`| \`${finding.file}:${finding.line}\` | \`${finding.valueName ?? '—'}\` | ${formatReferences(finding.references)} | ${caveats} |`); + } + + lines.push(''); + } + + return `${lines.join('\n')}\n`; +} + +function renderReport(findings: readonly Finding[], inputs: readonly string[], snapshotKeyPaths: Set, fileCount: number): string { + const byVerdict = (verdict: Verdict) => findings.filter((finding) => finding.verdict === verdict); + const blocked = findings.filter((finding) => finding.verdict === 'blocked-snapshot-key' || finding.verdict === 'blocked-unresolvable-key'); + const blockedButConvertible = blocked.filter((finding) => !!finding.verdictIfUnblocked && CONVERTIBLE_VERDICTS.has(finding.verdictIfUnblocked)); + const convertible = findings.filter((finding) => CONVERTIBLE_VERDICTS.has(finding.verdict)); + const dropIn = byVerdict('drop-in'); + const needsFix = byVerdict('needs-fix'); + const needsDecision = byVerdict('needs-decision'); + + const sections: string[] = []; + + sections.push( + [ + '# `Onyx.get()` migration candidates', + '', + `Generated by \`bun scripts/onyxGetMigrationCandidates.ts ${inputs.join(' ')}\`.`, + '', + `Scanned ${fileCount} files and found ${findings.length} \`useOnyx()\` subscriptions.`, + '', + '## What the scan decides, and what it does not', + '', + 'A `useOnyx()` subscription can become an `await Onyx.get()` read only when nothing about the value reaches render.', + 'This scan resolves the `ONYXKEYS` path of every subscription, follows every reference to the destructured value, and', + 'classifies the position each reference sits in with the same render/event model as `rulesdir/no-unsafe-onyx-read`.', + '', + 'Decided mechanically:', + '', + '- **Search snapshot keys are out.** `@hooks/useOnyx` rewrites these keys to `snapshot_` inside a `SearchScopeProvider` subtree, so a read would swap snapshot data for live data. The lint rule fails the build on them, and they are excluded here rather than proposed.', + '- **Keys that cannot be resolved statically are out.** The rule refuses them too, since a caller could route a snapshot key through a variable or a helper.', + '- **A value referenced anywhere render can reach is out.** JSX, the component body, a `selector`, a `useMemo` factory, a `useState` initializer, or a `memo`/`forwardRef` wrapper.', + '', + 'Left to the reviewer, per candidate, as a caveat column, and for every candidate as the checklist at the end.', + '', + `Search snapshot keys read from \`${SNAPSHOT_KEYS_SOURCE}\`: ${[...snapshotKeyPaths].map((key) => `\`${ONYXKEYS_ROOT}.${key}\``).join(', ')}.`, + '', + '## Summary', + '', + 'Only the three convertible verdicts are listed after this table. The rest are counted here and nowhere else, since nothing about them is a work item.', + '', + '| Verdict | Subscriptions | Meaning |', + '| --- | --- | --- |', + `| Drop-in | ${dropIn.length} | every use is at event time, and no caveat applies |`, + `| Needs a fix | ${needsFix.length} | every use is at event time, with a caveat that has a known fix |`, + `| Needs a decision | ${needsDecision.length} | every use is at event time, but a caveat has to be settled by reading the code |`, + `| Render-bound | ${byVerdict('render-bound').length} | the value reaches render, so the subscription stays |`, + `| Value never referenced | ${byVerdict('unreferenced').length} | nothing reads the value, so the subscription is either vestigial or a trigger |`, + `| Blocked: snapshot key | ${byVerdict('blocked-snapshot-key').length} | banned by \`rulesdir/no-unsafe-onyx-read\` |`, + `| Blocked: unresolvable key | ${byVerdict('blocked-unresolvable-key').length} | banned by \`rulesdir/no-unsafe-onyx-read\` |`, + '', + `Of the ${blocked.length} blocked subscriptions, ${blockedButConvertible.length} would be candidates if the key were readable, so that is what the Search snapshot ban and the unresolvable keys cost today. Counting them, ${convertible.length + blockedButConvertible.length} of the ${findings.length} subscriptions never reach render.`, + '', + ].join('\n'), + ); + + sections.push( + renderCandidateSection( + 'Drop-in candidates', + 'Every reference to the value sits in a function that runs on an event, the key is neither a snapshot key nor unresolvable, and no caveat applies. Convert the read where it is used, and work through the checklist at the end before opening the PR.', + dropIn, + ), + ); + + sections.push( + renderCandidateSection( + 'Needs a fix', + 'Every reference is at event time, but something about the subscription changes at the read site. Each caveat has a known fix; the caveat names are explained below the tables.', + needsFix, + ), + ); + + sections.push( + renderCandidateSection( + 'Needs a decision', + 'Every reference is at event time, but the subscription carries something a static pass cannot settle: a status the read does not have, or a dependency-array use that may mean the subscription exists to trigger work rather than to supply a value.', + needsDecision, + ), + ); + + sections.push( + [ + '## Caveat reference', + '', + '| Caveat | What it means |', + '| --- | --- |', + ...[...CAVEAT_EXPLANATIONS].map(([caveat, explanation]) => `| \`${caveat}\` | ${explanation} |`), + '', + '## Manual checklist for every conversion', + '', + 'These are the hazards no static pass catches. They come from `contributingGuides/philosophies/ONYX-DATA-MANAGEMENT.md`.', + '', + '1. **Source or trigger?** A subscription that exists to re-run work when a key changes stops doing that when it becomes a read. Follow the value through `useCallback` wrappers and dependency arrays before deleting it.', + '2. **Reads before the first write in the tick.** `Onyx.get()` samples the cache when it is called, so a read after a write in the same synchronous stretch resolves to the pre-write value. Awaiting the read does not fix it; await the write, or read first.', + '3. **One read block per synchronous stretch.** Code after an `await` or a `runAfterTransitions` is meant to see the earlier writes, so do not hoist its read above the deferral.', + '4. **Do not mutate the result.** A single-key read resolves to the cached object itself, so assigning to a property writes the cache with no subscriber told.', + '5. **Was the value on screen?** Conversion moves the sample from the last render to the moment the handler runs. A dialog confirming an amount must act on the amount it displayed.', + '6. **Check every caller.** A read placed in a shared function becomes a render read the moment a component or hook calls that function. One render call site anywhere in `src/` settles it. This scan follows calls one hop inside the same file, so a helper called by a helper, or from another file, is still yours to check.', + '', + ].join('\n'), + ); + + return sections.join('\n'); +} + +/** + * A conversion is written per file, so a file is the unit a PR can own: two PRs touching the same file would + * conflict in the same callback. Files are therefore assigned to exactly one PR, and grouped by the work the + * conversion actually takes, which is what the caveats already say. + */ +type PullRequestBucket = { + id: string; + title: string; + /** What the conversion involves for every file in the bucket. */ + change: string; + /** What has to be exercised before the PR is trusted, beyond the shared checks. */ + test: readonly string[]; +}; + +/** + * Ordered from the conversion that is purely mechanical to the one that changes control flow. A file lands in + * the bucket of its hardest subscription, so a PR never mixes a one-line change with a restructure. + */ +const PULL_REQUEST_BUCKETS: readonly PullRequestBucket[] = [ + { + id: 'mechanical', + title: 'Mechanical reads', + change: 'Delete the `useOnyx` line and `await Onyx.get(key)` inside the handler that uses it. A destructuring default becomes a `??` at the read site. Nothing else about the file changes.', + test: [ + 'Exercise each handler listed in the Uses column and confirm it acts on the same value it did before.', + 'Repeat once offline, so the read is served from a hydrated cache with no network in flight.', + ], + }, + { + id: 'selector', + title: 'Reads with a selector to reapply', + change: 'Move the read into the handler and call the same selector on the result: `selector(await Onyx.get(key))`. `Onyx.get` returns the stored value, so dropping the selector compiles and silently changes the shape.', + test: [ + 'Assert the projected shape at the read site, not the stored one: a selector that returned a boolean must still yield a boolean.', + 'Exercise each handler listed in the Uses column.', + ], + }, + { + id: 'collection', + title: 'Collection and derived reads', + change: 'Move the read into the handler. A collection key resolves to every member, exactly as the subscription did, and resolves frozen. A `DERIVED` key needs the tick checked: the source and the derived value are a revision apart in a tick that wrote either.', + test: [ + 'Confirm nothing mutates the result. A collection read throws on mutation; a single-key read would write the cache silently.', + 'For a `DERIVED` key, add or extend a test that writes the source and then reads both, asserting the post-write value.', + ], + }, + { + id: 'caller-audit', + title: 'Reads behind a callback that leaves the file', + change: 'Move the read into the handler, then check the receiving side: the callback is returned out of the hook, passed as a prop not named for an event, or its value is parked in state or a ref. Every consumer has to attach it to an event rather than call it while rendering.', + test: [ + 'Open each consumer of the returned callback or prop and confirm it is attached to an event, never invoked in a body, in JSX, or in a `useMemo`.', + 'Where the value was parked in state or a ref, confirm the screen still shows what the user acted on.', + ], + }, + { + id: 'async-restructure', + title: 'Reads inside synchronous callbacks', + change: 'The use sits in a callback its caller runs synchronously (a `map`, a `filter`, a `Promise` executor), so the read cannot simply be awaited in place. Hoist the read above the synchronous stretch, or make the caller await. Take this bucket last.', + test: [ + 'Confirm the read happens before the first write in the same synchronous stretch, and that each deferral does its own read.', + 'Cover the reordering with a test that asserts the post-write value, since ordering is what this bucket changes.', + ], + }, +]; + +/** Caveats that decide the bucket, hardest first. The first match wins. */ +const BUCKET_BY_CAVEAT = new Map([ + ['synchronous-reference', 'async-restructure'], + ['escapes-via-return', 'caller-audit'], + ['non-event-prop', 'caller-audit'], + ['parked-in-state', 'caller-audit'], + ['collection-key', 'collection'], + ['derived-key', 'collection'], + ['has-selector', 'selector'], +]); + +/** What a plan covers: which verdicts it converts, and what the report says about the ones it leaves out. */ +type PlanScope = { + verdicts: readonly Verdict[]; + /** How the covered set is described in the first line. */ + label: string; + /** What the plan says about the candidates it excludes, and what excluding them costs. */ + exclusionNote: string; +}; + +/** The two scopes a plan is written for: everything convertible in one sweep, or the drop-in tier alone. */ +const PLAN_SCOPES = { + full: { + verdicts: ['drop-in', 'needs-fix'], + label: 'drop-in and needs-a-fix candidates', + exclusionNote: 'The needs-a-decision candidates are deliberately absent. Each of them turns on a question a reviewer has to answer first, so they cannot be batched by shape.', + }, + dropInOnly: { + verdicts: ['drop-in'], + label: 'drop-in candidates only', + exclusionNote: + 'This is the narrow scope: only subscriptions with no caveat at all. Every conversion here is the same one-line move, which makes the wave uniform to review and to test. The cost is that most of these files also hold needs-a-fix subscriptions, so the next wave reopens them later; each PR below states how many of its files come back.', + }, +} as const satisfies Record; + +/** How large a PR may get before it is split, in files and in subscriptions. */ +const MAX_FILES_PER_PULL_REQUEST = 10; +const MAX_SUBSCRIPTIONS_PER_PULL_REQUEST = 24; + +/** An area smaller than this is folded into its bucket's mixed PR rather than getting one of its own. */ +const MIN_FILES_PER_AREA = 3; + +type PullRequest = { + bucket: PullRequestBucket; + area: string; + part: number; + partCount: number; + files: Array<{file: string; findings: Finding[]}>; +}; + +function getBucketId(caveats: readonly Caveat[]): string { + for (const [caveat, bucketId] of BUCKET_BY_CAVEAT) { + if (caveats.includes(caveat)) { + return bucketId; + } + } + + return 'mechanical'; +} + +/** The feature area a file belongs to, at the depth the codebase groups reviewers by. */ +function getArea(file: string): string { + return path.dirname(file).split('/').slice(0, 3).join('/'); +} + +function groupFindingsByFile(findings: readonly Finding[]): Map { + const byFile = new Map(); + + for (const finding of findings) { + const group = byFile.get(finding.file) ?? []; + group.push(finding); + byFile.set(finding.file, group); + } + + return byFile; +} + +/** Splits a bucket's files into PRs of a reviewable size, keeping each area together where it is big enough. */ +function buildPullRequests(findings: readonly Finding[]): PullRequest[] { + const byFile = groupFindingsByFile(findings); + const byBucket = new Map>(); + + for (const [file, fileFindings] of byFile) { + const bucketId = fileFindings + .map((finding) => getBucketId(finding.caveats)) + .reduce((hardest, current) => { + const order = PULL_REQUEST_BUCKETS.map((bucket) => bucket.id); + return order.indexOf(current) > order.indexOf(hardest) ? current : hardest; + }, 'mechanical'); + + const group = byBucket.get(bucketId) ?? []; + group.push({file, findings: fileFindings}); + byBucket.set(bucketId, group); + } + + const pullRequests: PullRequest[] = []; + + for (const bucket of PULL_REQUEST_BUCKETS) { + const bucketFiles = byBucket.get(bucket.id) ?? []; + const byArea = new Map>(); + + for (const entry of bucketFiles) { + const area = getArea(entry.file); + const group = byArea.get(area) ?? []; + group.push(entry); + byArea.set(area, group); + } + + const mixed: Array<{file: string; findings: Finding[]}> = []; + const areas: Array<[string, Array<{file: string; findings: Finding[]}>]> = []; + + for (const [area, entries] of [...byArea.entries()].sort((left, right) => right[1].length - left[1].length || left[0].localeCompare(right[0]))) { + if (entries.length < MIN_FILES_PER_AREA) { + mixed.push(...entries); + continue; + } + + areas.push([area, entries]); + } + + if (mixed.length > 0) { + areas.push(['mixed areas', mixed.sort((left, right) => left.file.localeCompare(right.file))]); + } + + for (const [area, entries] of areas) { + const batches: Array> = [[]]; + + for (const entry of entries.sort((left, right) => left.file.localeCompare(right.file))) { + const current = batches.at(-1) ?? []; + const subscriptions = current.reduce((total, batched) => total + batched.findings.length, 0); + + if (current.length >= MAX_FILES_PER_PULL_REQUEST || (current.length > 0 && subscriptions + entry.findings.length > MAX_SUBSCRIPTIONS_PER_PULL_REQUEST)) { + batches.push([entry]); + continue; + } + + current.push(entry); + } + + for (const [index, batch] of batches.entries()) { + if (batch.length === 0) { + continue; + } + + pullRequests.push({bucket, area, part: index + 1, partCount: batches.length, files: batch}); + } + } + } + + return pullRequests; +} + +function countSubscriptions(pullRequest: PullRequest): number { + return pullRequest.files.reduce((total, entry) => total + entry.findings.length, 0); +} + +function getKeyPaths(pullRequest: PullRequest): string[] { + const keys = new Set(); + + for (const entry of pullRequest.files) { + for (const finding of entry.findings) { + keys.add(`${ONYXKEYS_ROOT}.${finding.keyPath ?? ''}`); + } + } + + return [...keys].sort(); +} + +function renderPullRequestPlan(findings: readonly Finding[], scope: PlanScope): string { + const candidates = findings.filter((finding) => scope.verdicts.includes(finding.verdict)); + const pullRequests = buildPullRequests(candidates); + const fileCount = groupFindingsByFile(candidates).size; + + // A file left with convertible subscriptions this plan does not cover comes back in a later PR, which is + // the cost of a narrower scope and is worth stating per PR rather than discovering during review. + const bucketCount = new Set(pullRequests.map((pullRequest) => pullRequest.bucket.id)).size; + const revisitedFiles = new Set(findings.filter((finding) => CONVERTIBLE_VERDICTS.has(finding.verdict) && !scope.verdicts.includes(finding.verdict)).map((finding) => finding.file)); + + const lines = [ + '# `Onyx.get()` migration: PR map', + '', + `Generated by \`bun scripts/onyxGetMigrationCandidates.ts\` from the ${scope.label}: ${candidates.length} subscriptions across ${fileCount} files, split into ${pullRequests.length} PRs.`, + '', + scope.exclusionNote, + '', + '## How the split works', + '', + `- **A PR owns whole files.** A conversion is written inside the callback that uses the value, so two PRs touching one file would conflict. Every file below appears in exactly one PR.`, + ...(bucketCount > 1 + ? [ + '- **A PR owns one kind of work.** A file lands in the bucket of its hardest subscription, so a PR is either all one-line moves or all restructures, never both. That is what makes the test plan per PR short.', + ] + : ['- **Every PR is the same conversion.** This scope holds one kind of work, so the change and the test steps below are identical in each PR.']), + `- **A PR stays inside one feature area** where the area has at least ${MIN_FILES_PER_AREA} files, so one reviewer can own it. Smaller areas are collected into the mixed PR.`, + `- **A PR is capped** at ${MAX_FILES_PER_PULL_REQUEST} files and ${MAX_SUBSCRIPTIONS_PER_PULL_REQUEST} subscriptions, and split into parts beyond that.`, + ...(bucketCount > 1 ? ['- **Waves run in bucket order.** Each wave is more invasive than the last, so land the earlier ones first and let them sit before starting the next.'] : []), + '', + bucketCount > 1 ? '## Waves' : '## Size', + '', + '| Wave | Bucket | PRs | Files | Subscriptions |', + '| --- | --- | --- | --- | --- |', + ]; + + let wave = 0; + + for (const bucket of PULL_REQUEST_BUCKETS) { + const bucketPullRequests = pullRequests.filter((pullRequest) => pullRequest.bucket.id === bucket.id); + + if (bucketPullRequests.length === 0) { + continue; + } + + wave++; + const files = bucketPullRequests.reduce((total, pullRequest) => total + pullRequest.files.length, 0); + const subscriptions = bucketPullRequests.reduce((total, pullRequest) => total + countSubscriptions(pullRequest), 0); + lines.push(`| ${wave} | ${bucket.title} | ${bucketPullRequests.length} | ${files} | ${subscriptions} |`); + } + + const pilot = pullRequests + .filter((pullRequest) => pullRequest.bucket.id === 'mechanical') + .sort((left, right) => countSubscriptions(left) - countSubscriptions(right)) + .at(0); + + lines.push( + '', + '## Shared checks, every PR', + '', + '1. `npm run lint-changed` — `rulesdir/no-unsafe-onyx-read` is the gate that refuses a render read, a module-scope read, and a Search snapshot key.', + '2. `npm run typecheck`.', + '3. Run the existing Jest tests that touch the changed files.', + '4. Walk the manual checklist in `.context/onyx-get-migration-candidates.md`: trigger vs source, read before the first write in the tick, no mutation of the result, and whether the value was on screen.', + '', + ); + + if (pilot) { + lines.push( + '## Start here', + '', + `PR ${pullRequests.indexOf(pilot) + 1} (\`${pilot.bucket.title}: ${pilot.area}\`) is the smallest mechanical batch: ${pilot.files.length} files, ${countSubscriptions(pilot)} subscriptions. Land it first to settle the review shape, the PR description template, and the test steps before the bigger batches follow.`, + '', + ); + } + + for (const [index, pullRequest] of pullRequests.entries()) { + const part = pullRequest.partCount > 1 ? ` (part ${pullRequest.part} of ${pullRequest.partCount})` : ''; + const revisited = pullRequest.files.filter((entry) => revisitedFiles.has(entry.file)).length; + + lines.push( + `## PR ${index + 1} — ${pullRequest.bucket.title}: \`${pullRequest.area}\`${part}`, + '', + `${pullRequest.files.length} file${pullRequest.files.length === 1 ? '' : 's'}, ${countSubscriptions(pullRequest)} subscription${countSubscriptions(pullRequest) === 1 ? '' : 's'}. Keys: ${getKeyPaths( + pullRequest, + ) + .map((key) => `\`${key}\``) + .join(', ')}.`, + '', + `**Change.** ${pullRequest.bucket.change}`, + '', + ); + + if (revisited > 0) { + lines.push(`**Comes back.** ${revisited} of these ${pullRequest.files.length} files hold convertible subscriptions this plan leaves behind, so a later PR opens them again.`, ''); + } + + lines.push('**Test.**', ''); + + for (const step of pullRequest.bucket.test) { + lines.push(`- ${step}`); + } + + lines.push('', '| Site | Value | Key | Uses | Caveats |', '| --- | --- | --- | --- | --- |'); + + for (const entry of pullRequest.files) { + for (const finding of entry.findings) { + const caveats = finding.caveats.length > 0 ? finding.caveats.map((caveat) => `\`${caveat}\``).join(' ') : '—'; + lines.push( + `| \`${finding.file}:${finding.line}\` | \`${finding.valueName ?? '—'}\` | \`${ONYXKEYS_ROOT}.${finding.keyPath ?? ''}\` | ${formatReferences(finding.references)} | ${caveats} |`, + ); + } + } + + lines.push(''); + } + + return `${lines.join('\n')}\n`; +} + +function parseArguments(argv: readonly string[]): {inputs: string[]; output: string; json: string | null; prs: string} { + const inputs: string[] = []; + let output = DEFAULT_OUTPUT; + let json: string | null = null; + let prs = DEFAULT_PR_OUTPUT; + + for (let index = 0; index < argv.length; index++) { + const argument = argv.at(index); + + if (argument === undefined) { + continue; + } + + if (argument === '--out') { + output = argv[++index] ?? DEFAULT_OUTPUT; + continue; + } + + if (argument === '--json') { + json = argv[++index] ?? null; + continue; + } + + if (argument === '--prs') { + prs = argv[++index] ?? DEFAULT_PR_OUTPUT; + continue; + } + + inputs.push(argument); + } + + return {inputs: inputs.length > 0 ? inputs : [...DEFAULT_INPUTS], output, json, prs}; +} + +function main() { + const {inputs, output, json, prs} = parseArguments(process.argv.slice(2)); + const repoRoot = process.cwd(); + const snapshotKeyPaths = resolveSnapshotKeyPaths(repoRoot); + const files = collectFiles(inputs); + const findings: Finding[] = []; + const failures: Array<{file: string; message: string}> = []; + + for (const file of files) { + const source = fs.readFileSync(file, 'utf8'); + + if (!source.includes(`${HOOK_NAME}(`)) { + continue; + } + + try { + findings.push(...analyzeFile(file, source, snapshotKeyPaths)); + } catch (error) { + failures.push({file, message: error instanceof Error ? error.message : String(error)}); + } + } + + const report = renderReport(findings, inputs, snapshotKeyPaths, files.length); + fs.mkdirSync(path.dirname(output), {recursive: true}); + fs.writeFileSync(output, report); + + fs.mkdirSync(path.dirname(prs), {recursive: true}); + fs.writeFileSync(prs, renderPullRequestPlan(findings, PLAN_SCOPES.full)); + + const dropInPrs = prs.replace(/\.md$/, '-drop-in-only.md'); + fs.writeFileSync(dropInPrs, renderPullRequestPlan(findings, PLAN_SCOPES.dropInOnly)); + + if (json) { + fs.mkdirSync(path.dirname(json), {recursive: true}); + const candidates = findings.filter((finding) => CONVERTIBLE_VERDICTS.has(finding.verdict)); + fs.writeFileSync(json, `${JSON.stringify(candidates, null, 2)}\n`); + } + + const counts = new Map(); + + for (const finding of findings) { + counts.set(finding.verdict, (counts.get(finding.verdict) ?? 0) + 1); + } + + console.log(`Scanned ${files.length} files, found ${findings.length} useOnyx() subscriptions.`); + + for (const [verdict, title] of VERDICT_TITLES) { + console.log(` ${title}: ${counts.get(verdict) ?? 0}`); + } + + console.log(`Report written to ${output}, PR maps to ${prs} and ${dropInPrs}${json ? `, findings to ${json}` : ''}.`); + + for (const failure of failures) { + console.warn(`Could not analyze ${failure.file}: ${failure.message}`); + } +} + +main(); From aa8f824278d53e109287804f2e2728f2c26174f4 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 12:06:21 +0200 Subject: [PATCH 05/11] Type getViolationsOnyxData as the SET update it always returns --- src/libs/ReportUtils.ts | 4 +--- src/libs/Violations/ViolationsUtils.ts | 8 +++++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 145b1301a0b7..9940ec885a89 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -2440,9 +2440,7 @@ function pushTransactionViolationsOnyxData( }); // Keep the pre-toggle taxOutOfPolicy state when the update isn't about tax tracking. - // Narrow on `onyxMethod` rather than `Array.isArray`: the latter cannot pick the array arm out of a - // readonly union, so it degrades the value to `any` without `tsc` saying anything. - const recomputedViolations = optimisticViolations.onyxMethod === Onyx.METHOD.SET ? optimisticViolations.value : undefined; + const recomputedViolations = optimisticViolations.value; if (!isTaxTrackingUpdate && recomputedViolations) { const preservedTaxViolations = (existingViolations ?? []).filter((violation) => violation.name === CONST.VIOLATIONS.TAX_OUT_OF_POLICY); optimisticViolations.value = [...recomputedViolations.filter((violation) => violation.name !== CONST.VIOLATIONS.TAX_OUT_OF_POLICY), ...preservedTaxViolations]; diff --git a/src/libs/Violations/ViolationsUtils.ts b/src/libs/Violations/ViolationsUtils.ts index 08a392df8f45..7ededae198c6 100644 --- a/src/libs/Violations/ViolationsUtils.ts +++ b/src/libs/Violations/ViolationsUtils.ts @@ -453,6 +453,12 @@ function syncCustomUnitRateOutOfDateRangeViolation(violations: TransactionViolat ); } +/** + * `getViolationsOnyxData` always replaces the whole violations list, so its result is only ever the `SET` arm of + * `OnyxUpdate`. Saying that in the type lets callers read `value` as the violations array directly. + */ +type TransactionViolationsSetUpdate = Extract, {onyxMethod: typeof Onyx.METHOD.SET}>; + const ViolationsUtils = { /** * Checks a transaction for policy violations and returns an object with Onyx method, key and updated transaction @@ -486,7 +492,7 @@ const ViolationsUtils = { shouldRemoveRejectedExpenseViolation?: boolean; distanceOriginalPolicy?: OnyxEntry; ownerLogin: string | undefined; - }): OnyxUpdate { + }): TransactionViolationsSetUpdate { const isScanning = TransactionUtils.isScanning(updatedTransaction); const isScanRequest = TransactionUtils.isScanRequest(updatedTransaction); const isPartialTransaction = TransactionUtils.isPartialTransaction(updatedTransaction); From 3b9adfd0c4518ed05b5da6c53f8474afc74af2d1 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 12:12:03 +0200 Subject: [PATCH 06/11] Read the bank account list before the writes in the currency handler --- .../WorkspaceOverviewCurrencyPage.tsx | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx b/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx index 63f2bb975a37..ce389dedbb11 100644 --- a/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx +++ b/src/pages/workspace/WorkspaceOverviewCurrencyPage.tsx @@ -49,25 +49,28 @@ function WorkspaceOverviewCurrencyPage({policy}: WorkspaceOverviewCurrencyPagePr if (!policy) { return; } + + // The bank account list has to be read before the writes below, not next to where it is used: a read that + // follows a write in the same tick resolves against the pre-write cache. + const shouldRouteToExistingBankAccount = isForcedToChangeCurrency && isCurrencySupportedForGlobalReimbursement(item.currencyCode as CurrencyType); + const bankAccountList = shouldRouteToExistingBankAccount ? await Onyx.get(ONYXKEYS.BANK_ACCOUNT_LIST) : undefined; + clearDraftValues(ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM); updateGeneralSettings(policy, policy?.name ?? '', item.currencyCode, getReviewWorkspaceSettingsTaskCompletion()); clearCorpayBankAccountFields(); - if (isForcedToChangeCurrency) { - if (isCurrencySupportedForGlobalReimbursement(item.currencyCode as CurrencyType)) { - const bankAccountList = await Onyx.get(ONYXKEYS.BANK_ACCOUNT_LIST); - const hasValidExistingAccounts = getEligibleExistingBusinessBankAccounts(bankAccountList, item.currencyCode, true).length > 0; - if (hasValidExistingAccounts) { - Navigation.navigate(ROUTES.BANK_ACCOUNT_CONNECT_EXISTING_BUSINESS_BANK_ACCOUNT.getRoute(policy.id, ROUTES.WORKSPACE_WORKFLOWS.getRoute(policy.id))); - return; - } - navigateToBankAccountRoute({ - policyID: policy.id, - backTo: ROUTES.WORKSPACE_WORKFLOWS.getRoute(policy.id), - navigationOptions: {forceReplace: true}, - }); + if (shouldRouteToExistingBankAccount) { + const hasValidExistingAccounts = getEligibleExistingBusinessBankAccounts(bankAccountList, item.currencyCode, true).length > 0; + if (hasValidExistingAccounts) { + Navigation.navigate(ROUTES.BANK_ACCOUNT_CONNECT_EXISTING_BUSINESS_BANK_ACCOUNT.getRoute(policy.id, ROUTES.WORKSPACE_WORKFLOWS.getRoute(policy.id))); return; } + navigateToBankAccountRoute({ + policyID: policy.id, + backTo: ROUTES.WORKSPACE_WORKFLOWS.getRoute(policy.id), + navigationOptions: {forceReplace: true}, + }); + return; } Navigation.setNavigationActionToMicrotaskQueue(Navigation.goBack); }; From 448f13ea494e2c44d82c3a52bd6ad30979fb5302 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 14:15:15 +0200 Subject: [PATCH 07/11] Widen the report chain to ReadonlyDeep so Onyx.get reads can reach addComment --- src/libs/ReportUtils.ts | 44 ++++++++++++++++---------------- src/libs/actions/Report/index.ts | 8 +++--- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 9940ec885a89..54ae9f8c4a56 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -1291,7 +1291,7 @@ function getCurrentUserDisplayNameOrEmail(): string | undefined { return currentUserPersonalDetails?.displayName ?? deprecatedCurrentUserEmail; } -function getChatType(report: OnyxInputOrEntry | Participant): ValueOf | undefined { +function getChatType(report: ReadonlyDeep | Participant>): ValueOf | undefined { return report?.chatType; } @@ -1675,7 +1675,7 @@ function isCurrentUserSubmitter(report: ReadonlyDeep>, current /** * Whether the provided report is an Admin room */ -function isAdminRoom(report: OnyxEntry): boolean { +function isAdminRoom(report: ReadonlyDeep>): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_ADMINS; } @@ -1689,7 +1689,7 @@ function isAdminsOnlyPostingRoom(report: OnyxEntry): boolean { /** * Whether the provided report is a Announce room */ -function isAnnounceRoom(report: OnyxEntry): boolean { +function isAnnounceRoom(report: ReadonlyDeep>): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.POLICY_ANNOUNCE; } @@ -1717,11 +1717,11 @@ function isUserCreatedPolicyRoom(report: OnyxEntry): boolean { /** * Whether the provided report is a Policy Expense chat. */ -function isPolicyExpenseChat(option: OnyxInputOrEntry | OptionData | Participant): boolean { +function isPolicyExpenseChat(option: ReadonlyDeep | OptionData | Participant>): boolean { return getChatType(option) === CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT || !!(option && typeof option === 'object' && 'isPolicyExpenseChat' in option && option.isPolicyExpenseChat); } -function isInvoiceRoom(report: OnyxEntry): boolean { +function isInvoiceRoom(report: ReadonlyDeep>): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.INVOICE; } @@ -1815,7 +1815,7 @@ function isChatRoom(report: OnyxEntry): boolean { /** * Whether the provided report is a public room */ -function isPublicRoom(report: OnyxEntry): boolean { +function isPublicRoom(report: ReadonlyDeep>): boolean { return report?.visibility === CONST.REPORT.VISIBILITY.PUBLIC || report?.visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE; } @@ -1866,7 +1866,7 @@ function isWorkspaceTaskReport(report: OnyxEntry): boolean { /** * Returns true if report has a parent */ -function isThread(report: OnyxInputOrEntry): report is Thread { +function isThread>>(report: TReport): report is TReport & ReadonlyDeep { return !!(report?.parentReportID && report?.parentReportActionID); } @@ -1900,11 +1900,11 @@ function isDM(report: OnyxEntry): boolean { return isChatReport(report) && !getChatType(report) && !isThread(report); } -function isSelfDM(report: OnyxInputOrEntry): boolean { +function isSelfDM(report: ReadonlyDeep>): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.SELF_DM; } -function isGroupChat(report: OnyxEntry | Partial): boolean { +function isGroupChat(report: ReadonlyDeep | Partial>): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.GROUP; } @@ -1936,7 +1936,7 @@ function isSystemChat(report: OnyxEntry): boolean { return getChatType(report) === CONST.REPORT.CHAT_TYPE.SYSTEM; } -function getDefaultNotificationPreferenceForReport(report: OnyxEntry): ValueOf { +function getDefaultNotificationPreferenceForReport(report: ReadonlyDeep>): ValueOf { if (isAnnounceRoom(report)) { return CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS; } @@ -1959,7 +1959,7 @@ function getDefaultNotificationPreferenceForReport(report: OnyxEntry): V * Get the notification preference given a report. This should ALWAYS default to 'hidden'. Do not change this! */ // TODO: currentUserAccountID will be required eventually so this becomes a pure function. Subscribe the data via useOnyx and pass it from the component. Refactor issue: https://github.com/Expensify/App/issues/66412 -function getReportNotificationPreference(report: OnyxEntry, currentUserAccountID?: number): ValueOf { +function getReportNotificationPreference(report: ReadonlyDeep>, currentUserAccountID?: number): ValueOf { const accountID = currentUserAccountID ?? deprecatedCurrentUserAccountID; const participant = accountID ? report?.participants?.[accountID] : undefined; @@ -1971,7 +1971,7 @@ function getReportNotificationPreference(report: OnyxEntry, currentUserA /** * Only returns true if this is our main 1:1 DM report with Concierge. */ -function isConciergeChatReport(report: OnyxInputOrEntry, conciergeReportID: string | undefined): boolean { +function isConciergeChatReport(report: ReadonlyDeep>, conciergeReportID: string | undefined): boolean { return !!report && !!conciergeReportID && report.reportID === conciergeReportID; } @@ -2504,8 +2504,8 @@ function canCreateTaskInReport(report: OnyxEntry): boolean { * We will remove the 'hidden' field entirely once the backend changes for https://github.com/Expensify/Expensify/issues/450891 are done. */ function isHiddenForCurrentUser(notificationPreference: string | null | undefined): boolean; -function isHiddenForCurrentUser(report: OnyxEntry): boolean; -function isHiddenForCurrentUser(reportOrPreference: OnyxEntry | string | null | undefined): boolean { +function isHiddenForCurrentUser(report: ReadonlyDeep>): boolean; +function isHiddenForCurrentUser(reportOrPreference: ReadonlyDeep> | string | null | undefined): boolean { if (typeof reportOrPreference === 'object' && reportOrPreference !== null) { const notificationPreference = getReportNotificationPreference(reportOrPreference); return isHiddenForCurrentUser(notificationPreference); @@ -2648,7 +2648,7 @@ function isClosedExpenseReportWithNoExpenses(report: OnyxEntry, transact /** * Whether the provided report is an archived room */ -function isArchivedNonExpenseReport(report: OnyxInputOrEntry, isReportArchived = false): boolean { +function isArchivedNonExpenseReport(report: ReadonlyDeep>, isReportArchived = false): boolean { return isReportArchived && !(isExpenseReport(report) || isExpenseRequest(report)); } @@ -2725,7 +2725,7 @@ function isAuditor(report: OnyxEntry): boolean { /** * Checks if the user can write in the provided report */ -function canWriteInReport(report: OnyxEntry): boolean { +function canWriteInReport(report: ReadonlyDeep>): boolean { if (Array.isArray(report?.permissions) && report?.permissions.length > 0 && !report?.permissions?.includes(CONST.REPORT.PERMISSIONS.AUDITOR)) { return report?.permissions?.includes(CONST.REPORT.PERMISSIONS.WRITE) || report?.permissions?.includes(CONST.REPORT.PERMISSIONS.COMMENT); } @@ -2736,7 +2736,7 @@ function canWriteInReport(report: OnyxEntry): boolean { /** * Checks if the current user is allowed to comment on the given report. */ -function isAllowedToComment(report: OnyxEntry): boolean { +function isAllowedToComment(report: ReadonlyDeep>): boolean { if (!canWriteInReport(report)) { return false; } @@ -2784,7 +2784,7 @@ function isWorkspaceThread(report: OnyxEntry): boolean { * An Expense Request is a thread where the parent report is an Expense Report and * the parentReportAction is a transaction. */ -function isExpenseRequest(report: OnyxInputOrEntry): report is Thread { +function isExpenseRequest>>(report: TReport): report is TReport & ReadonlyDeep { if (isThread(report)) { const parentReportAction = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`]?.[report.parentReportActionID]; const parentReport = getReport(report?.parentReportID, deprecatedAllReports); @@ -11021,7 +11021,7 @@ function isValidReportIDFromPath(reportIDFromPath: string | undefined): boolean /** * Return the errors we have when creating a chat, a workspace room, or a new empty report */ -function getCreationReportErrors(report: OnyxEntry): Errors | null | undefined { +function getCreationReportErrors(report: ReadonlyDeep>): Errors | null | undefined { // We are either adding a workspace room, creating a chat, or we're creating a report, it isn't possible for all of these to have errors for the same report at the same time, so // simply looking up the first truthy value will get the relevant property if it's set. return report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat ?? report?.errorFields?.createReport; @@ -11030,7 +11030,7 @@ function getCreationReportErrors(report: OnyxEntry): Errors | null | und /** * Return true if the expense report is marked for deletion. */ -function isMoneyRequestReportPendingDeletion(reportOrID: OnyxEntry | string): boolean { +function isMoneyRequestReportPendingDeletion(reportOrID: ReadonlyDeep> | string): boolean { const report = typeof reportOrID === 'string' ? getReport(reportOrID, deprecatedAllReports) : reportOrID; if (!isMoneyRequestReport(report)) { return false; @@ -11096,7 +11096,7 @@ function navigateToLinkedReportAction( * not on its way out, it did not fail to be created, and the person looking is signed in. Permission to write is left * out on purpose, so this also covers read-only actions such as opening an attachment. */ -function canUserInteractWithReport(report: OnyxEntry, isReportArchived: boolean | undefined) { +function canUserInteractWithReport(report: ReadonlyDeep>, isReportArchived: boolean | undefined) { const reportErrors = getCreationReportErrors(report); // If the expense report is marked for deletion, let us prevent any further interaction. @@ -11107,7 +11107,7 @@ function canUserInteractWithReport(report: OnyxEntry, isReportArchived: return !isArchivedNonExpenseReport(report, isReportArchived) && isEmptyObject(reportErrors) && report && !deprecatedIsAnonymousUser; } -function canUserPerformWriteAction(report: OnyxEntry, isReportArchived: boolean | undefined) { +function canUserPerformWriteAction(report: ReadonlyDeep>, isReportArchived: boolean | undefined) { return canUserInteractWithReport(report, isReportArchived) && isAllowedToComment(report) && canWriteInReport(report); } diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 595806f8cd98..e91ce524df11 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -263,7 +263,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {Dimensions} from '@src/types/utils/Layout'; import type {NullishDeep, OnyxCollection, OnyxCollectionInputValue, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; -import type {PartialDeep, ValueOf} from 'type-fest'; +import type {PartialDeep, ReadonlyDeep, ValueOf} from 'type-fest'; /* eslint-disable max-lines */ import {Str} from 'expensify-common'; @@ -404,7 +404,7 @@ type PregeneratedResponseParams = { }; type AddCommentParams = { - report: OnyxEntry; + report: ReadonlyDeep>; notifyReportID: string; ancestors: Ancestor[]; text: string; @@ -421,7 +421,7 @@ type AddCommentParams = { }; type AddActionsParams = { - report: OnyxEntry; + report: ReadonlyDeep>; notifyReportID: string | string[]; ancestors: Ancestor[]; timezoneParam: Timezone; @@ -3326,7 +3326,7 @@ function broadcastUserIsLeavingRoom(reportID: string, currentUserAccountID: numb /** Deletes a comment from the report, basically sets it as empty string */ function deleteReportComment( - report: OnyxEntry, + report: ReadonlyDeep>, reportAction: ReportAction, originalReportActions: OnyxEntry, reportActions: OnyxEntry, From 6bfb47c92edfdbafac2e8999d9012f35be363935 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 14:15:17 +0200 Subject: [PATCH 08/11] Accept async submit handlers and replace the Array.isArray violations guard --- src/components/Form/FormProvider.tsx | 4 +++- src/components/FormAlertWithSubmitButton.tsx | 2 +- src/components/SelectionList/types.ts | 2 +- src/hooks/usePressLoading.ts | 7 ++++--- src/libs/actions/IOU/SplitTransactionUpdate.ts | 13 ++++++++++++- 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/components/Form/FormProvider.tsx b/src/components/Form/FormProvider.tsx index 5972fdf2e685..fa3752b1c141 100644 --- a/src/components/Form/FormProvider.tsx +++ b/src/components/Form/FormProvider.tsx @@ -345,7 +345,9 @@ function FormProvider({ if (keyboardSubmitBehavior === CONST.KEYBOARD_SUBMIT_BEHAVIOR.DISMISS_THEN_SUBMIT) { KeyboardUtils.dismiss().then(() => onSubmit(trimmedStringValues)); } else if (keyboardSubmitBehavior === CONST.KEYBOARD_SUBMIT_BEHAVIOR.SUBMIT_AND_DISMISS) { - KeyboardUtils.dismissKeyboardAndExecute(() => onSubmit(trimmedStringValues)); + KeyboardUtils.dismissKeyboardAndExecute(() => { + onSubmit(trimmedStringValues); + }); } else { onSubmit(trimmedStringValues); } diff --git a/src/components/FormAlertWithSubmitButton.tsx b/src/components/FormAlertWithSubmitButton.tsx index 4250e8623679..361089a5c647 100644 --- a/src/components/FormAlertWithSubmitButton.tsx +++ b/src/components/FormAlertWithSubmitButton.tsx @@ -45,7 +45,7 @@ type FormAlertWithSubmitButtonProps = WithSentryLabel & { onFixTheErrorsLinkPressed?: () => void; /** Submit function */ - onSubmit: () => void; + onSubmit: () => void | Promise; /** Should the button be enabled when offline */ enabledWhenOffline?: boolean; diff --git a/src/components/SelectionList/types.ts b/src/components/SelectionList/types.ts index 475c9ae4eb15..6860ff18cde5 100644 --- a/src/components/SelectionList/types.ts +++ b/src/components/SelectionList/types.ts @@ -23,7 +23,7 @@ type BaseSelectionListProps = { initiallyFocusedItemKey?: string; /** Called when a row is pressed */ - onSelectRow: (item: TItem) => void; + onSelectRow: (item: TItem) => void | Promise; /** Whether this is a multi-select list */ canSelectMultiple?: boolean; diff --git a/src/hooks/usePressLoading.ts b/src/hooks/usePressLoading.ts index e6df11293364..58b86576f301 100644 --- a/src/hooks/usePressLoading.ts +++ b/src/hooks/usePressLoading.ts @@ -12,7 +12,7 @@ type UsePressLoadingReturn = { /** True while the button press is pending or the external loading flag is set, so the spinner stays visible. */ isLoading: boolean; /** Call instead of a bare press handler to show the spinner immediately on press. */ - startWithLoading: (runAfterPaint: () => void) => Promise; + startWithLoading: (runAfterPaint: () => void | Promise) => Promise; }; /** @@ -32,13 +32,14 @@ function usePressLoading({isLoading = false, resetOnFocus = true}: UsePressLoadi setIsPressed(false); } // Defer the work by one macrotask so React can commit isPressed and paint the spinner before the consumer code that may block the JS thread runs. - const startWithLoading = async (runAfterPaint: () => void) => { + const startWithLoading = async (runAfterPaint: () => void | Promise) => { setIsPressed(true); await new Promise((resolve) => { setTimeout(resolve, 0); }); try { - runAfterPaint(); + // Awaited so a rejected async handler still resets the pressed state instead of leaving the spinner up. + await runAfterPaint(); } catch (error) { setIsPressed(false); throw error; diff --git a/src/libs/actions/IOU/SplitTransactionUpdate.ts b/src/libs/actions/IOU/SplitTransactionUpdate.ts index 3d024da4c316..1a7bb11f8e4c 100644 --- a/src/libs/actions/IOU/SplitTransactionUpdate.ts +++ b/src/libs/actions/IOU/SplitTransactionUpdate.ts @@ -73,6 +73,7 @@ import type {TransactionChanges} from '@src/types/onyx/Transaction'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; import Onyx from 'react-native-onyx'; @@ -126,6 +127,15 @@ type UpdateSplitTransactionsParams = { getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol']; }; +/** + * Whether an Onyx update value is a list of violations. `Array.isArray` cannot be used directly here: + * it is typed `(value: any) => value is any[]`, so on a value whose array arm is readonly it narrows to + * `any[]`, which typechecks silently and only surfaces as an ESLint unsafe-access error at the use site. + */ +function isTransactionViolationsList(value: TValue): value is TValue & ReadonlyDeep { + return Array.isArray(value); +} + /** * Picks the transaction in `snapshotData` whose conversion can be reused for `transaction`: candidates are * tried in order and the first one holding a conversion for the same currency wins. @@ -1024,7 +1034,8 @@ function updateSplitTransactions({ ) => { const transactionViolationsKey = `${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${targetTransactionID}` as typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS; const latestViolationUpdate = onyxUpdates?.findLast((update) => update.key === transactionViolationsKey) as OnyxUpdate; - const baseViolations = latestViolationUpdate && 'value' in latestViolationUpdate && Array.isArray(latestViolationUpdate.value) ? latestViolationUpdate.value : fallbackViolations; + const baseViolations = + latestViolationUpdate && 'value' in latestViolationUpdate && isTransactionViolationsList(latestViolationUpdate.value) ? latestViolationUpdate.value : fallbackViolations; const nextViolations = [ ...baseViolations.filter((violation) => violation.name !== CONST.VIOLATIONS.HOLD), {name: CONST.VIOLATIONS.HOLD, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true}, From d5bdd74c4fe53ba161c7b4290d0839c7787c69a7 Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Fri, 4 Sep 2026 18:44:55 +0200 Subject: [PATCH 09/11] Widen the transaction helper chain to ReadonlyDeep for Onyx.get reads --- src/libs/TransactionUtils/index.ts | 38 +++++++++++++++++------------- src/selectors/Transaction.ts | 3 ++- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index cf582f10f5fd..e1f6e1148720 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -162,7 +162,7 @@ function isDeletedTransaction(transaction: {reportID?: string}): boolean { return transaction.reportID === CONST.REPORT.TRASH_REPORT_ID; } -function isDistanceRequest(transaction: OnyxEntry): boolean { +function isDistanceRequest(transaction: ReadonlyDeep>): boolean { const requestType = transaction?.iouRequestType; return requestType === CONST.IOU.REQUEST_TYPE.DISTANCE || isDistanceExpenseType(requestType); } @@ -279,7 +279,7 @@ function isMapBasedDistanceRequest(transaction: OnyxEntry): boolean return isMapDistanceRequest(transaction) || isGPSDistanceRequest(transaction) || hasWaypoints; } -function isScanRequest(transaction: OnyxEntry>): boolean { +function isScanRequest(transaction: ReadonlyDeep>>): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.SCAN; } @@ -404,7 +404,7 @@ function getReceiptTypeTranslationKey(receiptType: ValueOf): boolean { +function isPartialTransaction(transaction: ReadonlyDeep>): boolean { const merchant = getMerchant(transaction); if (!merchant || isPartialMerchant(merchant)) { @@ -586,18 +586,18 @@ function buildOptimisticTransaction(params: BuildOptimisticTransactionParams): T /** * Check if the transaction has an Ereceipt */ -function hasEReceipt(transaction: Transaction | undefined | null): boolean { +function hasEReceipt(transaction: ReadonlyDeep | undefined | null): boolean { return !!transaction?.hasEReceipt; } -function hasReceipt(transaction: OnyxInputOrEntry | undefined): boolean { +function hasReceipt(transaction: ReadonlyDeep> | undefined): boolean { return !!transaction?.receipt?.state || hasEReceipt(transaction); } /** * Whether the transaction already has its receipt stored server-side. */ -function hasUploadedReceipt(transaction: OnyxInputOrEntry | undefined): boolean { +function hasUploadedReceipt(transaction: ReadonlyDeep> | undefined): boolean { return !!transaction?.receipt?.receiptID; } @@ -650,14 +650,14 @@ function isPartialMerchant(merchant: string): boolean { return merchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT; } -function isAmountMissing(transaction: OnyxEntry, isFromExpenseReport = true) { +function isAmountMissing(transaction: ReadonlyDeep>, isFromExpenseReport = true) { if (isFromExpenseReport) { return transaction?.amount === undefined && (transaction?.modifiedAmount === undefined || transaction?.modifiedAmount === ''); } return (transaction?.amount === 0 || transaction?.amount === undefined) && (!transaction?.modifiedAmount || transaction?.modifiedAmount === 0 || transaction?.modifiedAmount === ''); } -function hasValidModifiedAmount(transaction: OnyxEntry | null): boolean { +function hasValidModifiedAmount(transaction: ReadonlyDeep> | null): boolean { if (!transaction) { return false; } @@ -1231,7 +1231,13 @@ function getDescription(transaction: OnyxInputOrEntry): string { /** * Return the amount field from the transaction, return the modifiedAmount if present. */ -function getAmount(transaction: OnyxInputOrEntry, isFromExpenseReport = false, isFromTrackedExpense = false, allowNegative = false, disableOppositeConversion = false): number { +function getAmount( + transaction: ReadonlyDeep>, + isFromExpenseReport = false, + isFromTrackedExpense = false, + allowNegative = false, + disableOppositeConversion = false, +): number { // IOU requests cannot have negative values, but they can be stored as negative values, let's return absolute value if (!isFromExpenseReport && !isFromTrackedExpense && !allowNegative) { const amount = Number(transaction?.modifiedAmount) ?? 0; @@ -1398,7 +1404,7 @@ function getOriginalCurrencyForDisplay(transaction: Pick): boolean { +function isFetchingWaypointsFromServer(transaction: ReadonlyDeep>): boolean { return !!transaction?.pendingFields?.waypoints; } @@ -1409,7 +1415,7 @@ function isFetchingWaypointsFromServer(transaction: OnyxInputOrEntry): boolean { +function hasLocallyKnownDistance(transaction: ReadonlyDeep>): boolean { const hasDistanceSource = !!transaction?.comment?.customUnit?.quantity || !!transaction?.routes?.route0?.distance; return hasDistanceSource && !!getAmount(transaction); } @@ -1435,7 +1441,7 @@ function hasPendingDistanceReceiptRegeneration(transaction: OnyxInputOrEntry): string { +function getMerchant(transaction: ReadonlyDeep>): string { return transaction?.modifiedMerchant ? transaction.modifiedMerchant : (transaction?.merchant ?? ''); } @@ -1899,7 +1905,7 @@ function showHeldExpensesBlockModal( * The transaction is considered scanning if it is a partial transaction, has a receipt, and the receipt is being scanned. * Note that this does not include receipts that are being scanned in the background for auditing / smart scan everything, because there should be no indication to the user that the receipt is being scanned. */ -function isScanning(transaction: OnyxEntry): boolean { +function isScanning(transaction: ReadonlyDeep>): boolean { // Performance optimization: Check the receipt state first (cheapest check) before doing more expensive checks if (!isReceiptBeingScanned(transaction)) { return false; @@ -1913,7 +1919,7 @@ function isScanning(transaction: OnyxEntry): boolean { return isPartialTransaction(transaction) && hasReceipt(transaction); } -function isReceiptBeingScanned(transaction: OnyxInputOrEntry): boolean { +function isReceiptBeingScanned(transaction: ReadonlyDeep>): boolean { return transaction?.receipt?.state === CONST.IOU.RECEIPT_STATE.SCAN_READY || transaction?.receipt?.state === CONST.IOU.RECEIPT_STATE.SCANNING; } @@ -2443,7 +2449,7 @@ function isDuplicate( /** * Check if transaction is on hold */ -function isOnHold(transaction: OnyxEntry): boolean { +function isOnHold(transaction: ReadonlyDeep>): boolean { if (!transaction) { return false; } @@ -3331,7 +3337,7 @@ function isSplitChildTransaction(transaction: OnyxEntry | Transacti * hidden and has no dismiss UI of its own. Used to decide whether a split failure error on the original * should be cleared alongside the visible child's error. */ -function isSplitContainerTransaction(transaction: OnyxEntry | Transaction): boolean { +function isSplitContainerTransaction(transaction: ReadonlyDeep> | Transaction): boolean { return transaction?.reportID === CONST.REPORT.SPLIT_REPORT_ID; } diff --git a/src/selectors/Transaction.ts b/src/selectors/Transaction.ts index 6c849d4484c0..6867276fe621 100644 --- a/src/selectors/Transaction.ts +++ b/src/selectors/Transaction.ts @@ -1,9 +1,10 @@ import type TransactionType from '@src/types/onyx/Transaction'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; /** Resolves the original (container) transaction ID that a split child points to. */ -const originalTransactionIDSelector = (transaction: OnyxEntry): string | undefined => transaction?.comment?.originalTransactionID; +const originalTransactionIDSelector = (transaction: ReadonlyDeep>): string | undefined => transaction?.comment?.originalTransactionID; const transactionsByReportIDSelector = (transactions: OnyxCollection): Record => { const result: Record = {}; From 4b908271e1821027fee8549e359ebf0ded86975d Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 8 Sep 2026 09:57:07 +0200 Subject: [PATCH 10/11] migrate Transaction --- src/libs/DistanceRequestUtils.ts | 3 ++- src/libs/TransactionUtils/index.ts | 29 +++++++++++++++++------------ src/libs/actions/IOU/Split.ts | 4 ++-- src/libs/cloneMutable.ts | 15 +++++++++++++++ 4 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 src/libs/cloneMutable.ts diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index d369ce83097b..79e8c614a10d 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -12,6 +12,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {Locale as DateFnsLocale} from 'date-fns'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; import {format, parseISO} from 'date-fns'; @@ -379,7 +380,7 @@ function getTransactionCommuterExclusionData({ transaction: OnyxEntry; policy: OnyxEntry; customUnit?: TransactionCustomUnit; - storedCustomUnit?: TransactionCustomUnit; + storedCustomUnit?: ReadonlyDeep; translate?: LocaleContextProps['translate']; toLocaleDigit?: LocaleContextProps['toLocaleDigit']; getCurrencySymbol?: CurrencyListActionsContextType['getCurrencySymbol']; diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index 3cf008c858f9..cb849db7b9cb 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -100,7 +100,7 @@ import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {format, isValid, parse} from 'date-fns'; import {SafeString, Str} from 'expensify-common'; import {deepEqual} from 'fast-equals'; -import lodashDeepClone from 'lodash/cloneDeep'; +import cloneMutable from '@libs/cloneMutable'; import lodashSet from 'lodash/set'; import Onyx from 'react-native-onyx'; @@ -190,11 +190,11 @@ function isGPSDistanceRequest(transaction: OnyxEntry): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_GPS; } -function isManualDistanceRequest(transaction: OnyxEntry): boolean { +function isManualDistanceRequest(transaction: ReadonlyDeep>): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; } -function isOdometerDistanceRequest(transaction: OnyxEntry): boolean { +function isOdometerDistanceRequest(transaction: ReadonlyDeep>): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER; } @@ -726,7 +726,7 @@ function getDistanceMerchantForTransaction({ getCurrencySymbol, commuterExclusionData, }: { - transaction: OnyxEntry; + transaction: ReadonlyDeep>; distanceInMeters: number; unit: Unit | undefined; rate: number | undefined; @@ -754,7 +754,7 @@ function getDistanceMerchantForTransaction({ * imperative locale accessors the optimistic update paths below have to rely on. */ function getRecalculatedDistanceMerchant( - transaction: OnyxEntry, + transaction: ReadonlyDeep>, distanceInMeters: number, unit: Unit | undefined, rate: number | undefined, @@ -789,7 +789,7 @@ function getUpdatedTransaction({ getCurrencyDecimals, getCurrencySymbol, }: { - transaction: Transaction; + transaction: ReadonlyDeep; transactionChanges: TransactionChanges; isFromExpenseReport: boolean; shouldUpdateReceiptState?: boolean; @@ -803,7 +803,7 @@ function getUpdatedTransaction({ const isUnReportedExpense = transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; // Only changing the first level fields so no need for deep clone now - const updatedTransaction = lodashDeepClone(transaction); + const updatedTransaction = cloneMutable(transaction); let shouldStopSmartscan = false; // The comment property does not have its modifiedComment counterpart @@ -2665,7 +2665,7 @@ function isPayAtEndExpense(transaction: Transaction | undefined | null): boolean /** * Get custom unit rate (distance rate) ID from the transaction object */ -function getRateID(transaction: OnyxInputOrEntry): string { +function getRateID(transaction: ReadonlyDeep>): string { return transaction?.comment?.customUnit?.customUnitRateID ?? CONST.CUSTOM_UNITS.FAKE_P2P_ID; } @@ -2674,7 +2674,7 @@ function getRateID(transaction: OnyxInputOrEntry): string { * If it is distance request, then returns the tax code corresponding to the custom unit rate * Else returns policy default tax rate if transaction is in policy default currency, otherwise foreign default tax rate */ -function getDefaultTaxCode(policy: OnyxEntry, transaction: OnyxEntry, currency?: string | undefined, newCustomUnitRateID?: string): string | undefined { +function getDefaultTaxCode(policy: OnyxEntry, transaction: ReadonlyDeep>, currency?: string | undefined, newCustomUnitRateID?: string): string | undefined { if (isDistanceRequest(transaction)) { // When editing a distance rate, the draft transaction's customUnitRateID // does not reflect the newly selected rate until setMoneyRequestDistanceRate is called, and the draft transaction's is updated. @@ -2699,7 +2699,7 @@ function getDefaultTaxCode(policy: OnyxEntry, transaction: OnyxEntry | undefined, transaction?: OnyxEntry): Record { +function transformedTaxRates(policy: OnyxEntry | undefined, transaction?: ReadonlyDeep>): Record { const taxRates = policy?.taxRates; const defaultExternalID = taxRates?.defaultExternalID; @@ -2718,7 +2718,7 @@ function transformedTaxRates(policy: OnyxEntry | undefined, transaction? /** * Gets the tax value of a selected tax */ -function getTaxValue(policy: OnyxEntry, transaction: OnyxEntry, taxCode: string) { +function getTaxValue(policy: OnyxEntry, transaction: ReadonlyDeep>, taxCode: string) { const resolvedTaxCode = resolveCurrentTaxCode(policy, taxCode); return Object.values(transformedTaxRates(policy, transaction)).find((taxRate) => taxRate.code === resolvedTaxCode)?.value; } @@ -3272,7 +3272,12 @@ function buildMergeDuplicatesParams( }; } -function getCategoryTaxDetails(category: string, transaction: OnyxEntry, policy: OnyxEntry, getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals']) { +function getCategoryTaxDetails( + category: string, + transaction: ReadonlyDeep>, + policy: OnyxEntry, + getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals'], +) { const taxRules = policy?.rules?.expenseRules?.filter((rule) => rule.tax); if (!taxRules || taxRules?.length === 0 || isDistanceRequest(transaction)) { return {categoryTaxCode: undefined, categoryTaxAmount: undefined, categoryTaxValue: undefined}; diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index 10eb3e5da269..694af410fce7 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -69,7 +69,7 @@ import type {Receipt, ReceiptSource, SplitShares, TransactionChanges, WaypointCo import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OnyxCollection, OnyxEntry, OnyxInputValue, OnyxUpdate} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {fastMerge} from 'expensify-common'; import Onyx from 'react-native-onyx'; @@ -1315,7 +1315,7 @@ function resetSplitShares( function setDraftSplitTransaction( transactionID: string | undefined, - splitTransactionDraft: OnyxEntry, + splitTransactionDraft: ReadonlyDeep>, transactionChanges: TransactionChanges, getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals'], getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol'], diff --git a/src/libs/cloneMutable.ts b/src/libs/cloneMutable.ts new file mode 100644 index 000000000000..0565985371c4 --- /dev/null +++ b/src/libs/cloneMutable.ts @@ -0,0 +1,15 @@ +import lodashDeepClone from 'lodash/cloneDeep'; +import type {ReadonlyDeep} from 'type-fest'; + +/** + * Deep-clones a readonly Onyx value into a mutable one. + * + * `lodash/cloneDeep` is typed `(value: T) => T`, so cloning a `ReadonlyDeep` returns a + * `ReadonlyDeep` even though the clone is a fresh object no one else holds. The assertion restores + * the mutability the clone actually has, and lives here so the codebase has exactly one of them. + */ +function cloneMutable(value: ReadonlyDeep): T { + return lodashDeepClone(value) as T; +} + +export default cloneMutable; From f62071bd4112eddd68c4e05c84d97644cfb86e4f Mon Sep 17 00:00:00 2001 From: Lukasz Modzelewski Date: Tue, 8 Sep 2026 11:15:54 +0200 Subject: [PATCH 11/11] Migrate TRANSACTION_DRAFT reads to readonly Onyx.get --- config/eslint/eslint.seatbelt.tsv | 1 + src/components/Search/BulkDuplicateHandler.tsx | 2 +- src/hooks/useBulkDuplicateAction.ts | 6 ++++-- src/hooks/useSearchBulkActions.ts | 4 ++-- src/hooks/useSelectedTransactionsActions.ts | 4 ++-- src/libs/DistanceRequestUtils.ts | 8 ++++---- src/libs/TransactionUtils/index.ts | 12 ++++++------ src/libs/actions/IOU/Duplicate.ts | 6 +++--- src/libs/actions/IOU/MoneyRequest.ts | 4 ++-- src/libs/actions/IOU/MoneyRequestBuilder.ts | 17 +++++++++++------ src/libs/actions/IOU/Split.ts | 2 +- src/libs/actions/IOU/TrackExpense.ts | 6 ++++-- src/selectors/TransactionDraft.ts | 7 ++++--- .../hooks/useSearchBulkActionsDuplicateTest.ts | 11 +++++++++++ 14 files changed, 56 insertions(+), 34 deletions(-) diff --git a/config/eslint/eslint.seatbelt.tsv b/config/eslint/eslint.seatbelt.tsv index fe338aedf784..e1cd038caadc 100644 --- a/config/eslint/eslint.seatbelt.tsv +++ b/config/eslint/eslint.seatbelt.tsv @@ -864,6 +864,7 @@ "../../src/libs/actions/replaceOptimisticReportWithActualReport.ts" "@typescript-eslint/no-unsafe-type-assertion" 2 "../../src/libs/canFocusInputOnScreenFocus/index.ts" "no-restricted-syntax" 1 "../../src/libs/clearSelectedTextIfComposerBlurred/clearSelectedTextIfComposerBlurred.web.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 +"../../src/libs/cloneMutable.ts" "@typescript-eslint/no-unsafe-type-assertion" 1 "../../src/libs/compoundParamsKey.ts" "@typescript-eslint/no-unsafe-type-assertion" 2 "../../src/libs/fileDownload/DownloadUtils.ts" "no-restricted-syntax" 2 "../../src/libs/fileDownload/FileUtils.ts" "@typescript-eslint/no-unsafe-type-assertion" 4 diff --git a/src/components/Search/BulkDuplicateHandler.tsx b/src/components/Search/BulkDuplicateHandler.tsx index e6323cbe5f86..e78c621667e8 100644 --- a/src/components/Search/BulkDuplicateHandler.tsx +++ b/src/components/Search/BulkDuplicateHandler.tsx @@ -11,7 +11,7 @@ type BulkDuplicateHandlerProps = { allTransactions: OnyxCollection; allReports: OnyxCollection | undefined; searchData: Record | undefined; - onHandlerReady: (handler: () => void) => void; + onHandlerReady: (handler: () => void | Promise) => void; onAfterDuplicate?: () => void; }; diff --git a/src/hooks/useBulkDuplicateAction.ts b/src/hooks/useBulkDuplicateAction.ts index c4125492bbb8..a9f8db7a2f3a 100644 --- a/src/hooks/useBulkDuplicateAction.ts +++ b/src/hooks/useBulkDuplicateAction.ts @@ -14,6 +14,7 @@ import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import {hasSeenTourSelector} from '@selectors/Onboarding'; import {validTransactionDraftsSelector} from '@selectors/TransactionDraft'; +import Onyx from 'react-native-onyx'; import {useCurrencyListActions} from './useCurrencyList'; import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; @@ -54,7 +55,6 @@ function useBulkDuplicateAction({selectedTransactionsKeys, allTransactions, allR const [quickAction] = useOnyx(ONYXKEYS.NVP_QUICK_ACTION_GLOBAL_CREATE); const [policyRecentlyUsedCurrencies] = useOnyx(ONYXKEYS.RECENTLY_USED_CURRENCIES); const [isSelfTourViewed = false] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {selector: hasSeenTourSelector}); - const [transactionDrafts] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_DRAFT, {selector: validTransactionDraftsSelector}); const [betas] = useOnyx(ONYXKEYS.BETAS); const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); const [recentWaypoints] = useOnyx(ONYXKEYS.NVP_RECENT_WAYPOINTS); @@ -78,7 +78,9 @@ function useBulkDuplicateAction({selectedTransactionsKeys, allTransactions, allR const participants = getMoneyRequestParticipantsFromReport(activePolicyExpenseChat, accountID); const participantsPolicyTags = useParticipantsPolicyTags(participants); - const handleDuplicate = () => { + const handleDuplicate = async () => { + const transactionDrafts = validTransactionDraftsSelector(await Onyx.get(ONYXKEYS.COLLECTION.TRANSACTION_DRAFT)); + bulkDuplicateExpenses({ dateFnsLocale, getCurrencyDecimals, diff --git a/src/hooks/useSearchBulkActions.ts b/src/hooks/useSearchBulkActions.ts index 2e675893b684..51aff461c9d2 100644 --- a/src/hooks/useSearchBulkActions.ts +++ b/src/hooks/useSearchBulkActions.ts @@ -1653,8 +1653,8 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) { ); }, [selectedTransactionReportIDs, currentSearchResults?.data, allReports]); - const duplicateHandlerRef = useRef<() => void>(() => {}); - const setDuplicateHandler = useCallback((handler: () => void) => { + const duplicateHandlerRef = useRef<() => void | Promise>(() => {}); + const setDuplicateHandler = useCallback((handler: () => void | Promise) => { duplicateHandlerRef.current = handler; }, []); const invokeDuplicateHandler = useCallback(() => { diff --git a/src/hooks/useSelectedTransactionsActions.ts b/src/hooks/useSelectedTransactionsActions.ts index 57fcc37210d2..c48f90546586 100644 --- a/src/hooks/useSelectedTransactionsActions.ts +++ b/src/hooks/useSelectedTransactionsActions.ts @@ -211,8 +211,8 @@ function useSelectedTransactionsActions({ ], ); - const duplicateHandlerRef = useRef<() => void>(() => {}); - const setDuplicateHandler = useCallback((handler: () => void) => { + const duplicateHandlerRef = useRef<() => void | Promise>(() => {}); + const setDuplicateHandler = useCallback((handler: () => void | Promise) => { duplicateHandlerRef.current = handler; }, []); const invokeDuplicateHandler = useCallback(() => { diff --git a/src/libs/DistanceRequestUtils.ts b/src/libs/DistanceRequestUtils.ts index 79e8c614a10d..a91e1ba87e79 100644 --- a/src/libs/DistanceRequestUtils.ts +++ b/src/libs/DistanceRequestUtils.ts @@ -290,7 +290,7 @@ function getDistanceMerchant( * @param currency * @returns The rate and unit in MileageRate object. */ -function getRateForP2P(currency: string, transaction: OnyxEntry): MileageRate { +function getRateForP2P(currency: string, transaction: ReadonlyDeep>): MileageRate { const defaultRate = getStoredDefaultP2PMileageRate(); const p2pRate: DefaultP2PMileageRate = defaultRate ?? {rate: DEFAULT_P2P_RATE_CENTS_PER_MILE, unit: CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES}; const rate = transaction && getCurrency(transaction) === currency ? (transaction.comment?.customUnit?.defaultP2PRate ?? p2pRate.rate) : p2pRate.rate; @@ -707,7 +707,7 @@ function getTaxableAmount(policy: OnyxEntry, customUnitRateID: string, d return amount * taxClaimablePercentage; } -function getDistanceUnit(transaction: OnyxEntry, mileageRate: OnyxEntry): Unit { +function getDistanceUnit(transaction: ReadonlyDeep>, mileageRate: OnyxEntry): Unit { return transaction?.comment?.customUnit?.distanceUnit ?? mileageRate?.unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES; } @@ -727,7 +727,7 @@ function getRate({ isMovingTransactionFromTrackExpense, personalPolicyOutputCurrency, }: { - transaction: OnyxEntry; + transaction: ReadonlyDeep>; policy: OnyxEntry; policyDraft?: OnyxEntry; policyForMovingExpenses?: OnyxEntry; @@ -764,7 +764,7 @@ function getRate({ * For example, if an expense is '10 mi @ $1.00 / mi' and the rate is updated to '$1.00 / km', * then the updated distance unit should be 'km' from the updated rate, not 'mi' from the currently stored transaction distance unit. */ -function getUpdatedDistanceUnit({transaction, policy, policyDraft}: {transaction: OnyxEntry; policy: OnyxEntry; policyDraft?: OnyxEntry}) { +function getUpdatedDistanceUnit({transaction, policy, policyDraft}: {transaction: ReadonlyDeep>; policy: OnyxEntry; policyDraft?: OnyxEntry}) { // The distance unit doesn't depend on the currency (the rate is selected by ID/P2P, not currency), so // personalPolicyOutputCurrency isn't accepted here and is passed as undefined to getRate. return getRate({transaction, policy, policyDraft, useTransactionDistanceUnit: false, personalPolicyOutputCurrency: undefined}).unit; diff --git a/src/libs/TransactionUtils/index.ts b/src/libs/TransactionUtils/index.ts index cb849db7b9cb..902839b58059 100644 --- a/src/libs/TransactionUtils/index.ts +++ b/src/libs/TransactionUtils/index.ts @@ -10,6 +10,7 @@ import type {MergeDuplicatesParams} from '@libs/API/parameters'; import {convertAttendeesToArray, normalizeAttendees} from '@libs/AttendeeUtils'; import {isPersonalCard, isTravelCardTransaction} from '@libs/CardUtils'; import {getCategoryDefaultTaxRate, isCategoryMissing} from '@libs/CategoryUtils'; +import cloneMutable from '@libs/cloneMutable'; import {convertToBackendAmount} from '@libs/CurrencyUtils'; import type {MachineDateFormat} from '@libs/DateUtils'; import DateUtils from '@libs/DateUtils'; @@ -100,7 +101,6 @@ import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {format, isValid, parse} from 'date-fns'; import {SafeString, Str} from 'expensify-common'; import {deepEqual} from 'fast-equals'; -import cloneMutable from '@libs/cloneMutable'; import lodashSet from 'lodash/set'; import Onyx from 'react-native-onyx'; @@ -152,7 +152,7 @@ type TransactionParams = { type BuildOptimisticTransactionParams = { originalTransactionID?: string; existingTransactionID?: string; - existingTransaction?: OnyxEntry; + existingTransaction?: ReadonlyDeep>; policy?: OnyxEntry; transactionParams: TransactionParams; isDemoTransactionParam?: boolean; @@ -186,7 +186,7 @@ function isMapDistanceRequest(transaction: OnyxEntry): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; } -function isGPSDistanceRequest(transaction: OnyxEntry): boolean { +function isGPSDistanceRequest(transaction: ReadonlyDeep>): boolean { return transaction?.iouRequestType === CONST.IOU.REQUEST_TYPE.DISTANCE_GPS; } @@ -198,7 +198,7 @@ function isOdometerDistanceRequest(transaction: ReadonlyDeep): boolean { +function hasAppliedCommuterExclusion(transaction: ReadonlyDeep>): boolean { return isDistanceRequest(transaction) && (transaction?.comment?.customUnit?.commuterExclusion ?? 0) > 0; } @@ -2647,7 +2647,7 @@ function getEnabledTaxRateCount(options: TaxRates) { /** * Check if the customUnitRateID has a value default for P2P distance requests */ -function isCustomUnitRateIDForP2P(transaction: OnyxInputOrEntry): boolean { +function isCustomUnitRateIDForP2P(transaction: ReadonlyDeep>): boolean { return transaction?.comment?.customUnit?.customUnitRateID === CONST.CUSTOM_UNITS.FAKE_P2P_ID; } @@ -3531,7 +3531,7 @@ function willFieldBeAutomaticallyFilled(transaction: OnyxEntry, fie return autoFillableFields.includes(fieldType); } -function isExpenseUnreported(transaction?: Transaction): transaction is UnreportedTransaction { +function isExpenseUnreported(transaction?: ReadonlyDeep): transaction is ReadonlyDeep { return transaction?.reportID === CONST.REPORT.UNREPORTED_REPORT_ID; } diff --git a/src/libs/actions/IOU/Duplicate.ts b/src/libs/actions/IOU/Duplicate.ts index eb886b0f9977..bfb64710e794 100644 --- a/src/libs/actions/IOU/Duplicate.ts +++ b/src/libs/actions/IOU/Duplicate.ts @@ -53,7 +53,7 @@ import type {WaypointCollection} from '@src/types/onyx/Transaction'; import type {Locale as DateFnsLocale} from 'date-fns'; import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx'; -import type {PartialDeep} from 'type-fest'; +import type {PartialDeep, ReadonlyDeep} from 'type-fest'; import {format} from 'date-fns'; import Onyx from 'react-native-onyx'; @@ -774,7 +774,7 @@ type DuplicateExpenseTransactionParams = { targetPolicy?: OnyxEntry; targetPolicyCategories?: OnyxEntry; targetReport?: OnyxTypes.Report; - existingTransactionDraft: OnyxEntry; + existingTransactionDraft: ReadonlyDeep>; betas: OnyxEntry; personalDetails: OnyxEntry; recentWaypoints: OnyxEntry; @@ -1163,7 +1163,7 @@ type BulkDuplicateExpensesParams = { quickAction: OnyxEntry; policyRecentlyUsedCurrencies: string[]; isSelfTourViewed: boolean; - transactionDrafts: Record | undefined; + transactionDrafts: Record> | undefined; betas: OnyxEntry; recentWaypoints: OnyxEntry; currentUser: CurrentUser; diff --git a/src/libs/actions/IOU/MoneyRequest.ts b/src/libs/actions/IOU/MoneyRequest.ts index b6fd46e96813..b654bf0671b4 100644 --- a/src/libs/actions/IOU/MoneyRequest.ts +++ b/src/libs/actions/IOU/MoneyRequest.ts @@ -63,7 +63,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {Locale as DateFnsLocale} from 'date-fns'; import type {NullishDeep, OnyxCollection, OnyxEntry} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {format} from 'date-fns'; import Onyx from 'react-native-onyx'; @@ -883,7 +883,7 @@ function setLastSelectedDistanceRate(policy: OnyxEntry, customUnitRateID * Set the distance rate of a transaction. * Used when creating a new transaction or moving an existing one from Self DM */ -function setMoneyRequestDistanceRate(currentTransaction: OnyxEntry, customUnitRateID: string, policy: OnyxEntry, isDraft: boolean) { +function setMoneyRequestDistanceRate(currentTransaction: ReadonlyDeep>, customUnitRateID: string, policy: OnyxEntry, isDraft: boolean) { if (!currentTransaction) { Log.warn('setMoneyRequestDistanceRate is called without a valid transaction, skipping setting distance rate.'); return; diff --git a/src/libs/actions/IOU/MoneyRequestBuilder.ts b/src/libs/actions/IOU/MoneyRequestBuilder.ts index 9680e161ed89..449577ebb979 100644 --- a/src/libs/actions/IOU/MoneyRequestBuilder.ts +++ b/src/libs/actions/IOU/MoneyRequestBuilder.ts @@ -2,6 +2,7 @@ import type {LocaleContextProps} from '@components/LocaleContextProvider'; import type {CurrencyListActionsContextType} from '@hooks/useCurrencyList'; +import cloneMutable from '@libs/cloneMutable'; import DateUtils from '@libs/DateUtils'; import {getMicroSecondOnyxErrorObject, getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import {isLocalFile} from '@libs/fileDownload/FileUtils'; @@ -71,7 +72,7 @@ import type {Receipt, TransactionChanges, TransactionCustomUnit, WaypointCollect import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OnyxCollection, OnyxEntry, OnyxInputValue, OnyxUpdate} from 'react-native-onyx'; -import type {ValueOf} from 'type-fest'; +import type {ReadonlyDeep, ValueOf} from 'type-fest'; import {fastMerge} from 'expensify-common'; import Onyx from 'react-native-onyx'; @@ -188,8 +189,8 @@ type RequestMoneyInformation = { transactionViolations: OnyxCollection; quickAction: OnyxEntry; policyRecentlyUsedCurrencies: string[]; - existingTransactionDraft: OnyxEntry; - existingTransaction?: OnyxEntry; + existingTransactionDraft: ReadonlyDeep>; + existingTransaction?: ReadonlyDeep>; isSelfTourViewed: boolean; conciergeChat: OnyxEntry; betas: OnyxEntry; @@ -211,7 +212,7 @@ type MoneyRequestInformationParams = { moneyRequestReportID?: string; existingTransactionID?: string; optimisticTransactionID?: string; - existingTransaction?: OnyxEntry; + existingTransaction?: ReadonlyDeep>; retryParams?: StartSplitBilActionParams | CreateTrackExpenseParams | RequestMoneyInformation | ReplaceReceiptRetryParams; newReportTotal?: number; newNonReimbursableTotal?: number; @@ -1544,7 +1545,9 @@ function getMoneyRequestInformation(moneyRequestInformation: MoneyRequestInforma // Explicitly set merchant from splitExpense to ensure it's not overwritten optimisticTransaction.merchant = preservedMerchant; } else { - optimisticTransaction = fastMerge(existingTransaction, optimisticTransaction, false); + // `fastMerge` is typed `(target: TValue, source: TValue)`, so it cannot take a readonly + // target and a mutable source. The clone is the honest fix: the merge result is mutated in place below. + optimisticTransaction = fastMerge(cloneMutable(existingTransaction), optimisticTransaction, false); } } @@ -1574,7 +1577,9 @@ function getMoneyRequestInformation(moneyRequestInformation: MoneyRequestInforma } if (isSplitExpense && existingTransaction) { - const {convertedAmount: originalConvertedAmount, ...existingTransactionWithoutConvertedAmount} = existingTransaction; + // Clone before destructuring: a rest element of a readonly value keeps its nested arrays readonly, + // and `fastMerge` requires target and source to be the same type. + const {convertedAmount: originalConvertedAmount, ...existingTransactionWithoutConvertedAmount} = cloneMutable(existingTransaction); optimisticTransaction = fastMerge(existingTransactionWithoutConvertedAmount, optimisticTransaction, false); if (customUnit && optimisticTransaction.comment) { optimisticTransaction.comment.customUnit = customUnit; diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index 694af410fce7..4425460932c5 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -131,7 +131,7 @@ type CreateDistanceRequestInformation = { currentUserAccountID: number; iouType?: ValueOf; existingIOUReport?: OnyxEntry; - existingTransaction?: OnyxEntry; + existingTransaction?: ReadonlyDeep>; transactionParams: DistanceRequestTransactionParams; policyParams?: BasePolicyParams; isASAPSubmitBetaEnabled: boolean; diff --git a/src/libs/actions/IOU/TrackExpense.ts b/src/libs/actions/IOU/TrackExpense.ts index 9dc10bf44111..6906ddf6be75 100644 --- a/src/libs/actions/IOU/TrackExpense.ts +++ b/src/libs/actions/IOU/TrackExpense.ts @@ -7,6 +7,7 @@ import type {CurrencyListActionsContextType} from '@hooks/useCurrencyList'; import * as API from '@libs/API'; import type {AddTrackedExpenseToPolicyParams, CreateWorkspaceParams, DeleteMoneyRequestParams, RequestMoneyParams, ShareTrackedExpenseParams, TrackExpenseParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; +import cloneMutable from '@libs/cloneMutable'; import DateUtils from '@libs/DateUtils'; import {deferOrExecuteWrite} from '@libs/deferredLayoutWrite'; import DistanceRequestUtils from '@libs/DistanceRequestUtils'; @@ -105,6 +106,7 @@ import type {Receipt, ReceiptSource} from '@src/types/onyx/Transaction'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import type {OnyxCollection, OnyxEntry, OnyxInputValue, OnyxUpdate} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; import {fastMerge} from 'expensify-common'; import Onyx from 'react-native-onyx'; @@ -184,7 +186,7 @@ type GetTrackExpenseInformationParticipantParams = { type GetTrackExpenseInformationParams = { parentChatReport: OnyxEntry; moneyRequestReportID?: string; - existingTransaction?: OnyxEntry; + existingTransaction?: ReadonlyDeep>; existingTransactionID?: string; optimisticTransactionID?: string; participantParams: GetTrackExpenseInformationParticipantParams; @@ -1150,7 +1152,7 @@ function getTrackExpenseInformation(params: GetTrackExpenseInformationParams): T // I want to clean this up at some point, but it's possible this will live in the code for a while so I've created https://github.com/Expensify/App/issues/25417 // to remind me to do this. if (isDistanceRequest) { - optimisticTransaction = fastMerge(existingTransactionData, optimisticTransaction, false); + optimisticTransaction = fastMerge(cloneMutable(existingTransactionData), optimisticTransaction, false); } // STEP 4: Build optimistic reportActions. We need: diff --git a/src/selectors/TransactionDraft.ts b/src/selectors/TransactionDraft.ts index 7b35faf45473..28f2f2132462 100644 --- a/src/selectors/TransactionDraft.ts +++ b/src/selectors/TransactionDraft.ts @@ -1,16 +1,17 @@ import type Transaction from '@src/types/onyx/Transaction'; import type {OnyxCollection} from 'react-native-onyx'; +import type {ReadonlyDeep} from 'type-fest'; -const validTransactionDraftsSelector = (drafts: OnyxCollection): Record => - Object.values(drafts ?? {}).reduce>((acc, draft) => { +const validTransactionDraftsSelector = >(drafts: OnyxCollection): Record => + Object.values(drafts ?? {}).reduce>((acc, draft) => { if (draft) { acc[draft.transactionID] = draft; } return acc; }, {}); -const validTransactionDraftIDsSelector = (drafts: OnyxCollection): string[] => +const validTransactionDraftIDsSelector = (drafts: ReadonlyDeep>): string[] => Object.values(drafts ?? {}).reduce((acc, draft) => { if (draft) { acc.push(draft.transactionID); diff --git a/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts b/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts index 91dceb5526ae..aa98e5c16182 100644 --- a/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts +++ b/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts @@ -621,6 +621,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledTimes(1); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( @@ -648,6 +649,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -671,6 +673,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -707,6 +710,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -745,6 +749,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -789,6 +794,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledTimes(1); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( @@ -819,6 +825,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -847,6 +854,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -873,6 +881,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledWith( expect.objectContaining({ @@ -1125,6 +1134,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(mockShowConfirmModal).toHaveBeenCalled()); expect(bulkDuplicateExpenses).not.toHaveBeenCalled(); expect(mockShowConfirmModal).toHaveBeenCalledWith( @@ -1149,6 +1159,7 @@ describe('useSearchBulkActions - duplicate option', () => { }); result.current.headerButtonsOptions.find((o) => o.value === CONST.SEARCH.BULK_ACTION_TYPES.DUPLICATE)?.onSelected?.(); + await waitFor(() => expect(bulkDuplicateExpenses).toHaveBeenCalled()); expect(bulkDuplicateExpenses).toHaveBeenCalledTimes(1); expect(mockClearSelectedTransactions).toHaveBeenCalledTimes(1);