From 7d9220c1e0af3dd033b312e094446e187d9633f9 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 8 Oct 2026 17:37:26 +0200 Subject: [PATCH 1/3] Replace PERSONAL_AND_WORKSPACE_CARD_LIST derived value with a memoized useOnyxState hook --- __mocks__/react-native-onyx.ts | 4 +- src/ONYXKEYS.ts | 2 - .../Search/SearchAutocompleteList.tsx | 3 +- .../Search/SearchRouter/SearchRouter.tsx | 3 +- .../Search/hooks/useFilterCardValue.tsx | 5 +- .../Search/hooks/useFilterFeedData.tsx | 3 +- .../Search/hooks/useSearchSnapshot.ts | 3 +- src/hooks/useAdvancedSearchFilters.ts | 3 +- src/hooks/usePersonalAndWorkspaceCardList.ts | 42 ++++++ src/hooks/useSearchSections.ts | 3 +- .../OnyxDerived/ONYX_DERIVED_VALUES.ts | 2 - .../configs/personalAndWorkspaceCardList.ts | 14 -- src/selectors/Card.ts | 42 +++++- src/types/onyx/DerivedValues.ts | 6 - src/types/onyx/index.ts | 2 - tests/unit/OnyxDerivedTest.tsx | 77 ---------- tests/unit/hooks/useFilterFeedDataTest.ts | 6 +- .../unit/pages/Search/SearchSavePageTest.tsx | 5 +- tests/unit/selectors/CardTest.ts | 91 +++++++++++- .../usePersonalAndWorkspaceCardListTest.tsx | 137 ++++++++++++++++++ 20 files changed, 334 insertions(+), 119 deletions(-) create mode 100644 src/hooks/usePersonalAndWorkspaceCardList.ts delete mode 100644 src/libs/actions/OnyxDerived/configs/personalAndWorkspaceCardList.ts create mode 100644 tests/unit/usePersonalAndWorkspaceCardListTest.tsx diff --git a/__mocks__/react-native-onyx.ts b/__mocks__/react-native-onyx.ts index 6db22ac1b627..f5b09f2949da 100644 --- a/__mocks__/react-native-onyx.ts +++ b/__mocks__/react-native-onyx.ts @@ -5,7 +5,7 @@ import type {ConnectOptions, OnyxKey} from 'react-native-onyx'; // eslint-disable-next-line no-restricted-imports -import Onyx, {useOnyx} from 'react-native-onyx'; +import Onyx, {useOnyx, useOnyxState} from 'react-native-onyx'; let connectCallbackDelay = 0; function addDelayToConnectCallback(delay: number) { @@ -41,4 +41,4 @@ const reactNativeOnyxMock: ReactNativeOnyxMock = { export default reactNativeOnyxMock; -export {useOnyx}; +export {useOnyx, useOnyxState}; diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index f870557af1e2..da7af4bd51b7 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -1484,7 +1484,6 @@ const ONYXKEYS = { OUTSTANDING_REPORTS_BY_POLICY_ID: 'outstandingReportsByPolicyID', VISIBLE_REPORT_ACTIONS: 'visibleReportActions', NON_PERSONAL_AND_WORKSPACE_CARD_LIST: 'nonPersonalAndWorkspaceCardList', - PERSONAL_AND_WORKSPACE_CARD_LIST: 'personalAndWorkspaceCardList', CARD_FEED_ERRORS: 'cardFeedErrors', RAM_ONLY_SORTED_REPORT_ACTIONS: 'sortedReportActions', LOGIN_TO_ACCOUNT_ID_MAP: 'loginToAccountIDMap', @@ -2085,7 +2084,6 @@ type OnyxDerivedValuesMapping = { [ONYXKEYS.DERIVED.OUTSTANDING_REPORTS_BY_POLICY_ID]: OnyxTypes.OutstandingReportsByPolicyIDDerivedValue; [ONYXKEYS.DERIVED.VISIBLE_REPORT_ACTIONS]: OnyxTypes.VisibleReportActionsDerivedValue; [ONYXKEYS.DERIVED.NON_PERSONAL_AND_WORKSPACE_CARD_LIST]: OnyxTypes.NonPersonalAndWorkspaceCardListDerivedValue; - [ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST]: OnyxTypes.PersonalAndWorkspaceCardListDerivedValue; [ONYXKEYS.DERIVED.CARD_FEED_ERRORS]: OnyxTypes.CardFeedErrorsDerivedValue; [ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS]: OnyxTypes.SortedReportActionsDerivedValue; [ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP]: OnyxTypes.LoginToAccountIDMapDerivedValue; diff --git a/src/components/Search/SearchAutocompleteList.tsx b/src/components/Search/SearchAutocompleteList.tsx index 209f0f8c831d..0674f0d5d9f0 100644 --- a/src/components/Search/SearchAutocompleteList.tsx +++ b/src/components/Search/SearchAutocompleteList.tsx @@ -19,6 +19,7 @@ import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; import usePermissions from '@hooks/usePermissions'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import useReportAttributes from '@hooks/useReportAttributes'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; import useSortedReportActionsData from '@hooks/useSortedReportActionsData'; @@ -224,7 +225,7 @@ function SearchAutocompleteList({ const lastActions = sortedReportActionsData?.lastActions; const personalDetails = usePersonalDetails(); const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT); - const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); + const personalAndWorkspaceCards = usePersonalAndWorkspaceCardList(); const [allFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); const allCards = personalAndWorkspaceCards ?? CONST.EMPTY_OBJECT; diff --git a/src/components/Search/SearchRouter/SearchRouter.tsx b/src/components/Search/SearchRouter/SearchRouter.tsx index 91e3aaf69e1f..e0c5ff6ddea5 100644 --- a/src/components/Search/SearchRouter/SearchRouter.tsx +++ b/src/components/Search/SearchRouter/SearchRouter.tsx @@ -19,6 +19,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import {useGetPersonalDetailsByLogin} from '@hooks/usePersonalDetailByLogin'; import useReportAttributes from '@hooks/useReportAttributes'; import useReportOrReportDraft from '@hooks/useReportOrReportDraft'; @@ -131,7 +132,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla const {currentSearchQueryJSON, currentSearchHash, currentSearchKey} = useSearchQueryContext(); const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT); const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY); - const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); + const personalAndWorkspaceCards = usePersonalAndWorkspaceCardList(); const [allFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); const getPersonalDetailsByLogin = useGetPersonalDetailsByLogin(); diff --git a/src/components/Search/hooks/useFilterCardValue.tsx b/src/components/Search/hooks/useFilterCardValue.tsx index a0b2510cbb20..d84c95ddad88 100644 --- a/src/components/Search/hooks/useFilterCardValue.tsx +++ b/src/components/Search/hooks/useFilterCardValue.tsx @@ -1,14 +1,13 @@ import useLocalize from '@hooks/useLocalize'; -import useOnyx from '@hooks/useOnyx'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import {getCardDescription} from '@libs/CardUtils'; -import ONYXKEYS from '@src/ONYXKEYS'; import {filterCardsHiddenFromSearch} from '@src/selectors/Card'; function useFilterCardValue(value: string[]): string { const {translate} = useLocalize(); - const [searchCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST, {selector: filterCardsHiddenFromSearch}); + const searchCards = usePersonalAndWorkspaceCardList(filterCardsHiddenFromSearch); const cardNames = Object.values(searchCards ?? {}) .filter((card) => value.includes(card.cardID.toString())) diff --git a/src/components/Search/hooks/useFilterFeedData.tsx b/src/components/Search/hooks/useFilterFeedData.tsx index 8e73e3676312..0a852357dd53 100644 --- a/src/components/Search/hooks/useFilterFeedData.tsx +++ b/src/components/Search/hooks/useFilterFeedData.tsx @@ -1,6 +1,7 @@ import useFeedKeysWithAssignedCards from '@hooks/useFeedKeysWithAssignedCards'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import {getFeedOptions} from '@libs/SearchUIUtils'; @@ -9,7 +10,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; function useFilterFeedData(value: string[] | undefined) { const {translate, localeCompare} = useLocalize(); const [allFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); - const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); + const personalAndWorkspaceCards = usePersonalAndWorkspaceCardList(); const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY); const [domains] = useOnyx(ONYXKEYS.COLLECTION.DOMAIN); const [expensifyCardSettings] = useOnyx(ONYXKEYS.COLLECTION.PRIVATE_EXPENSIFY_CARD_SETTINGS); diff --git a/src/components/Search/hooks/useSearchSnapshot.ts b/src/components/Search/hooks/useSearchSnapshot.ts index f29acd32ae8f..2df821545a21 100644 --- a/src/components/Search/hooks/useSearchSnapshot.ts +++ b/src/components/Search/hooks/useSearchSnapshot.ts @@ -9,6 +9,7 @@ import useLocalize from '@hooks/useLocalize'; import useMultipleSnapshots from '@hooks/useMultipleSnapshots'; import useNetwork from '@hooks/useNetwork'; import useOnyx from '@hooks/useOnyx'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import {useAllPersonalDetails} from '@hooks/usePersonalDetails'; import usePolicyForMovingExpenses from '@hooks/usePolicyForMovingExpenses'; import useReportAttributes from '@hooks/useReportAttributes'; @@ -169,7 +170,7 @@ function useSearchSnapshot({queryJSON, searchResults, transactions, reportAction const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); const [onyxPersonalDetailsList] = useAllPersonalDetails(); const [cardFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); - const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); + const personalAndWorkspaceCards = usePersonalAndWorkspaceCardList(); const [nonPersonalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.NON_PERSONAL_AND_WORKSPACE_CARD_LIST); const [customCardNames] = useOnyx(ONYXKEYS.NVP_EXPENSIFY_COMPANY_CARDS_CUSTOM_NAMES); const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID); diff --git a/src/hooks/useAdvancedSearchFilters.ts b/src/hooks/useAdvancedSearchFilters.ts index c2f63fa2915e..66b66dea13a6 100644 --- a/src/hooks/useAdvancedSearchFilters.ts +++ b/src/hooks/useAdvancedSearchFilters.ts @@ -15,6 +15,7 @@ import {emailSelector} from '@selectors/Session'; import useLocalize from './useLocalize'; import useOnyx from './useOnyx'; import usePermissions from './usePermissions'; +import usePersonalAndWorkspaceCardList from './usePersonalAndWorkspaceCardList'; import useWorkspaceList from './useWorkspaceList'; /** @@ -312,7 +313,7 @@ function shouldDisplayCardFilterSelector(cardList: OnyxEntry) { } function useAdvancedSearchFilters(type: SearchDataTypes | undefined) { - const [shouldDisplayCardFilter] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST, {selector: shouldDisplayCardFilterSelector}); + const shouldDisplayCardFilter = usePersonalAndWorkspaceCardList(shouldDisplayCardFilterSelector); const [policies = getEmptyObject>>()] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: advancedSearchPoliciesSelector}); const [policyDerived] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: policyDerivedSelector}); const {isBetaEnabled} = usePermissions(); diff --git a/src/hooks/usePersonalAndWorkspaceCardList.ts b/src/hooks/usePersonalAndWorkspaceCardList.ts new file mode 100644 index 000000000000..086aaeb279a7 --- /dev/null +++ b/src/hooks/usePersonalAndWorkspaceCardList.ts @@ -0,0 +1,42 @@ +import ONYXKEYS from '@src/ONYXKEYS'; +import type {CardList, WorkspaceCardsList} from '@src/types/onyx'; + +import type {OnyxCollection} from 'react-native-onyx'; + +import {personalAndWorkspaceCardListSelector} from '@selectors/Card'; +import {useOnyxState} from 'react-native-onyx'; + +type CardListSelector = (cardList: CardList) => TReturn; + +const dependencies = [ONYXKEYS.CARD_LIST, ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST]; + +// useOnyxState runs the selector on every write to either key, so each selector's output is kept per merged list +const selectedValues = new WeakMap, {cardList: CardList; value: unknown}>(); + +function select(selector: CardListSelector, cardList: CardList): TReturn { + const cached = selectedValues.get(selector); + if (cached?.cardList === cardList) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return cached.value as TReturn; + } + const value = selector(cardList); + selectedValues.set(selector, {cardList, value}); + return value; +} + +function usePersonalAndWorkspaceCardList(): CardList; +function usePersonalAndWorkspaceCardList(selector: CardListSelector): TReturn; +function usePersonalAndWorkspaceCardList(selector?: CardListSelector) { + return useOnyxState( + (state) => { + // useOnyxState's state view types a collection key as one member, but at runtime it returns the whole collection + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + const workspaceCardFeeds = state[ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST] as OnyxCollection; + const cardList = personalAndWorkspaceCardListSelector(state[ONYXKEYS.CARD_LIST], workspaceCardFeeds); + return selector ? select(selector, cardList) : cardList; + }, + {dependencies}, + ); +} + +export default usePersonalAndWorkspaceCardList; diff --git a/src/hooks/useSearchSections.ts b/src/hooks/useSearchSections.ts index 19733b0ac474..f3e9960b3a18 100644 --- a/src/hooks/useSearchSections.ts +++ b/src/hooks/useSearchSections.ts @@ -11,6 +11,7 @@ import useCurrentUserPersonalDetails from './useCurrentUserPersonalDetails'; import useFilterPendingDeleteReports from './useFilterPendingDeleteReports'; import useLocalize from './useLocalize'; import useOnyx from './useOnyx'; +import usePersonalAndWorkspaceCardList from './usePersonalAndWorkspaceCardList'; import useReportAttributes from './useReportAttributes'; type UseSearchSectionsResult = { @@ -28,7 +29,7 @@ function useSearchSections(): UseSearchSectionsResult { const {convertToDisplayString} = useCurrencyListActions(); const [cardFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); - const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); + const personalAndWorkspaceCards = usePersonalAndWorkspaceCardList(); const [nonPersonalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.NON_PERSONAL_AND_WORKSPACE_CARD_LIST); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); diff --git a/src/libs/actions/OnyxDerived/ONYX_DERIVED_VALUES.ts b/src/libs/actions/OnyxDerived/ONYX_DERIVED_VALUES.ts index 4ea4fabd9560..d01d9759f785 100644 --- a/src/libs/actions/OnyxDerived/ONYX_DERIVED_VALUES.ts +++ b/src/libs/actions/OnyxDerived/ONYX_DERIVED_VALUES.ts @@ -10,7 +10,6 @@ import loginToAccountIDMapConfig from './configs/loginToAccountIDMap'; import nonPersonalAndWorkspaceCardListConfig from './configs/nonPersonalAndWorkspaceCardList'; import oneOnOneChatReportIDsConfig from './configs/oneOnOneChatReportIDs'; import outstandingReportsByPolicyIDConfig from './configs/outstandingReportsByPolicyID'; -import personalAndWorkspaceCardListConfig from './configs/personalAndWorkspaceCardList'; import reportAttributesConfig from './configs/reportAttributes'; import reportTransactionsAndViolationsConfig from './configs/reportTransactionsAndViolations'; import sortedReportActionsConfig from './configs/sortedReportActions'; @@ -27,7 +26,6 @@ const ONYX_DERIVED_VALUES = { [ONYXKEYS.DERIVED.OUTSTANDING_REPORTS_BY_POLICY_ID]: outstandingReportsByPolicyIDConfig, [ONYXKEYS.DERIVED.VISIBLE_REPORT_ACTIONS]: visibleReportActionsConfig, [ONYXKEYS.DERIVED.NON_PERSONAL_AND_WORKSPACE_CARD_LIST]: nonPersonalAndWorkspaceCardListConfig, - [ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST]: personalAndWorkspaceCardListConfig, [ONYXKEYS.DERIVED.CARD_FEED_ERRORS]: cardFeedErrorsConfig, [ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS]: sortedReportActionsConfig, [ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP]: loginToAccountIDMapConfig, diff --git a/src/libs/actions/OnyxDerived/configs/personalAndWorkspaceCardList.ts b/src/libs/actions/OnyxDerived/configs/personalAndWorkspaceCardList.ts deleted file mode 100644 index fe52455ddc82..000000000000 --- a/src/libs/actions/OnyxDerived/configs/personalAndWorkspaceCardList.ts +++ /dev/null @@ -1,14 +0,0 @@ -import {mergeCardListWithWorkspaceFeeds} from '@libs/CardUtils'; - -import createOnyxDerivedValueConfig from '@userActions/OnyxDerived/createOnyxDerivedValueConfig'; - -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; - -export default createOnyxDerivedValueConfig({ - key: ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST, - dependencies: [ONYXKEYS.CARD_LIST, ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST], - compute: ([cardList, workspaceCardFeeds]) => { - return mergeCardListWithWorkspaceFeeds(workspaceCardFeeds ?? CONST.EMPTY_OBJECT, cardList); - }, -}); diff --git a/src/selectors/Card.ts b/src/selectors/Card.ts index 7a75744ff56c..64dfff36e0e4 100644 --- a/src/selectors/Card.ts +++ b/src/selectors/Card.ts @@ -1,5 +1,15 @@ import {getExpensifyCardFeedsForDisplay} from '@libs/CardFeedUtils'; -import {hasActiveExpensifyCard, hasAssignedCardMatching, isActiveCard, isCard, isCardHiddenFromSearch, isCSVFeedOrExpensifyCard, isExpensifyCard, isPersonalCard} from '@libs/CardUtils'; +import { + hasActiveExpensifyCard, + hasAssignedCardMatching, + isActiveCard, + isCard, + isCardHiddenFromSearch, + isCSVFeedOrExpensifyCard, + isExpensifyCard, + isPersonalCard, + mergeCardListWithWorkspaceFeeds, +} from '@libs/CardUtils'; import {filterObject} from '@libs/ObjectUtils'; import CONST from '@src/CONST'; @@ -8,6 +18,8 @@ import type {CardFeeds, CardList, NonPersonalAndWorkspaceCardListDerivedValue, W import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; +import {deepEqual} from 'fast-equals'; + /** * Builds a lightweight map of "${domainID}_${feedName}" keys that have card entries. * A feed counts as having cards when: @@ -87,6 +99,33 @@ const defaultExpensifyCardSelector = (allCards: OnyxEntry { + let lastCardList: OnyxEntry; + let lastWorkspaceCardFeeds: OnyxCollection; + let lastResult: CardList | undefined; + + return (cardList: OnyxEntry, workspaceCardFeeds: OnyxCollection): CardList => { + if (lastResult && cardList === lastCardList && workspaceCardFeeds === lastWorkspaceCardFeeds) { + return lastResult; + } + lastCardList = cardList; + lastWorkspaceCardFeeds = workspaceCardFeeds; + const result = mergeCardListWithWorkspaceFeeds(workspaceCardFeeds ?? CONST.EMPTY_OBJECT, cardList, shouldFilterOutPersonalCards); + if (!lastResult || !deepEqual(result, lastResult)) { + lastResult = result; + } + return lastResult; + }; +}; + +/** The user's cards, personal ones included, plus every workspace feed card. */ +const personalAndWorkspaceCardListSelector = createCardListWithWorkspaceFeedsSelector(false); + /** * Returns a selector that picks a single card from the card list by card ID. */ @@ -135,4 +174,5 @@ export { companyCardCustomNamesSelector, hasIssuedExpensifyCardSelector, hasActiveExpensifyCardSelector, + personalAndWorkspaceCardListSelector, }; diff --git a/src/types/onyx/DerivedValues.ts b/src/types/onyx/DerivedValues.ts index 0d71120505c0..9d59a0a7cd31 100644 --- a/src/types/onyx/DerivedValues.ts +++ b/src/types/onyx/DerivedValues.ts @@ -209,11 +209,6 @@ type SortedReportActionsDerivedValue = { transactionThreadIDs: Record; }; -/** - * The derived value for merged personal and workspace card feeds. - */ -type PersonalAndWorkspaceCardListDerivedValue = CardList; - /** * The derived value mapping each user's login (lowercased) to their accountID. * @@ -257,7 +252,6 @@ export type { VisibleReportActionsDerivedValue, SortedReportActionsDerivedValue, NonPersonalAndWorkspaceCardListDerivedValue, - PersonalAndWorkspaceCardListDerivedValue, CardFeedErrorsDerivedValue, LoginToAccountIDMapDerivedValue, GuideAccountIDsDerivedValue, diff --git a/src/types/onyx/index.ts b/src/types/onyx/index.ts index 16d05ed89524..cae03cf31f02 100644 --- a/src/types/onyx/index.ts +++ b/src/types/onyx/index.ts @@ -69,7 +69,6 @@ import type { NonPersonalAndWorkspaceCardListDerivedValue, OneOnOneChatReportIDsDerivedValue, OutstandingReportsByPolicyIDDerivedValue, - PersonalAndWorkspaceCardListDerivedValue, ReportAttributesDerivedValue, ReportTransactionsAndViolationsDerivedValue, SortedReportActionsDerivedValue, @@ -522,7 +521,6 @@ export type { VisibleReportActionsDerivedValue, SortedReportActionsDerivedValue, NonPersonalAndWorkspaceCardListDerivedValue, - PersonalAndWorkspaceCardListDerivedValue, CardFeedErrorsDerivedValue, LoginToAccountIDMapDerivedValue, GuideAccountIDsDerivedValue, diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index 629837bb5080..e7b775bfe95f 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -1012,83 +1012,6 @@ describe('OnyxDerived', () => { }); }); - describe('personalAndWorkspaceCardList', () => { - it('merges cardList and workspaceCardFeeds when dependencies are set', async () => { - // Non-personal cards (fundID !== '0') from cardList are kept, workspace cards are always included - const nonPersonalCard1 = createRandomExpensifyCard(1, {fundID: '123'}); - const nonPersonalCard2 = createRandomExpensifyCard(2, {fundID: '456'}); - const personalCard = createRandomExpensifyCard(3, {fundID: '0'}); - const workspaceCard3 = createRandomCompanyCard(4, {bank: 'vcf'}); - - await Onyx.set(ONYXKEYS.CARD_LIST, { - '1': nonPersonalCard1, - '2': nonPersonalCard2, - '3': personalCard, - }); - await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_123`, { - '4': workspaceCard3, - }); - await waitForBatchedUpdates(); - const derivedCardList = await OnyxUtils.get(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); - - expect(derivedCardList).toMatchObject({ - '1': expect.objectContaining({cardID: 1}), - '2': expect.objectContaining({cardID: 2}), - '3': expect.objectContaining({cardID: 3}), - '4': expect.objectContaining({cardID: 4}), - }); - }); - - it('handles empty cardList when workspaceCardFeeds are set', async () => { - const workspaceCard = createRandomCompanyCard(1, {bank: 'vcf'}); - - await Onyx.set(ONYXKEYS.CARD_LIST, {}); - await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_123`, { - '1': workspaceCard, - }); - await waitForBatchedUpdates(); - const derivedCardList = await OnyxUtils.get(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); - - expect(derivedCardList).toMatchObject({ - '1': expect.objectContaining({cardID: 1}), - }); - }); - - it('handles empty workspaceCardFeeds when cardList is set', async () => { - const nonPersonalCard = createRandomExpensifyCard(1, {fundID: '123'}); - - await Onyx.set(ONYXKEYS.CARD_LIST, { - '1': nonPersonalCard, - }); - await waitForBatchedUpdates(); - const derivedCardList = await OnyxUtils.get(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); - - expect(derivedCardList).toMatchObject({ - '1': expect.objectContaining({cardID: 1}), - }); - }); - - it('includes cards from multiple workspace feeds when dependencies are set', async () => { - const card1 = createRandomCompanyCard(1, {bank: 'vcf'}); - const card2 = createRandomCompanyCard(2, {bank: 'stripe'}); - - await Onyx.set(ONYXKEYS.CARD_LIST, {}); - await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_123`, { - '1': card1, - }); - await Onyx.set(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_456`, { - '2': card2, - }); - await waitForBatchedUpdates(); - const derivedCardList = await OnyxUtils.get(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); - - expect(derivedCardList).toMatchObject({ - '1': expect.objectContaining({cardID: 1}), - '2': expect.objectContaining({cardID: 2}), - }); - }); - }); - describe('skipping the write when compute returns the current value', () => { beforeEach(async () => { jest.restoreAllMocks(); diff --git a/tests/unit/hooks/useFilterFeedDataTest.ts b/tests/unit/hooks/useFilterFeedDataTest.ts index 17a88f4dc38f..a2acd8bd8d7e 100644 --- a/tests/unit/hooks/useFilterFeedDataTest.ts +++ b/tests/unit/hooks/useFilterFeedDataTest.ts @@ -110,15 +110,19 @@ describe('useFilterFeedData', () => { }); it('should include Expensify Card feeds from allCards', async () => { - await Onyx.merge(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST, { + // Given an Expensify Card in the user's card list + await Onyx.merge(ONYXKEYS.CARD_LIST, { card1: { + cardID: 1, bank: 'Expensify Card', fundID: '999', }, }); + // When the feed options are built const {result} = renderHook(() => useFilterFeedData(undefined)); + // Then the card's feed is offered, since the merged card list is read live from the card list and workspace feeds expect(result.current.feedOptions).toHaveLength(1); expect(result.current.feedOptions.at(0)?.value).toBe('999_Expensify Card'); }); diff --git a/tests/unit/pages/Search/SearchSavePageTest.tsx b/tests/unit/pages/Search/SearchSavePageTest.tsx index ae56ee5f76a3..a105a50a2739 100644 --- a/tests/unit/pages/Search/SearchSavePageTest.tsx +++ b/tests/unit/pages/Search/SearchSavePageTest.tsx @@ -5,6 +5,7 @@ import useFilterTaxRateValue from '@components/Search/hooks/useFilterTaxRateValu import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; import * as SearchUIUtils from '@libs/SearchUIUtils'; @@ -35,6 +36,7 @@ jest.mock('@hooks/useAutoFocusInput', () => jest.fn(() => ({inputCallbackRef: je jest.mock('@hooks/useCurrencyList', () => ({useCurrencyListActions: jest.fn(() => ({convertToDisplayStringWithoutCurrency: jest.fn()}))})); jest.mock('@hooks/useLocalize'); jest.mock('@hooks/useOnyx'); +jest.mock('@hooks/usePersonalAndWorkspaceCardList'); jest.mock('@hooks/useThemeStyles', () => jest.fn(() => ({}))); const cards = createMock({}); cards[12] = createMock({cardID: 12, bank: CONST.COMPANY_CARD.FEED_BANK_NAME.UPLOAD, state: CONST.EXPENSIFY_CARD.STATE.OPEN, nameValuePairs: {cardTitle: 'Selected Alpha'}}); @@ -48,14 +50,13 @@ jest.mocked(useOnyx).mockImplementation((key) => { switch (key) { case ONYXKEYS.FORMS.SEARCH_ADVANCED_FILTERS_FORM: return [form, {status: 'loaded'}]; - case ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST: - return [cards, {status: 'loaded'}]; default: return [undefined, {status: 'loaded'}]; } }); // Without a loaded locale, translate returns key paths and translated filter values never show the user's text. beforeAll(() => IntlStore.load(CONST.LOCALES.EN)); +jest.mocked(usePersonalAndWorkspaceCardList).mockImplementation((selector) => selector(cards)); beforeEach(() => jest.clearAllMocks()); it.each([[['12']], [['12', '23']], [['123']]])('renders canonical card selection %j exactly', (cardID) => { // Given a saved search form with cards, feeds, tax rates and a Merchant filter that uses the default "contains" match diff --git a/tests/unit/selectors/CardTest.ts b/tests/unit/selectors/CardTest.ts index 836a9a0c81ce..0a1b7454fb29 100644 --- a/tests/unit/selectors/CardTest.ts +++ b/tests/unit/selectors/CardTest.ts @@ -1,9 +1,10 @@ import {isCard, isCardPendingActivate, isCardPendingIssue, isCardWithPotentialFraud, isExpensifyCard} from '@libs/CardUtils'; import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; import type {Card, CardList, WorkspaceCardsList} from '@src/types/onyx'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import type {ValueOf} from 'type-fest'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -14,6 +15,7 @@ import { filterOutPersonalCards, hasActiveExpensifyCardSelector, hasIssuedExpensifyCardSelector, + personalAndWorkspaceCardListSelector, } from '@selectors/Card'; import createRandomCard, {createRandomCompanyCard, createRandomExpensifyCard} from '../../utils/collections/card'; @@ -1000,3 +1002,90 @@ describe('hasActiveExpensifyCardSelector', () => { expect(hasActiveExpensifyCardSelector(undefined)).toBe(false); }); }); + +describe('personalAndWorkspaceCardListSelector', () => { + const workspaceFeedKey = `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_123`; + const otherWorkspaceFeedKey = `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_456`; + + it('should return an empty list when neither input is set', () => { + // Given no card list and no workspace feeds, as before they are loaded + // When the merged list is selected + // Then it is an empty list rather than undefined, so consumers can look cards up right away + expect(personalAndWorkspaceCardListSelector(undefined, undefined)).toEqual({}); + }); + + it('should merge every card of the card list, personal ones included, with the workspace feed cards', () => { + // Given two non-personal cards, a personal card (fundID '0') and a workspace feed card + const cardList: CardList = { + '1': createRandomExpensifyCard(1, {fundID: '123'}), + '2': createRandomExpensifyCard(2, {fundID: '456'}), + '3': createRandomExpensifyCard(3, {fundID: '0'}), + }; + const workspaceCardFeeds: OnyxCollection = {[workspaceFeedKey]: {'4': createRandomCompanyCard(4, {bank: 'vcf'})}}; + + // When the merged list is selected + // Then every card is in it, since Search filters on personal cards too + expect(personalAndWorkspaceCardListSelector(cardList, workspaceCardFeeds)).toMatchObject({ + '1': expect.objectContaining({cardID: 1}), + '2': expect.objectContaining({cardID: 2}), + '3': expect.objectContaining({cardID: 3}), + '4': expect.objectContaining({cardID: 4}), + }); + }); + + it('should keep the workspace feed cards when the card list is empty', () => { + // Given an empty card list and one workspace feed card + const workspaceCardFeeds: OnyxCollection = {[workspaceFeedKey]: {'1': createRandomCompanyCard(1, {bank: 'vcf'})}}; + + // When the merged list is selected + // Then the workspace card is still there, since the two sources are independent + expect(personalAndWorkspaceCardListSelector({}, workspaceCardFeeds)).toMatchObject({'1': expect.objectContaining({cardID: 1})}); + }); + + it('should keep the card list cards when there are no workspace feeds', () => { + // Given one card and no workspace feeds + const cardList: CardList = {'1': createRandomExpensifyCard(1, {fundID: '123'})}; + + // When the merged list is selected + // Then the card is there, since a missing feed collection must not hide the user's own cards + expect(personalAndWorkspaceCardListSelector(cardList, undefined)).toMatchObject({'1': expect.objectContaining({cardID: 1})}); + }); + + it('should include the cards of every workspace feed', () => { + // Given two workspace feeds with one card each + const workspaceCardFeeds: OnyxCollection = { + [workspaceFeedKey]: {'1': createRandomCompanyCard(1, {bank: 'vcf'})}, + [otherWorkspaceFeedKey]: {'2': createRandomCompanyCard(2, {bank: 'stripe'})}, + }; + + // When the merged list is selected + // Then cards from both feeds are in it + expect(personalAndWorkspaceCardListSelector({}, workspaceCardFeeds)).toMatchObject({ + '1': expect.objectContaining({cardID: 1}), + '2': expect.objectContaining({cardID: 2}), + }); + }); + + it('should return the same list while the merged content is unchanged and a new one when it changes', () => { + // Given a card list and a workspace feed collection + const cardList: CardList = {'1': createRandomExpensifyCard(1, {fundID: '0'})}; + const workspaceCard = createRandomCompanyCard(2, {bank: 'vcf'}); + const workspaceCardFeeds: OnyxCollection = {[workspaceFeedKey]: {'2': workspaceCard}}; + const first = personalAndWorkspaceCardListSelector(cardList, workspaceCardFeeds); + const feedWithCardsToAssign: WorkspaceCardsList = {'2': workspaceCard}; + Object.assign(feedWithCardsToAssign, {cardList: {'Visa 1234': 'encrypted'}}); + + // When the list is selected again from the same inputs, from new feeds that only add a card still to assign, + // and from a new card list with another card + const fromSameInputs = personalAndWorkspaceCardListSelector(cardList, workspaceCardFeeds); + const afterUnassignedCardAdded = personalAndWorkspaceCardListSelector(cardList, {[workspaceFeedKey]: feedWithCardsToAssign}); + const afterCardAdded = personalAndWorkspaceCardListSelector({...cardList, '3': createRandomExpensifyCard(3, {fundID: '0'})}, workspaceCardFeeds); + + // Then the first two reuse the same object, so consumers skip their selectors and re-renders, + // and only a real change gives a new list + expect(fromSameInputs).toBe(first); + expect(afterUnassignedCardAdded).toBe(first); + expect(afterCardAdded).not.toBe(first); + expect(afterCardAdded['3']).toMatchObject({cardID: 3}); + }); +}); diff --git a/tests/unit/usePersonalAndWorkspaceCardListTest.tsx b/tests/unit/usePersonalAndWorkspaceCardListTest.tsx new file mode 100644 index 000000000000..e489e34713ed --- /dev/null +++ b/tests/unit/usePersonalAndWorkspaceCardListTest.tsx @@ -0,0 +1,137 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; + +import ONYXKEYS from '@src/ONYXKEYS'; +import type {CardList, WorkspaceCardsList} from '@src/types/onyx'; + +/* eslint-disable @typescript-eslint/naming-convention */ +import Onyx from 'react-native-onyx'; + +import {createRandomCompanyCard, createRandomExpensifyCard} from '../utils/collections/card'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +// `cardList` clashes with the WorkspaceCardsList index signature in a typed literal, so it is attached with Object.assign +function createCardsToAssign(): WorkspaceCardsList { + const feed: WorkspaceCardsList = {}; + Object.assign(feed, {cardList: {'Visa 1234': 'encrypted'}}); + return feed; +} + +const WORKSPACE_FEED_KEY = `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}workspace_123` as const; + +describe('usePersonalAndWorkspaceCardList', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + await act(async () => { + await Onyx.clear(); + }); + }); + + it('should follow writes to the card list and the workspace feeds', async () => { + // Given a personal card in the card list + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, {'1': createRandomExpensifyCard(1, {fundID: '0'})}); + }); + const {result} = renderHook(() => usePersonalAndWorkspaceCardList()); + await waitForBatchedUpdatesWithAct(); + expect(Object.keys(result.current)).toEqual(['1']); + + // When a workspace feed with another card arrives + await act(async () => { + await Onyx.set(WORKSPACE_FEED_KEY, {'2': createRandomCompanyCard(2, {bank: 'vcf'})}); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the hook returns both cards, since it reads its inputs live instead of a stored derived value + expect(Object.keys(result.current).sort()).toEqual(['1', '2']); + }); + + it('should not re-render when a write leaves the merged list unchanged', async () => { + // Given a non-personal card and a workspace feed card + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, {'1': createRandomExpensifyCard(1, {fundID: '123'})}); + await Onyx.set(WORKSPACE_FEED_KEY, {'2': createRandomCompanyCard(2, {bank: 'vcf'})}); + }); + let renderCount = 0; + const {result} = renderHook(() => { + renderCount++; + return usePersonalAndWorkspaceCardList(); + }); + await waitForBatchedUpdatesWithAct(); + const listBeforeWrite = result.current; + const renderCountBeforeWrite = renderCount; + + // When a card still to assign is added to the workspace feed, which is not a card of the list + await act(async () => { + await Onyx.merge(WORKSPACE_FEED_KEY, createCardsToAssign()); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer keeps the same list and does not re-render + expect(result.current).toBe(listBeforeWrite); + expect(renderCount).toBe(renderCountBeforeWrite); + }); + + it('should give every mounted consumer the same list', async () => { + // Given two consumers mounted over the same inputs + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, {'1': createRandomExpensifyCard(1, {fundID: '123'})}); + }); + const {result: first} = renderHook(() => usePersonalAndWorkspaceCardList()); + const {result: second} = renderHook(() => usePersonalAndWorkspaceCardList()); + await waitForBatchedUpdatesWithAct(); + + // When a workspace feed card arrives + await act(async () => { + await Onyx.set(WORKSPACE_FEED_KEY, {'2': createRandomCompanyCard(2, {bank: 'vcf'})}); + }); + await waitForBatchedUpdatesWithAct(); + + // Then both get the same object, because the merge runs once per pair of inputs, not once per consumer + expect(second.current).toBe(first.current); + }); + + it('should apply the selector to the merged list', async () => { + // Given a non-personal card and a workspace feed card + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, {'1': createRandomExpensifyCard(1, {fundID: '123'})}); + await Onyx.set(WORKSPACE_FEED_KEY, {'2': createRandomCompanyCard(2, {bank: 'vcf'})}); + }); + + // When a consumer selects only the card count + const {result} = renderHook(() => usePersonalAndWorkspaceCardList((cardList) => Object.keys(cardList).length)); + await waitForBatchedUpdatesWithAct(); + + // Then it gets the count of the merged list, so selector consumers see the same cards as everyone else + expect(result.current).toBe(2); + }); + + it('should run each selector once per merged list, however many consumers use it', async () => { + // Given two consumers mounted with the same selector + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, {'1': createRandomExpensifyCard(1, {fundID: '123'})}); + }); + const countCards = jest.fn((cardList: CardList) => Object.keys(cardList).length); + renderHook(() => usePersonalAndWorkspaceCardList(countCards)); + renderHook(() => usePersonalAndWorkspaceCardList(countCards)); + await waitForBatchedUpdatesWithAct(); + countCards.mockClear(); + + // When a workspace feed card arrives, and then a card still to assign that leaves the merged list unchanged + await act(async () => { + await Onyx.set(WORKSPACE_FEED_KEY, {'2': createRandomCompanyCard(2, {bank: 'vcf'})}); + }); + await waitForBatchedUpdatesWithAct(); + await act(async () => { + await Onyx.merge(WORKSPACE_FEED_KEY, createCardsToAssign()); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the selector ran once for the one new merged list, since a big card list makes every run cost a full scan + expect(countCards).toHaveBeenCalledTimes(1); + }); +}); From 722cc8f2012279c16bb99bb709d63ed7aaedd7c4 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 8 Oct 2026 17:39:00 +0200 Subject: [PATCH 2/3] Add reassure test for the personal card list on card writes --- ...PersonalAndWorkspaceCardList.perf-test.tsx | 305 ++++++++++++++++++ 1 file changed, 305 insertions(+) create mode 100644 tests/perf-test/PersonalAndWorkspaceCardList.perf-test.tsx diff --git a/tests/perf-test/PersonalAndWorkspaceCardList.perf-test.tsx b/tests/perf-test/PersonalAndWorkspaceCardList.perf-test.tsx new file mode 100644 index 000000000000..47ef5f002ecd --- /dev/null +++ b/tests/perf-test/PersonalAndWorkspaceCardList.perf-test.tsx @@ -0,0 +1,305 @@ +import {render, screen} from '@testing-library/react-native'; + +import useFilterCardValue from '@components/Search/hooks/useFilterCardValue'; +import useFilterFeedData from '@components/Search/hooks/useFilterFeedData'; + +import useAdvancedSearchFilters from '@hooks/useAdvancedSearchFilters'; +import useSearchSections from '@hooks/useSearchSections'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Card, CardList, WorkspaceCardsList} from '@src/types/onyx'; + +import type * as Navigation from '@react-navigation/native'; + +import React from 'react'; +import {View} from 'react-native'; +import Onyx from 'react-native-onyx'; +import {measureAsyncFunction, measureRenders} from 'reassure'; + +import createMock from '../utils/createMock'; +import * as TestHelper from '../utils/TestHelper'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates'; + +jest.mock('@libs/Permissions'); +jest.mock('../../src/libs/Navigation/Navigation', () => ({ + navigate: jest.fn(), + isActiveRoute: jest.fn(), + getTopmostReportId: jest.fn(), + getActiveRoute: jest.fn(), + getTopmostReportActionId: jest.fn(), + isNavigationReady: jest.fn(() => Promise.resolve()), + isDisplayedInModal: jest.fn(() => false), + getActiveRouteWithoutParams: jest.fn(() => ''), +})); +jest.mock('../../src/libs/Navigation/navigationRef', () => ({ + getState: () => ({ + routes: [{name: 'Report'}], + }), + getRootState: () => ({ + routes: [], + }), + addListener: () => () => {}, + isReady: () => true, +})); +jest.mock('@react-navigation/native', () => { + const actualNav = jest.requireActual('@react-navigation/native'); + return { + ...actualNav, + useNavigationState: () => true, + useRoute: jest.fn(() => ({key: '', name: '', params: {reportID: '1'}})), + useFocusEffect: jest.fn(), + useIsFocused: () => true, + useNavigation: () => ({ + navigate: jest.fn(), + addListener: jest.fn(() => jest.fn()), + }), + createNavigationContainerRef: jest.fn(), + }; +}); +const CURRENT_USER_ACCOUNT_ID = 1; +const CURRENT_USER_EMAIL = 'email1@test.com'; +const WORKSPACE_ACCOUNT_ID = 777; +const HOOK_CONSUMER_COUNT = 3; +const SMALL_CARD_COUNT = 20; +const LARGE_CARD_COUNT = 3000; +const WRITES_PER_SCENARIO = 10; +const RENAMED_CARD_ID = 1; + +const WORKSPACE_FEED_KEYS = [ + `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${WORKSPACE_ACCOUNT_ID}_${CONST.EXPENSIFY_CARD.BANK}`, + `${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${WORKSPACE_ACCOUNT_ID}_${CONST.COMPANY_CARD.FEED_BANK_NAME.VISA}`, +] as const; + +function createCard(cardID: number, bank: Card['bank'], fundID: string | undefined): Card { + return createMock({ + cardID, + bank, + fundID, + state: CONST.EXPENSIFY_CARD.STATE.OPEN, + lastFourPAN: String(1000 + (cardID % 9000)), + cardName: `Card ${cardID}`, + accountID: CURRENT_USER_ACCOUNT_ID, + domainName: 'expensify-policy.exfy', + }); +} + +function buildCards(count: number) { + const cardList: CardList = {}; + const expensifyFeed: WorkspaceCardsList = {}; + const companyFeed: WorkspaceCardsList = {}; + for (let index = 0; index < count; index++) { + const userCardID = index + 1; + cardList[userCardID] = index % 2 === 0 ? createCard(userCardID, CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, '0') : createCard(userCardID, CONST.EXPENSIFY_CARD.BANK, '5000'); + const expensifyCardID = 100000 + index; + expensifyFeed[expensifyCardID] = createCard(expensifyCardID, CONST.EXPENSIFY_CARD.BANK, '5000'); + const companyCardID = 200000 + index; + companyFeed[companyCardID] = createCard(companyCardID, CONST.COMPANY_CARD.FEED_BANK_NAME.VISA, undefined); + } + return {cardList, workspaceFeeds: [expensifyFeed, companyFeed]}; +} + +const smallCards = buildCards(SMALL_CARD_COUNT); +const largeCards = buildCards(LARGE_CARD_COUNT); + +const SELECTED_CARD_IDS = ['1', '2']; + +function SearchSectionsConsumer() { + useSearchSections(); + return ; +} + +function FeedDataConsumer() { + useFilterFeedData(undefined); + return ; +} + +function CardValueConsumer() { + useFilterCardValue(SELECTED_CARD_IDS); + return ; +} + +function AdvancedFiltersConsumer() { + useAdvancedSearchFilters(CONST.SEARCH.DATA_TYPES.EXPENSE); + return ; +} + +/** Search filter surfaces that read the card list through selectors and nothing else of the cards, so their re-renders isolate card list churn */ +function CardFilterConsumers() { + return ( + <> + {Array.from({length: HOOK_CONSUMER_COUNT}, (value, index) => ( + + + + + ))} + + ); +} + +/** Every Search surface that reads the merged card list, mounted together as on the Search page with its filters open */ +function AllCardConsumers() { + return ( + <> + {Array.from({length: HOOK_CONSUMER_COUNT}, (value, index) => ( + + + + + ))} + + + ); +} + +async function seed(cards: ReturnType) { + await Onyx.multiSet({ + [ONYXKEYS.CARD_LIST]: cards.cardList, + [WORKSPACE_FEED_KEYS[0]]: cards.workspaceFeeds.at(0), + [WORKSPACE_FEED_KEYS[1]]: cards.workspaceFeeds.at(1), + }); + await waitForBatchedUpdates(); +} + +/** Writes to a workspace feed's list of cards still to assign, which are not cards, so the merged list keeps its content */ +async function writeUnassignedCards(label: string) { + for (let index = 0; index < WRITES_PER_SCENARIO; index++) { + await Onyx.merge(WORKSPACE_FEED_KEYS[1], {cardList: {[`account ${label}-${index}`]: 'encrypted'}}); + await waitForBatchedUpdates(); + } +} + +describe('Personal and workspace card list consumers on card writes', () => { + beforeAll(() => { + Onyx.init({ + keys: ONYXKEYS, + evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS], + }); + // Required lazily so the navigation mocks above are registered before the engine's imports load it + // eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access + require('@userActions/OnyxDerived').default(); + }); + + beforeEach(async () => { + global.fetch = TestHelper.getGlobalFetchMock(); + wrapOnyxWithWaitForBatchedUpdates(Onyx); + await TestHelper.signInWithTestUser(CURRENT_USER_ACCOUNT_ID, CURRENT_USER_EMAIL, undefined, undefined, 'One'); + await waitForBatchedUpdates(); + }); + + afterEach(async () => { + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + test('[PersonalCardList] should re-render Search surfaces when unassigned workspace cards change', async () => { + // Given every Search surface that reads the card list, mounted over a small card list + await seed(smallCards); + + // Reassure runs the scenario several times against one seeded Onyx, so every run writes keys that don't exist yet + let run = 0; + const scenario = async () => { + await screen.findAllByTestId('feed-data-consumer'); + + // When the unassigned cards of a workspace feed change repeatedly, which leaves the merged list unchanged + await writeUnassignedCards(`small ${run}`); + run++; + }; + + // Then reassure measures what the unchanged merges cost the consumers + await measureRenders(, {scenario}); + }); + + test('[PersonalCardList] should not re-render card filters when unassigned workspace cards change', async () => { + // Given only the card filters, which read the merged list through selectors and nothing else of the cards + await seed(smallCards); + + let run = 0; + const scenario = async () => { + await screen.findAllByTestId('card-value-consumer'); + + // When the unassigned cards of a workspace feed change repeatedly, which leaves the merged list unchanged + await writeUnassignedCards(`isolated ${run}`); + run++; + }; + + // Then every render here comes from the merged list or a selected value changing reference without changing content + await measureRenders(, {scenario}); + }); + + test('[PersonalCardList] should re-render Search surfaces when a personal card is renamed', async () => { + // Given every Search surface that reads the card list, mounted over a small card list + await seed(smallCards); + + let run = 0; + const scenario = async () => { + await screen.findAllByTestId('feed-data-consumer'); + + // When a personal card is renamed on every write, so the merged list really changes + for (let index = 0; index < WRITES_PER_SCENARIO; index++) { + await Onyx.merge(ONYXKEYS.CARD_LIST, {[RENAMED_CARD_ID]: {cardName: `Renamed ${run}-${index}`}}); + await waitForBatchedUpdates(); + } + run++; + }; + + // Then reassure measures the cost of a real change, which both versions have to pay + await measureRenders(, {scenario}); + }); + + test('[PersonalCardList] should re-render Search surfaces when unassigned workspace cards change in a large card list', async () => { + // Given the same consumers over 9000 cards + await seed(largeCards); + + let run = 0; + const scenario = async () => { + await screen.findAllByTestId('feed-data-consumer'); + + // When the unassigned cards of a workspace feed change repeatedly, so every write merges the whole list again + await writeUnassignedCards(`large ${run}`); + run++; + }; + + // Then reassure measures the render cost, which grows if each consumer scans the list during render + await measureRenders(, {scenario}); + }); + + test('[PersonalCardList] should process unassigned workspace card writes in a large card list', async () => { + // Given the consumers mounted over 9000 cards + await seed(largeCards); + render(); + await screen.findAllByTestId('feed-data-consumer'); + + // When the unassigned cards of a workspace feed change repeatedly + let run = 0; + const writeLoop = async () => { + await writeUnassignedCards(`timed ${run}`); + run++; + }; + + // Then reassure times the whole write loop, including work done outside React renders such as derived computes and their writes + await measureAsyncFunction(writeLoop); + }); + + test('[PersonalCardList] should process personal card renames in a large card list', async () => { + // Given the consumers mounted over 9000 cards + await seed(largeCards); + render(); + await screen.findAllByTestId('feed-data-consumer'); + + // When a personal card is renamed on every write + let run = 0; + const writeLoop = async () => { + for (let index = 0; index < WRITES_PER_SCENARIO; index++) { + await Onyx.merge(ONYXKEYS.CARD_LIST, {[RENAMED_CARD_ID]: {cardName: `Timed rename ${run}-${index}`}}); + await waitForBatchedUpdates(); + } + run++; + }; + + // Then reassure times the loop when both versions must merge and deliver a new list + await measureAsyncFunction(writeLoop); + }); +}); From 2e77a07d4f098404dec93b46842ab6c2928ecd0a Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Fri, 6 Nov 2026 11:43:47 +0100 Subject: [PATCH 3/3] Compare card list outputs by identity in usePersonalAndWorkspaceCardList useOnyxState's default selectorEquality deep-compares the output for every mounted consumer, so a real card change cost one full compare of the merged list per consumer. select() now compares each selector's output once and keeps the previous object when equal, and the hook compares by identity. Also drops the collection-type cast that Onyx no longer needs. --- src/hooks/usePersonalAndWorkspaceCardList.ts | 25 ++++++++-------- .../usePersonalAndWorkspaceCardListTest.tsx | 30 +++++++++++++++++++ 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/hooks/usePersonalAndWorkspaceCardList.ts b/src/hooks/usePersonalAndWorkspaceCardList.ts index 086aaeb279a7..bcceebc99a21 100644 --- a/src/hooks/usePersonalAndWorkspaceCardList.ts +++ b/src/hooks/usePersonalAndWorkspaceCardList.ts @@ -1,14 +1,13 @@ import ONYXKEYS from '@src/ONYXKEYS'; -import type {CardList, WorkspaceCardsList} from '@src/types/onyx'; - -import type {OnyxCollection} from 'react-native-onyx'; +import type {CardList} from '@src/types/onyx'; import {personalAndWorkspaceCardListSelector} from '@selectors/Card'; +import {deepEqual} from 'fast-equals'; import {useOnyxState} from 'react-native-onyx'; type CardListSelector = (cardList: CardList) => TReturn; -const dependencies = [ONYXKEYS.CARD_LIST, ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST]; +const dependencies = [ONYXKEYS.CARD_LIST, ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST] as const; // useOnyxState runs the selector on every write to either key, so each selector's output is kept per merged list const selectedValues = new WeakMap, {cardList: CardList; value: unknown}>(); @@ -16,26 +15,28 @@ const selectedValues = new WeakMap, {cardList: CardLis function select(selector: CardListSelector, cardList: CardList): TReturn { const cached = selectedValues.get(selector); if (cached?.cardList === cardList) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- each entry only ever holds its own selector's output return cached.value as TReturn; } - const value = selector(cardList); + // One compare per selector shared by all consumers, so useOnyxState can compare outputs by identity + const fresh = selector(cardList); + const value = cached && deepEqual(cached.value, fresh) ? cached.value : fresh; selectedValues.set(selector, {cardList, value}); - return value; + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion -- each entry only ever holds its own selector's output + return value as TReturn; } +// Pass a module-level selector: outputs are compared by identity, so a new selector each render returns a new object each render function usePersonalAndWorkspaceCardList(): CardList; function usePersonalAndWorkspaceCardList(selector: CardListSelector): TReturn; function usePersonalAndWorkspaceCardList(selector?: CardListSelector) { return useOnyxState( (state) => { - // useOnyxState's state view types a collection key as one member, but at runtime it returns the whole collection - // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion - const workspaceCardFeeds = state[ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST] as OnyxCollection; - const cardList = personalAndWorkspaceCardListSelector(state[ONYXKEYS.CARD_LIST], workspaceCardFeeds); + const cardList = personalAndWorkspaceCardListSelector(state[ONYXKEYS.CARD_LIST], state[ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST]); return selector ? select(selector, cardList) : cardList; }, - {dependencies}, + // Both the merge and select() return the previous object for equal content, so identity is enough + {dependencies, selectorEquality: (a, b) => a === b}, ); } diff --git a/tests/unit/usePersonalAndWorkspaceCardListTest.tsx b/tests/unit/usePersonalAndWorkspaceCardListTest.tsx index e489e34713ed..8721e86860b8 100644 --- a/tests/unit/usePersonalAndWorkspaceCardListTest.tsx +++ b/tests/unit/usePersonalAndWorkspaceCardListTest.tsx @@ -2,6 +2,7 @@ import {act, renderHook} from '@testing-library/react-native'; import usePersonalAndWorkspaceCardList from '@hooks/usePersonalAndWorkspaceCardList'; +import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {CardList, WorkspaceCardsList} from '@src/types/onyx'; @@ -134,4 +135,33 @@ describe('usePersonalAndWorkspaceCardList', () => { // Then the selector ran once for the one new merged list, since a big card list makes every run cost a full scan expect(countCards).toHaveBeenCalledTimes(1); }); + + it('should not re-render a selector consumer when a change leaves its output equal', async () => { + // Given a visible card and a card hidden from Search, read through a selector that drops hidden cards + await act(async () => { + await Onyx.set(ONYXKEYS.CARD_LIST, { + '1': createRandomExpensifyCard(1, {fundID: '123', state: CONST.EXPENSIFY_CARD.STATE.OPEN}), + '2': createRandomExpensifyCard(2, {fundID: '123', state: CONST.EXPENSIFY_CARD.STATE.CLOSED}), + }); + }); + const visibleCards = (cardList: CardList) => Object.fromEntries(Object.entries(cardList).filter(([, card]) => card.state !== CONST.EXPENSIFY_CARD.STATE.CLOSED)); + let renderCount = 0; + const {result} = renderHook(() => { + renderCount++; + return usePersonalAndWorkspaceCardList(visibleCards); + }); + await waitForBatchedUpdatesWithAct(); + const outputBeforeWrite = result.current; + const renderCountBeforeWrite = renderCount; + + // When the hidden card is renamed, which changes the merged list but not what the selector returns + await act(async () => { + await Onyx.merge(ONYXKEYS.CARD_LIST, {'2': {nameValuePairs: {cardTitle: 'Renamed'}}}); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer keeps the same output and does not re-render, though the hook compares outputs by identity + expect(result.current).toBe(outputBeforeWrite); + expect(renderCount).toBe(renderCountBeforeWrite); + }); });