diff --git a/package-lock.json b/package-lock.json index 3e99e1369e52..0478e7ed43ca 100644 --- a/package-lock.json +++ b/package-lock.json @@ -114,7 +114,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", @@ -34702,9 +34702,9 @@ } }, "node_modules/react-native-onyx": { - "version": "3.0.60", - "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-3.0.60.tgz", - "integrity": "sha512-rW5pTGYcnpQv3vNv9IvBpvIFg3McY3f4jDe96eQbkxeRyzm592J8dfZAKjhavkoJkRo0gLP+oInxRvVOQihfYw==", + "version": "3.0.61", + "resolved": "https://registry.npmjs.org/react-native-onyx/-/react-native-onyx-3.0.61.tgz", + "integrity": "sha512-heC6wja1BjarvV5o0V3DxZzu8ZU4owIrkPnh8tSYmg1TT2edXqNMnAtfVbC1WjXkkYOcKBmQbAaudNvyn9Ro7Q==", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", diff --git a/package.json b/package.json index 50720dea983c..6e605a6d281c 100644 --- a/package.json +++ b/package.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", diff --git a/patches/react-native-onyx/details.md b/patches/react-native-onyx/details.md index 29ad7ca30d77..6bb9930887b9 100644 --- a/patches/react-native-onyx/details.md +++ b/patches/react-native-onyx/details.md @@ -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 diff --git a/patches/react-native-onyx/react-native-onyx+3.0.60.patch b/patches/react-native-onyx/react-native-onyx+3.0.61.patch similarity index 100% rename from patches/react-native-onyx/react-native-onyx+3.0.60.patch rename to patches/react-native-onyx/react-native-onyx+3.0.61.patch diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx index 7b508606ff22..74a4ff1d886a 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/MoneyRequestReportPreviewContent.tsx @@ -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; diff --git a/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx b/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx index d9a62a7f9e29..291c77e8e668 100644 --- a/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx +++ b/src/components/ReportActionItem/MoneyRequestReportPreview/index.tsx @@ -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]; diff --git a/src/pages/workspace/withPolicy.tsx b/src/pages/workspace/withPolicy.tsx index 43c2aa669354..f58b9fca6640 100644 --- a/src/pages/workspace/withPolicy.tsx +++ b/src/pages/workspace/withPolicy.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 (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 ( { 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 @@ -3458,18 +3458,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'); }); }); @@ -3543,6 +3540,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, diff --git a/tests/unit/ReportSecondaryActionUtilsTest.ts b/tests/unit/ReportSecondaryActionUtilsTest.ts index e099db856bf3..726db7b49739 100644 --- a/tests/unit/ReportSecondaryActionUtilsTest.ts +++ b/tests/unit/ReportSecondaryActionUtilsTest.ts @@ -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', () => { diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts index 805fdb8f3412..3ea245876b82 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 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