From 872a52a11283bab2dc048399b54a56606e1b1a8b Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 19 Mar 2026 12:21:56 +0100 Subject: [PATCH] Revert "Merge pull request #85248 from callstack-internal/VickyStash/refactor/bump-onyx-3.0.46" This reverts commit 572af3bbfda3d9c3bbe3d1f75596d3e649d1bd35, reversing changes made to b12722fd849b582f45bdcefba40f66de36c6b43b. --- package-lock.json | 8 +-- package.json | 2 +- .../MoneyRequestReportPreviewContent.tsx | 51 ++++++++----------- .../MoneyRequestReportPreview/index.tsx | 4 +- src/pages/workspace/withPolicy.tsx | 9 ++-- tests/actions/ReportTest.ts | 8 --- tests/actions/SessionTest.ts | 4 -- tests/unit/APITest.ts | 4 +- tests/unit/OptionsListUtilsTest.tsx | 25 ++++----- tests/unit/ReportSecondaryActionUtilsTest.ts | 6 +-- tests/unit/SequentialQueueTest.ts | 21 +++----- 11 files changed, 56 insertions(+), 86 deletions(-) diff --git a/package-lock.json b/package-lock.json index 122da3f4b7a2..3fa694ec58f0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -117,7 +117,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.46", + "react-native-onyx": "3.0.45", "react-native-pager-view": "8.0.0", "react-native-pdf": "7.0.2", "react-native-permissions": "^5.4.0", @@ -34741,9 +34741,9 @@ } }, "node_modules/react-native-onyx": { - "version": "3.0.46", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-3.0.46.tgz", - "integrity": "sha512-/dB5PrK+AZ4QiaFdIdb2iC1q5tu7L3n6pxh+QZJiMhDORlQT6jK832b/6sqwgQ3qIZCrrsnqmyLVKCqEbYTrlQ==", + "version": "3.0.45", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-3.0.45.tgz", + "integrity": "sha512-oJyizoazptOzKfY8ENDu0Y+QcFvCvRFySIEgQGOMZ0SDvmuni9MJ57zPd6/C/3n84GzTCJ5yDLmOFQRiHuPoRQ==", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", diff --git a/package.json b/package.json index 89c5000ba76c..3cf6a42d4ff8 100644 --- a/package.json +++ b/package.json @@ -181,7 +181,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.46", + "react-native-onyx": "3.0.45", "react-native-pager-view": "8.0.0", "react-native-pdf": "7.0.2", "react-native-permissions": "^5.4.0", diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx index 10d28496053f..a1536c827050 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx @@ -1,4 +1,4 @@ -import {useIsFocused} from '@react-navigation/native'; +import {useFocusEffect} from '@react-navigation/native'; import {hasSeenTourSelector} from '@selectors/Onboarding'; import {FlashList} from '@shopify/flash-list'; import type {FlashListRef, ListRenderItemInfo} from '@shopify/flash-list'; @@ -552,40 +552,31 @@ function MoneyRequestReportPreviewContent({ carouselTransactionsRef.current = carouselTransactions; }, [carouselTransactions]); - const isFocused = useIsFocused(); - const isFocusedRef = useRef(isFocused); + useFocusEffect( + useCallback(() => { + const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID)); - useEffect(() => { - isFocusedRef.current = isFocused; - }, [isFocused]); - - useEffect(() => { - const index = carouselTransactions.findIndex((transaction) => newTransactionIDs?.has(transaction.transactionID)); - - if (index < 0) { - return; - } - const newTransaction = carouselTransactions.at(index); - setTimeout(() => { - if (!isFocusedRef.current) { - return; - } - // 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) { + if (index < 0) { 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); + 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]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [newTransactionIDs]), + ); const onViewableItemsChanged = useRef(({viewableItems}: {viewableItems: ViewToken[]; changed: ViewToken[]}) => { const newIndex = viewableItems.at(0)?.index; diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx index 08e953542f52..9ce325e9dc7b 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx @@ -1,3 +1,4 @@ +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'; @@ -121,8 +122,9 @@ 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 = new Set(newTransactions.map((transaction) => transaction.transactionID)); + const newTransactionIDs = isFocused ? new Set(newTransactions.map((transaction) => transaction.transactionID)) : undefined; const transactionPreviewContainerStyles = [styles.h100, reportPreviewStyles.transactionPreviewCarouselStyle]; diff --git a/src/pages/workspace/withPolicy.tsx b/src/pages/workspace/withPolicy.tsx index 0d0377812d27..4a2e610b4e1d 100644 --- a/src/pages/workspace/withPolicy.tsx +++ b/src/pages/workspace/withPolicy.tsx @@ -1,5 +1,5 @@ import type {ComponentType} from 'react'; -import React, {useEffect} from 'react'; +import React from 'react'; import type {OnyxEntry} from 'react-native-onyx'; import useOnyx from '@hooks/useOnyx'; import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types'; @@ -91,12 +91,9 @@ export default function (WrappedComponent: Compo /* eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing */ const isLoadingPolicy = !hasLoadedApp || (!!policyID && isLoadingOnyxValue(policyResults, policyDraftResults)); - useEffect(() => { - if (!policyID) { - return; - } + if (policyID && policyID.length > 0) { updateLastAccessedWorkspace(policyID); - }, [policyID]); + } return ( { currentUserAccountID: TEST_USER_ACCOUNT_ID, }); - await waitForBatchedUpdates(); - // Need the reportActionID to delete the comments const newComment = PersistedRequests.getAll().at(1); const reportActionID = newComment?.data?.reportActionID as string | undefined; @@ -2041,9 +2039,6 @@ describe('actions/Report', () => { const newComment = PersistedRequests.getAll().at(0); const reportActionID = newComment?.data?.reportActionID as string | undefined; const reportAction = TestHelper.buildTestReportComment(created, TEST_USER_ACCOUNT_ID, reportActionID); - - await waitForBatchedUpdates(); - await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); // wait for Onyx.connect execute the callback and start processing the queue @@ -2237,7 +2232,6 @@ describe('actions/Report', () => { expect(requests?.at(0)?.data?.reportComment).toBe('value3'); await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); - await waitForBatchedUpdates(); TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.UPDATE_COMMENT, 1); }); @@ -2336,8 +2330,6 @@ describe('actions/Report', () => { expect(requests?.at(0)?.data?.reportComment).toBe('value3'); await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); - await waitForBatchedUpdates(); - TestHelper.expectAPICommandToHaveBeenCalled(WRITE_COMMANDS.UPDATE_COMMENT, 1); }); diff --git a/tests/actions/SessionTest.ts b/tests/actions/SessionTest.ts index 13b52bf0f4ee..94908d2eef7a 100644 --- a/tests/actions/SessionTest.ts +++ b/tests/actions/SessionTest.ts @@ -190,8 +190,6 @@ describe('Session', () => { await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); - await waitForBatchedUpdates(); - expect(getAllPersistedRequests().length).toBe(0); }); @@ -229,8 +227,6 @@ describe('Session', () => { await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); - await waitForBatchedUpdates(); - expect(getAllPersistedRequests().length).toBe(0); }); diff --git a/tests/unit/APITest.ts b/tests/unit/APITest.ts index 932beda22fa2..d6502055b09f 100644 --- a/tests/unit/APITest.ts +++ b/tests/unit/APITest.ts @@ -436,7 +436,7 @@ describe('APITests', () => { }); Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); - expect(NetworkStore.isOffline()).toBe(true); + expect(NetworkStore.isOffline()).toBe(false); expect(NetworkStore.isAuthenticating()).toBe(false); return waitForBatchedUpdates(); }) @@ -551,7 +551,7 @@ describe('APITests', () => { API.write('MockCommandThree' as WriteCommand, {}); // THEN the retryable requests should immediately be added to the persisted requests - expect(PersistedRequests.getLength()).toBe(2); + expect(PersistedRequests.getAll().length).toBe(2); // WHEN we wait for the queue to run and finish processing return waitForBatchedUpdates(); diff --git a/tests/unit/OptionsListUtilsTest.tsx b/tests/unit/OptionsListUtilsTest.tsx index 2ba7b1d614bd..0e09921d573d 100644 --- a/tests/unit/OptionsListUtilsTest.tsx +++ b/tests/unit/OptionsListUtilsTest.tsx @@ -3273,7 +3273,7 @@ describe('OptionsListUtils', () => { expect(canCreate).toBe(false); }); - it('createOptionList() localization', async () => { + it('createOptionList() localization', () => { renderLocaleContextProvider(); // Given a set of reports and personal details // When we call createOptionList and extract the reports @@ -3282,15 +3282,18 @@ describe('OptionsListUtils', () => { // Then the returned reports should match the expected values expect(reports.at(10)?.subtitle).toBe(`Submits to Mister Fantastic`); - await Onyx.set(ONYXKEYS.NVP_PREFERRED_LOCALE, CONST.LOCALES.ES); - - await waitForBatchedUpdates(); - - // When we call createOptionList again - const newReports = createOptionList(PERSONAL_DETAILS, CURRENT_USER_ACCOUNT_ID, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS).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'); + 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, CURRENT_USER_ACCOUNT_ID, EMPTY_PRIVATE_IS_ARCHIVED_MAP, REPORTS).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'); + }) + ); }); }); @@ -3364,8 +3367,6 @@ 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, diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index cd3755c6af04..23e31ea0de04 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -52,16 +52,14 @@ 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', () => { diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index 3ea245876b82..805fdb8f3412 100644 --- a/tests/unit/SequentialQueueTest.ts +++ b/tests/unit/SequentialQueueTest.ts @@ -53,10 +53,10 @@ describe('SequentialQueue', () => { }; SequentialQueue.push(requestWithConflictResolution); expect(getLength()).toBe(1); - // 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); + // 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); }); it('should push two requests with conflict resolution and push', () => { @@ -109,9 +109,7 @@ describe('SequentialQueue', () => { }; SequentialQueue.push(requestWithConflictResolution); - - const ongoingRequest = getOngoingRequest(); - expect(ongoingRequest?.data?.accountID).toBe(56789); + expect(getLength()).toBe(2); }); it('should replace request request in queue while a similar one is ongoing', async () => { @@ -177,14 +175,9 @@ describe('SequentialQueue', () => { expect(getLength()).toBe(4); const persistedRequests = getAll(); - 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 + // We know ReconnectApp is at index 1 in the queue, so we can get it to verify // that was replaced by the new request. - expect(persistedRequests.at(0)?.data?.accountID).toBe(56789); + expect(persistedRequests.at(1)?.data?.accountID).toBe(56789); }); // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver