Merge pull request #85919 from callstack-internal/VickyStash/refactor/82871-bump-onyx-3.0.46

Onyx bump to v3.0.61

Author: mountiny

package-lock.json

@@ -177,7 +177,7 @@
     "react-native-localize": "^3.5.4",
     "react-native-nitro-modules": "0.29.4",
     "react-native-nitro-sqlite": "9.2.0",
-    "react-native-onyx": "3.0.60",
+    "react-native-onyx": "3.0.61",
     "react-native-pager-view": "8.0.0",
     "react-native-pdf": "7.0.2",
     "react-native-permissions": "^5.4.0",

package.json

@@ -1,6 +1,6 @@
 # `react-native-onyx` patches
 
-### [react-native-onyx+3.0.60.patch](react-native-onyx+3.0.60.patch)
+### [react-native-onyx+3.0.61.patch](react-native-onyx+3.0.61.patch)
 
 - Reason: Onyx v3.0.59 ([PR #756](https://github.com/Expensify/react-native-onyx/pull/756)) added a state reset inside the `subscribe` callback of `useOnyx` to fix stale data when keys change dynamically. However, this reset runs unconditionally — including on initial mount — which causes `useSyncExternalStore` to see a new snapshot reference after subscription, triggering one extra render per `useOnyx` hook. This patch guards the reset with a `hasMountedRef` flag so it only runs on key-change re-subscriptions, not on initial mount.
 - E/App issue: https://github.com/Expensify/App/issues/85416

patches/react-native-onyx/details.md

@@ -1,7 +1,7 @@
-import {useFocusEffect} from '@react-navigation/native';
+import {useIsFocused} from '@react-navigation/native';
 import {FlashList} from '@shopify/flash-list';
 import type {FlashListRef, ListRenderItemInfo} from '@shopify/flash-list';
-import React, {useCallback, useDeferredValue, useEffect, useMemo, useRef, useState} from 'react';
+import React, {useCallback, useDeferredValue, useEffect, useEffectEvent, useMemo, useRef, useState} from 'react';
 import {View} from 'react-native';
 import type {ViewToken} from 'react-native';
 import type {OnyxEntry} from 'react-native-onyx';
@@ -375,31 +375,39 @@ function MoneyRequestReportPreviewContent({
         carouselTransactionsRef.current = carouselTransactions;
     }, [carouselTransactions]);
 
-    useFocusEffect(
-        useCallback(() => {
-            const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID));
+    const isFocused = useIsFocused();
+    const getIsFocused = useEffectEvent(() => {
+        return isFocused;
+    });
+
+    useEffect(() => {
+        const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID));
 
-            if (index < 0) {
+        if (index < 0) {
+            return;
+        }
+        const newTransaction = carouselTransactions.at(index);
+        setTimeout(() => {
+            if (!getIsFocused()) {
                 return;
             }
-            const newTransaction = carouselTransactions.at(index);
-            setTimeout(() => {
-                // If the new transaction is not available at the index it was on before the delay, avoid the scrolling
-                // because we are scrolling to either a wrong or unavailable transaction (which can cause crash).
-                if (newTransaction?.transactionID !== carouselTransactionsRef.current.at(index)?.transactionID) {
-                    return;
-                }
-
-                carouselRef.current?.scrollToIndex({
-                    index,
-                    viewOffset: -2 * styles.gap2.gap,
-                    animated: true,
-                });
-            }, CONST.ANIMATED_TRANSITION);
-
-            // eslint-disable-next-line react-hooks/exhaustive-deps
-        }, [newTransactionIDs]),
-    );
+
+            // If the new transaction is not available at the index it was on before the delay, avoid the scrolling
+            // because we are scrolling to either a wrong or unavailable transaction (which can cause crash).
+            if (newTransaction?.transactionID !== carouselTransactionsRef.current.at(index)?.transactionID) {
+                return;
+            }
+
+            carouselRef.current?.scrollToIndex({
+                index,
+                viewOffset: -2 * styles.gap2.gap,
+                animated: true,
+            });
+        }, CONST.ANIMATED_TRANSITION);
+
+        // We only want to scroll to a new transaction when the set of new transaction IDs changes.
+        // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [newTransactionIDs]);
 
     const onViewableItemsChanged = useRef(({viewableItems}: {viewableItems: ViewToken[]; changed: ViewToken[]}) => {
         const newIndex = viewableItems.at(0)?.index;

…tive-onyx/react-native-onyx+3.0.60.patch …tive-onyx/react-native-onyx+3.0.61.patchpatches/react-native-onyx/react-native-onyx+3.0.60.patch renamed to patches/react-native-onyx/react-native-onyx+3.0.61.patch patches/react-native-onyx/react-native-onyx+3.0.60.patch renamed to patches/react-native-onyx/react-native-onyx+3.0.61.patch

@@ -1,4 +1,3 @@
-import {useIsFocused} from '@react-navigation/native';
 import type {ListRenderItem} from '@shopify/flash-list';
 import React, {useCallback, useMemo, useRef, useState} from 'react';
 import type {LayoutChangeEvent} from 'react-native';
@@ -123,9 +122,7 @@ function MoneyRequestReportPreview({
         selector: hasOnceLoadedReportActionsSelector,
     });
     const newTransactions = useNewTransactions(hasOnceLoadedReportActions, transactions);
-    const isFocused = useIsFocused();
-    // We only want to highlight the new expenses if the screen is focused.
-    const newTransactionIDs = isFocused ? new Set(newTransactions.map((transaction) => transaction.transactionID)) : undefined;
+    const newTransactionIDs = new Set(newTransactions.map((transaction) => transaction.transactionID));
 
     const transactionPreviewContainerStyles = [styles.h100, reportPreviewStyles.transactionPreviewCarouselStyle];
 

src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx

@@ -1,5 +1,5 @@
 import type {ComponentType} from 'react';
-import React from 'react';
+import React, {useEffect} from 'react';
 import type {OnyxEntry} from 'react-native-onyx';
 import useOnyx from '@hooks/useOnyx';
 import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types';
@@ -94,9 +94,12 @@ export default function <TProps extends WithPolicyProps>(WrappedComponent: Compo
         /* eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing */
         const isLoadingPolicy = !hasLoadedApp || (!!policyID && isLoadingOnyxValue(policyResults, policyDraftResults));
 
-        if (policyID && policyID.length > 0) {
+        useEffect(() => {
+            if (!policyID) {
+                return;
+            }
             updateLastAccessedWorkspace(policyID);
-        }
+        }, [policyID]);
 
         return (
             <WrappedComponent

src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx

@@ -3513,7 +3513,7 @@ describe('OptionsListUtils', () => {
             expect(canCreate).toBe(false);
         });
 
-        it('createOptionList() localization', () => {
+        it('createOptionList() localization', async () => {
             renderLocaleContextProvider();
             // Given a set of reports and personal details
             // When we call createOptionList and extract the reports
@@ -3522,18 +3522,15 @@ describe('OptionsListUtils', () => {
             // Then the returned reports should match the expected values
             expect(reports.at(10)?.subtitle).toBe(`Submits to Mister Fantastic`);
 
-            return (
-                waitForBatchedUpdates()
-                    // When we set the preferred locale to Spanish
-                    .then(() => Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.ES))
-                    .then(() => {
-                        // When we call createOptionList again
-                        const newReports = createOptionList(PERSONAL_DETAILS, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS, undefined).reports;
-                        // Then the returned reports should change to Spanish
-                        // cspell:disable-next-line
-                        expect(newReports.at(10)?.subtitle).toBe('Se envía a Mister Fantastic');
-                    })
-            );
+            await Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.ES);
+
+            await waitForBatchedUpdates();
+
+            // When we call createOptionList again
+            const newReports = createOptionList(PERSONAL_DETAILS, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS, undefined).reports;
+            // Then the returned reports should change to Spanish
+            // cspell:disable-next-line
+            expect(newReports.at(10)?.subtitle).toBe('Se envía a Mister Fantastic');
         });
     });
 
@@ -3607,6 +3604,8 @@ describe('OptionsListUtils', () => {
                     '1': getFakeAdvancedReportAction(CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT),
                 },
             });
+            await waitForBatchedUpdates();
+
             // When we call createOptionList with report 10 marked as archived
             const archivedMap: PrivateIsArchivedMap = {
                 [`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}10`]: !!reportNameValuePairs.private_isArchived,

src/pages/workspace/withPolicy.tsx

@@ -54,14 +54,16 @@ describe('getSecondaryAction', () => {
     beforeAll(() => {
         Onyx.init({
             keys: ONYXKEYS,
+            initialKeyStates: {
+                [ONYXKEYS.SESSION]: SESSION,
+                [ONYXKEYS.PERSONAL_DETAILS_LIST]: {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}},
+            },
         });
     });
 
     beforeEach(async () => {
         jest.clearAllMocks();
         Onyx.clear();
-        await Onyx.merge(ONYXKEYS.SESSION, SESSION);
-        await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, {[EMPLOYEE_ACCOUNT_ID]: PERSONAL_DETAILS, [APPROVER_ACCOUNT_ID]: {accountID: APPROVER_ACCOUNT_ID, login: APPROVER_EMAIL}});
     });
 
     it('should always return default options', () => {

tests/unit/OptionsListUtilsTest.tsx

@@ -53,10 +53,10 @@ describe('SequentialQueue', () => {
         };
         SequentialQueue.push(requestWithConflictResolution);
         expect(getLength()).toBe(1);
-        // We know there is only one request in the queue, so we can get the first one and verify
-        // that the persisted request is the second one.
-        const persistedRequest = getAll().at(0);
-        expect(persistedRequest?.data?.accountID).toBe(56789);
+        // We know there is only one request and it's ongoing.
+        // We can get it and verify that the ongoing request is the second one.
+        const ongoingRequest = getOngoingRequest();
+        expect(ongoingRequest?.data?.accountID).toBe(56789);
     });
 
     it('should push two requests with conflict resolution and push', () => {
@@ -109,7 +109,9 @@ describe('SequentialQueue', () => {
         };
 
         SequentialQueue.push(requestWithConflictResolution);
-        expect(getLength()).toBe(2);
+
+        const ongoingRequest = getOngoingRequest();
+        expect(ongoingRequest?.data?.accountID).toBe(56789);
     });
 
     it('should replace request request in queue while a similar one is ongoing', async () => {
@@ -175,9 +177,14 @@ describe('SequentialQueue', () => {
 
         expect(getLength()).toBe(4);
         const persistedRequests = getAll();
-        // We know ReconnectApp is at index 1 in the queue, so we can get it to verify
+        const ongoingRequest = getOngoingRequest();
+
+        // The first OpenReport call is ongoing
+        expect(ongoingRequest?.command).toBe('OpenReport');
+
+        // We know ReconnectApp is at index 0 in the queue now, so we can get it to verify
         // that was replaced by the new request.
-        expect(persistedRequests.at(1)?.data?.accountID).toBe(56789);
+        expect(persistedRequests.at(0)?.data?.accountID).toBe(56789);
     });
 
     // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver