From 342e09798676590be5d1e9b8f01d3e3f576f8480 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Tue, 6 Oct 2026 16:32:41 +0200 Subject: [PATCH 1/3] Reuse unchanged derived values and skip no-op derived writes --- .../OnyxDerived/configs/guideAccountIDs.ts | 6 +- .../configs/loginToAccountIDMap.ts | 6 +- src/libs/actions/OnyxDerived/index.ts | 3 + ...eesCellPersonalDetailsWrites.perf-test.tsx | 93 ++++++++++++++ ...arLinksPersonalDetailsWrites.perf-test.tsx | 110 ++++++++++++++++ tests/unit/OnyxDerived/guideAccountIDsTest.ts | 32 +++++ .../OnyxDerived/loginToAccountIDMapTest.ts | 26 ++++ .../OnyxDerivedReferenceStabilityTest.tsx | 117 ++++++++++++++++++ tests/unit/OnyxDerivedRestoredValueTest.ts | 34 +++++ .../unit/OnyxDerivedSkipUnchangedWriteTest.ts | 80 ++++++++++++ 10 files changed, 503 insertions(+), 4 deletions(-) create mode 100644 tests/perf-test/AttendeesCellPersonalDetailsWrites.perf-test.tsx create mode 100644 tests/perf-test/SidebarLinksPersonalDetailsWrites.perf-test.tsx create mode 100644 tests/unit/OnyxDerivedReferenceStabilityTest.tsx create mode 100644 tests/unit/OnyxDerivedRestoredValueTest.ts create mode 100644 tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts diff --git a/src/libs/actions/OnyxDerived/configs/guideAccountIDs.ts b/src/libs/actions/OnyxDerived/configs/guideAccountIDs.ts index 45b760a68be4..76805965dc36 100644 --- a/src/libs/actions/OnyxDerived/configs/guideAccountIDs.ts +++ b/src/libs/actions/OnyxDerived/configs/guideAccountIDs.ts @@ -5,6 +5,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {GuideAccountIDsDerivedValue} from '@src/types/onyx'; import {Str} from 'expensify-common'; +import {shallowEqual} from 'fast-equals'; /** * Derives the accountIDs of every Expensify Guide (identified by login domain) in the personal details list. @@ -13,7 +14,7 @@ import {Str} from 'expensify-common'; export default createOnyxDerivedValueConfig({ key: ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS, dependencies: [ONYXKEYS.PERSONAL_DETAILS_LIST], - compute: ([personalDetailsList]) => { + compute: ([personalDetailsList], {currentValue}) => { if (!personalDetailsList) { return []; } @@ -25,6 +26,7 @@ export default createOnyxDerivedValueConfig({ } guideAccountIDs.push(personalDetails.accountID); } - return guideAccountIDs.sort((a, b) => a - b); + guideAccountIDs.sort((a, b) => a - b); + return currentValue && shallowEqual(currentValue, guideAccountIDs) ? currentValue : guideAccountIDs; }, }); diff --git a/src/libs/actions/OnyxDerived/configs/loginToAccountIDMap.ts b/src/libs/actions/OnyxDerived/configs/loginToAccountIDMap.ts index 5c0fc83521d5..5e8b422ae2b5 100644 --- a/src/libs/actions/OnyxDerived/configs/loginToAccountIDMap.ts +++ b/src/libs/actions/OnyxDerived/configs/loginToAccountIDMap.ts @@ -3,6 +3,8 @@ import createOnyxDerivedValueConfig from '@userActions/OnyxDerived/createOnyxDer import ONYXKEYS from '@src/ONYXKEYS'; import type {LoginToAccountIDMapDerivedValue} from '@src/types/onyx'; +import {shallowEqual} from 'fast-equals'; + /** * Derives a map of login (lowercased) -> accountID from the personal details list. * @@ -13,7 +15,7 @@ import type {LoginToAccountIDMapDerivedValue} from '@src/types/onyx'; export default createOnyxDerivedValueConfig({ key: ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP, dependencies: [ONYXKEYS.PERSONAL_DETAILS_LIST], - compute: ([personalDetailsList]) => { + compute: ([personalDetailsList], {currentValue}) => { if (!personalDetailsList) { return {}; } @@ -30,6 +32,6 @@ export default createOnyxDerivedValueConfig({ loginToAccountIDMap[login] = personalDetails.accountID; } } - return loginToAccountIDMap; + return currentValue && shallowEqual(currentValue, loginToAccountIDMap) ? currentValue : loginToAccountIDMap; }, }); diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index cfe1ec657075..8c12496c2730 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -146,6 +146,9 @@ function init() { try { // @ts-expect-error TypeScript can't confirm the shape of dependencyValues matches the compute function's parameters const newDerivedValue = compute(dependencyValues, context); + if (newDerivedValue === derivedValue) { + return; + } Log.info(`[OnyxDerived] updating value for ${key} in Onyx`); derivedValue = newDerivedValue; setDerivedValue(key, derivedValue); diff --git a/tests/perf-test/AttendeesCellPersonalDetailsWrites.perf-test.tsx b/tests/perf-test/AttendeesCellPersonalDetailsWrites.perf-test.tsx new file mode 100644 index 000000000000..e1e99f8c7e6c --- /dev/null +++ b/tests/perf-test/AttendeesCellPersonalDetailsWrites.perf-test.tsx @@ -0,0 +1,93 @@ +import {screen} from '@testing-library/react-native'; + +import ComposeProviders from '@components/ComposeProviders'; +import {LocaleContextProvider} from '@components/LocaleContextProvider'; +import OnyxListItemProvider from '@components/OnyxListItemProvider'; +import AttendeesCell from '@components/Search/SearchList/ListItem/AttendeesCell'; + +import initOnyxDerivedValues from '@userActions/OnyxDerived'; + +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetailsList} from '@src/types/onyx'; +import type {Attendee} from '@src/types/onyx/IOU'; + +import React from 'react'; +import {View} from 'react-native'; +import Onyx from 'react-native-onyx'; +import {measureRenders} from 'reassure'; + +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +const ROW_COUNT = 50; +const ATTENDEES_PER_ROW = 3; +const WRITES_PER_SCENARIO = 10; +const UNRELATED_ACCOUNT_ID = 1000; + +const personalDetails: PersonalDetailsList = { + [UNRELATED_ACCOUNT_ID]: {accountID: UNRELATED_ACCOUNT_ID, login: 'unrelated@test.com', displayName: 'Unrelated'}, +}; +const rows: Attendee[][] = []; +for (let row = 0; row < ROW_COUNT; row++) { + const attendees: Attendee[] = []; + for (let index = 0; index < ATTENDEES_PER_ROW; index++) { + const accountID = row * ATTENDEES_PER_ROW + index + 1; + const email = `attendee${accountID}@test.com`; + personalDetails[accountID] = {accountID, login: email, displayName: `Attendee ${accountID}`}; + attendees.push({email, displayName: `Attendee ${accountID}`, avatarUrl: ''}); + } + rows.push(attendees); +} + +function SearchRowsWithAttendees() { + return ( + + + {rows.map((attendees, index) => ( + + ))} + + + ); +} + +describe('AttendeesCell on personal details writes', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + initOnyxDerivedValues(); + }); + + beforeEach(async () => { + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, personalDetails); + await waitForBatchedUpdates(); + }); + + afterEach(async () => { + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + test('[AttendeesCell] should re-render 50 Search rows when a user who attends nothing is renamed', async () => { + // Reassure runs the scenario several times against one seeded Onyx, so every run writes a name that doesn't exist yet + let run = 0; + const scenario = async () => { + // Given 50 Search rows with attendees have rendered + await screen.findAllByTestId('AttendeesCell-Row'); + + // When a user who attends none of them is renamed, which makes the engine recompute the login map to the same content + for (let index = 0; index < WRITES_PER_SCENARIO; index++) { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[UNRELATED_ACCOUNT_ID]: {displayName: `Unrelated ${run}-${index}`}}); + await waitForBatchedUpdates(); + } + run++; + }; + + // Then reassure measures how many row re-renders the unrelated writes cause + await measureRenders(, {scenario}); + }); +}); diff --git a/tests/perf-test/SidebarLinksPersonalDetailsWrites.perf-test.tsx b/tests/perf-test/SidebarLinksPersonalDetailsWrites.perf-test.tsx new file mode 100644 index 000000000000..032d3602aef0 --- /dev/null +++ b/tests/perf-test/SidebarLinksPersonalDetailsWrites.perf-test.tsx @@ -0,0 +1,110 @@ +import {screen} from '@testing-library/react-native'; + +import {setHasRadio} from '@libs/NetworkState'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetailsList} from '@src/types/onyx'; + +import Onyx from 'react-native-onyx'; +import {measureRenders} from 'reassure'; + +import * as LHNTestUtils from '../utils/LHNTestUtils'; +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 REPORT_COUNT = 500; +const WRITES_PER_SCENARIO = 10; +const GUIDE_ACCOUNT_ID = 50; +const UNRELATED_ACCOUNT_ID = 60; + +const reports = Object.fromEntries( + Array.from({length: REPORT_COUNT}, (value, index) => { + const report = {...LHNTestUtils.getFakeReport([1, 2], 1, true), lastMessageText: 'hey'}; + return [`${ONYXKEYS.COLLECTION.REPORT}${index + 1}`, report]; + }), +); + +const personalDetails: PersonalDetailsList = { + ...LHNTestUtils.fakePersonalDetails, + [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`, displayName: 'Guide'}, + [UNRELATED_ACCOUNT_ID]: {accountID: UNRELATED_ACCOUNT_ID, login: 'unrelated@test.com', displayName: 'Unrelated'}, +}; + +describe('SidebarLinks on personal details writes', () => { + beforeAll(() => { + Onyx.init({ + keys: ONYXKEYS, + evictableKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS], + }); + // Required lazily so LHNTestUtils registers its @react-navigation/native mock 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); + setHasRadio(true); + await TestHelper.signInWithTestUser(1, 'email1@test.com', undefined, undefined, 'One'); + await waitForBatchedUpdates(); + }); + + afterEach(async () => { + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + test('[SidebarLinks] should re-render when a user outside the LHN is renamed', async () => { + // Given 500 reports in the LHN, a guide, and a user who is in none of the reports + await Onyx.multiSet({ + [ONYXKEYS.PERSONAL_DETAILS_LIST]: personalDetails, + [ONYXKEYS.BETAS]: [CONST.BETAS.DEFAULT_ROOMS], + [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD, + [ONYXKEYS.IS_LOADING_REPORT_DATA]: false, + ...reports, + }); + await waitForBatchedUpdates(); + + // Reassure runs the scenario several times against one seeded Onyx, so every run writes a name that doesn't exist yet + let run = 0; + const scenario = async () => { + await screen.findByTestId('lhn-options-list'); + + // When that user is renamed repeatedly, which makes the derived engine recompute the guide accountIDs to the same content + for (let index = 0; index < WRITES_PER_SCENARIO; index++) { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[UNRELATED_ACCOUNT_ID]: {displayName: `Unrelated ${run}-${index}`}}); + await waitForBatchedUpdates(); + } + run++; + }; + + // Then reassure measures what the unrelated writes cost the LHN + await measureRenders(, {scenario}); + }); +}); diff --git a/tests/unit/OnyxDerived/guideAccountIDsTest.ts b/tests/unit/OnyxDerived/guideAccountIDsTest.ts index fd8e7d6ff91f..01ad4c581d00 100644 --- a/tests/unit/OnyxDerived/guideAccountIDsTest.ts +++ b/tests/unit/OnyxDerived/guideAccountIDsTest.ts @@ -47,4 +47,36 @@ describe('guideAccountIDs', () => { expect(guideAccountIDsConfig.compute([guideListedLast], {})).toEqual([otherGuideAccountID, guideAccountID]); expect(guideAccountIDsConfig.compute([sameGuidesPlusAnAvatarChange], {})).toEqual([otherGuideAccountID, guideAccountID]); }); + + it('should return the current value when the guides are unchanged', () => { + // Given the guide accountIDs computed from the current personal details + const personalDetailsList: PersonalDetailsList = { + [guideAccountID]: {accountID: guideAccountID, login: guideLogin}, + [memberAccountID]: {accountID: memberAccountID, login: 'member@example.com'}, + }; + const currentValue = guideAccountIDsConfig.compute([personalDetailsList], {}); + + // When a member changes their name, which leaves the set of guides as it was + const renamedMember: PersonalDetailsList = {...personalDetailsList, [memberAccountID]: {accountID: memberAccountID, login: 'member@example.com', displayName: 'Renamed'}}; + + // Then the same array comes back, so consumers comparing by reference don't re-render + expect(guideAccountIDsConfig.compute([renamedMember], {currentValue})).toBe(currentValue); + }); + + it('should return a new array when a guide is removed', () => { + // Given two guides + const twoGuides: PersonalDetailsList = { + [guideAccountID]: {accountID: guideAccountID, login: guideLogin}, + [otherGuideAccountID]: {accountID: otherGuideAccountID, login: otherGuideLogin}, + }; + const currentValue = guideAccountIDsConfig.compute([twoGuides], {}); + + // When one of them leaves the personal details + const oneGuide: PersonalDetailsList = {[guideAccountID]: {accountID: guideAccountID, login: guideLogin}}; + const result = guideAccountIDsConfig.compute([oneGuide], {currentValue}); + + // Then a shorter list must not be mistaken for the current one + expect(result).not.toBe(currentValue); + expect(result).toEqual([guideAccountID]); + }); }); diff --git a/tests/unit/OnyxDerived/loginToAccountIDMapTest.ts b/tests/unit/OnyxDerived/loginToAccountIDMapTest.ts index c781a6644a9d..2da16bdd6f6e 100644 --- a/tests/unit/OnyxDerived/loginToAccountIDMapTest.ts +++ b/tests/unit/OnyxDerived/loginToAccountIDMapTest.ts @@ -38,4 +38,30 @@ describe('loginToAccountIDMap', () => { expect(loginToAccountIDMapConfig.compute([optimisticHasLowerAccountID], {})).toEqual({[login]: accountID2}); }); + + it('should return the current value when the logins are unchanged', () => { + // Given the map computed from the current personal details + const personalDetailsList: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login}}; + const currentValue = loginToAccountIDMapConfig.compute([personalDetailsList], {}); + + // When the user changes their name, which keeps every login on the same accountID + const renamed: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login, displayName: 'Renamed'}}; + + // Then the same map comes back, so consumers comparing by reference don't re-render + expect(loginToAccountIDMapConfig.compute([renamed], {currentValue})).toBe(currentValue); + }); + + it('should return a new map when a login moves to another accountID', () => { + // Given a login that only has an optimistic personal detail so far + const optimistic: PersonalDetailsList = {[accountID1]: {accountID: accountID1, login, isOptimisticPersonalDetail: true}}; + const currentValue = loginToAccountIDMapConfig.compute([optimistic], {}); + + // When the server replaces it with the real account, so the key stays and only its value changes + const real: PersonalDetailsList = {[accountID2]: {accountID: accountID2, login}}; + const result = loginToAccountIDMapConfig.compute([real], {currentValue}); + + // Then the change must be detected, or attendees would keep pointing at the optimistic account + expect(result).not.toBe(currentValue); + expect(result).toEqual({[login]: accountID2}); + }); }); diff --git a/tests/unit/OnyxDerivedReferenceStabilityTest.tsx b/tests/unit/OnyxDerivedReferenceStabilityTest.tsx new file mode 100644 index 000000000000..a1f35eb6766c --- /dev/null +++ b/tests/unit/OnyxDerivedReferenceStabilityTest.tsx @@ -0,0 +1,117 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import useOnyx from '@hooks/useOnyx'; + +import initOnyxDerivedValues from '@userActions/OnyxDerived'; + +import CONST from '@src/CONST'; +import IntlStore from '@src/languages/IntlStore'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetailsList} from '@src/types/onyx'; + +import type {TupleToUnion} from 'type-fest'; + +import Onyx from 'react-native-onyx'; + +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +const GUIDE_ACCOUNT_ID = 8; +const MEMBER_ACCOUNT_ID = 1; +const NEW_GUIDE_ACCOUNT_ID = 9; +const NEW_MEMBER_ACCOUNT_ID = 2; + +const PERSONAL_DETAILS: PersonalDetailsList = { + [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`, displayName: 'Guide'}, + [MEMBER_ACCOUNT_ID]: {accountID: MEMBER_ACCOUNT_ID, login: 'member@example.com', displayName: 'Member'}, +}; + +const KEYS_READ_WITHOUT_SELECTOR = [ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS, ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP] as const; + +function renderHookWithoutSelector>(key: TKey) { + let renderCount = 0; + const hook = renderHook(() => { + renderCount++; + return useOnyx(key); + }); + return {hook, getRenderCount: () => renderCount}; +} + +describe('OnyxDerived consumers without a selector on PERSONAL_DETAILS_LIST writes', () => { + beforeAll(async () => { + Onyx.init({keys: ONYXKEYS}); + initOnyxDerivedValues(); + await IntlStore.load(CONST.LOCALES.EN); + await Onyx.set(ONYXKEYS.RAM_ONLY_ARE_TRANSLATIONS_LOADING, false); + await waitForBatchedUpdatesWithAct(); + }); + + beforeEach(async () => { + await act(async () => { + await Onyx.clear(); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + }); + await waitForBatchedUpdatesWithAct(); + }); + + afterAll(async () => { + await act(async () => { + await Onyx.clear(); + }); + }); + + it.each(KEYS_READ_WITHOUT_SELECTOR)('should keep the %s reference when an unrelated personal detail changes', async (key) => { + // Given a component that reads the derived value without a selector, the way 10 App sites do + const {hook, getRenderCount} = renderHookWithoutSelector(key); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + const rendersBefore = getRenderCount(); + + // When a member renames themselves, which leaves both derived values with the same content + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the engine's fresh but equal result must not reach the consumer as a new reference or a re-render + expect(hook.result.current[0]).toBe(valueBefore); + expect(getRenderCount()).toBe(rendersBefore); + }); + + it('should return new guide accountIDs when a guide is added', async () => { + // Given a consumer of the guide accountIDs + const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + + // When a second guide's personal details arrive + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [NEW_GUIDE_ACCOUNT_ID]: {accountID: NEW_GUIDE_ACCOUNT_ID, login: `new.guide@${CONST.EMAIL.GUIDES_DOMAIN}`}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer sees the change, so the stability check above is not passing on a frozen value + expect(hook.result.current[0]).not.toBe(valueBefore); + expect(hook.result.current[0]).toEqual([GUIDE_ACCOUNT_ID, NEW_GUIDE_ACCOUNT_ID]); + }); + + it('should return a new login map when a login is added', async () => { + // Given a consumer of the login to accountID map + const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + + // When a new member's personal details arrive + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [NEW_MEMBER_ACCOUNT_ID]: {accountID: NEW_MEMBER_ACCOUNT_ID, login: 'new.member@example.com'}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer sees the new login + expect(hook.result.current[0]).not.toBe(valueBefore); + expect(hook.result.current[0]?.['new.member@example.com']).toBe(NEW_MEMBER_ACCOUNT_ID); + }); +}); diff --git a/tests/unit/OnyxDerivedRestoredValueTest.ts b/tests/unit/OnyxDerivedRestoredValueTest.ts new file mode 100644 index 000000000000..a7c485350817 --- /dev/null +++ b/tests/unit/OnyxDerivedRestoredValueTest.ts @@ -0,0 +1,34 @@ +import initOnyxDerivedValues from '@userActions/OnyxDerived'; +import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; + +import Onyx from 'react-native-onyx'; + +import getOnyxValue from '../utils/getOnyxValue'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +const GUIDE_ACCOUNT_ID = 8; + +describe('OnyxDerived with a value restored from disk', () => { + it('should not rewrite a restored value when compute returns it unchanged', async () => { + // Given the guide accountIDs and their personal details are already stored, as after an app restart + Onyx.init({keys: ONYXKEYS}); + await Onyx.multiSet({ + [ONYXKEYS.PERSONAL_DETAILS_LIST]: {[GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`}}, + [ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS]: [GUIDE_ACCOUNT_ID], + }); + await waitForBatchedUpdates(); + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // When the engine starts and runs its first compute from scratch + initOnyxDerivedValues(); + await waitForBatchedUpdates(); + + // Then the restored value is kept as it is, without another write + const guideWrites = setDerivedValueSpy.mock.calls.filter(([key]) => key === ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); + expect(guideWrites).toHaveLength(0); + expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); + }); +}); diff --git a/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts b/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts new file mode 100644 index 000000000000..d450258f2bed --- /dev/null +++ b/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts @@ -0,0 +1,80 @@ +import * as ActiveSpans from '@libs/telemetry/activeSpans'; + +import initOnyxDerivedValues from '@userActions/OnyxDerived'; +import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {PersonalDetailsList} from '@src/types/onyx'; + +import Onyx from 'react-native-onyx'; + +import getOnyxValue from '../utils/getOnyxValue'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +const GUIDE_ACCOUNT_ID = 8; +const MEMBER_ACCOUNT_ID = 1; + +const PERSONAL_DETAILS: PersonalDetailsList = { + [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`}, + [MEMBER_ACCOUNT_ID]: {accountID: MEMBER_ACCOUNT_ID, login: 'member@example.com', displayName: 'Member'}, +}; + +function getGuideWrites(setDerivedValueSpy: jest.Spied) { + return setDerivedValueSpy.mock.calls.filter(([key]) => key === ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); +} + +describe('OnyxDerived skips the write when compute returns the current value', () => { + beforeAll(async () => { + Onyx.init({keys: ONYXKEYS}); + initOnyxDerivedValues(); + await waitForBatchedUpdates(); + }); + + beforeEach(async () => { + jest.restoreAllMocks(); + await Onyx.clear(); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + await waitForBatchedUpdates(); + }); + + it('should not write to Onyx when compute returns the current value', async () => { + // Given the guide accountIDs have been computed and stored + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // When a member is renamed, which recomputes the guides to the value already held + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + await waitForBatchedUpdates(); + + // Then nothing is written, so subscribers aren't notified and the value isn't rewritten to disk + expect(getGuideWrites(setDerivedValueSpy)).toHaveLength(0); + expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); + }); + + it('should still end the compute span when the write is skipped', async () => { + // Given the compute span is observed + const endSpanSpy = jest.spyOn(ActiveSpans, 'endSpan'); + + // When a recompute returns the current value and skips the write + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + await waitForBatchedUpdates(); + + // Then the early return still goes through finally, so no compute span is left open + expect(endSpanSpy).toHaveBeenCalledWith(`${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS}`); + }); + + it('should rewrite the value after Onyx.clear', async () => { + // Given the guide accountIDs are stored and Onyx is then cleared, as on sign-out + await Onyx.clear(); + await waitForBatchedUpdates(); + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // When the same personal details come back, as on signing in again + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + await waitForBatchedUpdates(); + + // Then the engine writes the value again instead of treating the pre-clear value as current + expect(getGuideWrites(setDerivedValueSpy).length).toBeGreaterThan(0); + expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); + }); +}); From bb154de3a3daae5733fb1c99bd79fd82bf6c8fb6 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 8 Oct 2026 12:48:44 +0200 Subject: [PATCH 2/3] move tests --- .../OnyxDerivedReferenceStabilityTest.tsx | 117 -------------- .../unit/OnyxDerivedSkipUnchangedWriteTest.ts | 80 --------- tests/unit/OnyxDerivedTest.tsx | 153 +++++++++++++++++- 3 files changed, 152 insertions(+), 198 deletions(-) delete mode 100644 tests/unit/OnyxDerivedReferenceStabilityTest.tsx delete mode 100644 tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts diff --git a/tests/unit/OnyxDerivedReferenceStabilityTest.tsx b/tests/unit/OnyxDerivedReferenceStabilityTest.tsx deleted file mode 100644 index a1f35eb6766c..000000000000 --- a/tests/unit/OnyxDerivedReferenceStabilityTest.tsx +++ /dev/null @@ -1,117 +0,0 @@ -import {act, renderHook} from '@testing-library/react-native'; - -import useOnyx from '@hooks/useOnyx'; - -import initOnyxDerivedValues from '@userActions/OnyxDerived'; - -import CONST from '@src/CONST'; -import IntlStore from '@src/languages/IntlStore'; -import ONYXKEYS from '@src/ONYXKEYS'; -import type {PersonalDetailsList} from '@src/types/onyx'; - -import type {TupleToUnion} from 'type-fest'; - -import Onyx from 'react-native-onyx'; - -import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; - -const GUIDE_ACCOUNT_ID = 8; -const MEMBER_ACCOUNT_ID = 1; -const NEW_GUIDE_ACCOUNT_ID = 9; -const NEW_MEMBER_ACCOUNT_ID = 2; - -const PERSONAL_DETAILS: PersonalDetailsList = { - [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`, displayName: 'Guide'}, - [MEMBER_ACCOUNT_ID]: {accountID: MEMBER_ACCOUNT_ID, login: 'member@example.com', displayName: 'Member'}, -}; - -const KEYS_READ_WITHOUT_SELECTOR = [ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS, ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP] as const; - -function renderHookWithoutSelector>(key: TKey) { - let renderCount = 0; - const hook = renderHook(() => { - renderCount++; - return useOnyx(key); - }); - return {hook, getRenderCount: () => renderCount}; -} - -describe('OnyxDerived consumers without a selector on PERSONAL_DETAILS_LIST writes', () => { - beforeAll(async () => { - Onyx.init({keys: ONYXKEYS}); - initOnyxDerivedValues(); - await IntlStore.load(CONST.LOCALES.EN); - await Onyx.set(ONYXKEYS.RAM_ONLY_ARE_TRANSLATIONS_LOADING, false); - await waitForBatchedUpdatesWithAct(); - }); - - beforeEach(async () => { - await act(async () => { - await Onyx.clear(); - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); - }); - await waitForBatchedUpdatesWithAct(); - }); - - afterAll(async () => { - await act(async () => { - await Onyx.clear(); - }); - }); - - it.each(KEYS_READ_WITHOUT_SELECTOR)('should keep the %s reference when an unrelated personal detail changes', async (key) => { - // Given a component that reads the derived value without a selector, the way 10 App sites do - const {hook, getRenderCount} = renderHookWithoutSelector(key); - await waitForBatchedUpdatesWithAct(); - const [valueBefore] = hook.result.current; - const rendersBefore = getRenderCount(); - - // When a member renames themselves, which leaves both derived values with the same content - await act(async () => { - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); - }); - await waitForBatchedUpdatesWithAct(); - - // Then the engine's fresh but equal result must not reach the consumer as a new reference or a re-render - expect(hook.result.current[0]).toBe(valueBefore); - expect(getRenderCount()).toBe(rendersBefore); - }); - - it('should return new guide accountIDs when a guide is added', async () => { - // Given a consumer of the guide accountIDs - const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); - await waitForBatchedUpdatesWithAct(); - const [valueBefore] = hook.result.current; - - // When a second guide's personal details arrive - await act(async () => { - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { - [NEW_GUIDE_ACCOUNT_ID]: {accountID: NEW_GUIDE_ACCOUNT_ID, login: `new.guide@${CONST.EMAIL.GUIDES_DOMAIN}`}, - }); - }); - await waitForBatchedUpdatesWithAct(); - - // Then the consumer sees the change, so the stability check above is not passing on a frozen value - expect(hook.result.current[0]).not.toBe(valueBefore); - expect(hook.result.current[0]).toEqual([GUIDE_ACCOUNT_ID, NEW_GUIDE_ACCOUNT_ID]); - }); - - it('should return a new login map when a login is added', async () => { - // Given a consumer of the login to accountID map - const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP); - await waitForBatchedUpdatesWithAct(); - const [valueBefore] = hook.result.current; - - // When a new member's personal details arrive - await act(async () => { - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { - [NEW_MEMBER_ACCOUNT_ID]: {accountID: NEW_MEMBER_ACCOUNT_ID, login: 'new.member@example.com'}, - }); - }); - await waitForBatchedUpdatesWithAct(); - - // Then the consumer sees the new login - expect(hook.result.current[0]).not.toBe(valueBefore); - expect(hook.result.current[0]?.['new.member@example.com']).toBe(NEW_MEMBER_ACCOUNT_ID); - }); -}); diff --git a/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts b/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts deleted file mode 100644 index d450258f2bed..000000000000 --- a/tests/unit/OnyxDerivedSkipUnchangedWriteTest.ts +++ /dev/null @@ -1,80 +0,0 @@ -import * as ActiveSpans from '@libs/telemetry/activeSpans'; - -import initOnyxDerivedValues from '@userActions/OnyxDerived'; -import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; - -import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; -import type {PersonalDetailsList} from '@src/types/onyx'; - -import Onyx from 'react-native-onyx'; - -import getOnyxValue from '../utils/getOnyxValue'; -import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; - -const GUIDE_ACCOUNT_ID = 8; -const MEMBER_ACCOUNT_ID = 1; - -const PERSONAL_DETAILS: PersonalDetailsList = { - [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`}, - [MEMBER_ACCOUNT_ID]: {accountID: MEMBER_ACCOUNT_ID, login: 'member@example.com', displayName: 'Member'}, -}; - -function getGuideWrites(setDerivedValueSpy: jest.Spied) { - return setDerivedValueSpy.mock.calls.filter(([key]) => key === ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); -} - -describe('OnyxDerived skips the write when compute returns the current value', () => { - beforeAll(async () => { - Onyx.init({keys: ONYXKEYS}); - initOnyxDerivedValues(); - await waitForBatchedUpdates(); - }); - - beforeEach(async () => { - jest.restoreAllMocks(); - await Onyx.clear(); - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); - await waitForBatchedUpdates(); - }); - - it('should not write to Onyx when compute returns the current value', async () => { - // Given the guide accountIDs have been computed and stored - const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); - - // When a member is renamed, which recomputes the guides to the value already held - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); - await waitForBatchedUpdates(); - - // Then nothing is written, so subscribers aren't notified and the value isn't rewritten to disk - expect(getGuideWrites(setDerivedValueSpy)).toHaveLength(0); - expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); - }); - - it('should still end the compute span when the write is skipped', async () => { - // Given the compute span is observed - const endSpanSpy = jest.spyOn(ActiveSpans, 'endSpan'); - - // When a recompute returns the current value and skips the write - await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); - await waitForBatchedUpdates(); - - // Then the early return still goes through finally, so no compute span is left open - expect(endSpanSpy).toHaveBeenCalledWith(`${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS}`); - }); - - it('should rewrite the value after Onyx.clear', async () => { - // Given the guide accountIDs are stored and Onyx is then cleared, as on sign-out - await Onyx.clear(); - await waitForBatchedUpdates(); - const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); - - // When the same personal details come back, as on signing in again - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); - await waitForBatchedUpdates(); - - // Then the engine writes the value again instead of treating the pre-clear value as current - expect(getGuideWrites(setDerivedValueSpy).length).toBeGreaterThan(0); - expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); - }); -}); diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index cb5d50da8939..d831d8ed8426 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -1,5 +1,10 @@ +import {act, renderHook} from '@testing-library/react-native'; + +import useOnyx from '@hooks/useOnyx'; + import reportAttributes from '@libs/actions/OnyxDerived/configs/reportAttributes'; import {isReportActionVisible} from '@libs/ReportActionsUtils'; +import * as ActiveSpans from '@libs/telemetry/activeSpans'; import initOnyxDerivedValues from '@userActions/OnyxDerived'; import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; @@ -7,9 +12,10 @@ import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; import CONST from '@src/CONST'; import IntlStore from '@src/languages/IntlStore'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Report, Transaction, TransactionViolation, ReportAction, ReportActions} from '@src/types/onyx'; +import type {PersonalDetailsList, Report, Transaction, TransactionViolation, ReportAction, ReportActions} from '@src/types/onyx'; import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; +import type {TupleToUnion} from 'type-fest'; /* eslint-disable @typescript-eslint/naming-convention */ import Onyx from 'react-native-onyx'; @@ -18,14 +24,41 @@ import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; import {createRandomCompanyCard, createRandomExpensifyCard} from '../utils/collections/card'; import {createRandomReport} from '../utils/collections/reports'; import createRandomTransaction from '../utils/collections/transaction'; +import getOnyxValue from '../utils/getOnyxValue'; import {createMockReport, getFakeReportAction} from '../utils/ReportTestUtils'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; const onyxDerivedTestSetup = () => { Onyx.init({keys: ONYXKEYS}); initOnyxDerivedValues(); }; +const GUIDE_ACCOUNT_ID = 8; +const MEMBER_ACCOUNT_ID = 1; +const NEW_GUIDE_ACCOUNT_ID = 9; +const NEW_MEMBER_ACCOUNT_ID = 2; + +const PERSONAL_DETAILS: PersonalDetailsList = { + [GUIDE_ACCOUNT_ID]: {accountID: GUIDE_ACCOUNT_ID, login: `guide@${CONST.EMAIL.GUIDES_DOMAIN}`, displayName: 'Guide'}, + [MEMBER_ACCOUNT_ID]: {accountID: MEMBER_ACCOUNT_ID, login: 'member@example.com', displayName: 'Member'}, +}; + +const KEYS_READ_WITHOUT_SELECTOR = [ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS, ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP] as const; + +function renderHookWithoutSelector>(key: TKey) { + let renderCount = 0; + const hook = renderHook(() => { + renderCount++; + return useOnyx(key); + }); + return {hook, getRenderCount: () => renderCount}; +} + +function getGuideWrites(setDerivedValueSpy: jest.Spied) { + return setDerivedValueSpy.mock.calls.filter(([key]) => key === ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); +} + describe('OnyxDerived', () => { beforeAll(async () => { onyxDerivedTestSetup(); @@ -992,4 +1025,122 @@ describe('OnyxDerived', () => { }); }); }); + + describe('skipping the write when compute returns the current value', () => { + beforeEach(async () => { + jest.restoreAllMocks(); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + await waitForBatchedUpdates(); + }); + + it('should not write to Onyx when compute returns the current value', async () => { + // Given the guide accountIDs have been computed and stored + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // When a member is renamed, which recomputes the guides to the value already held + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + await waitForBatchedUpdates(); + + // Then nothing is written, so subscribers aren't notified and the value isn't rewritten to disk + expect(getGuideWrites(setDerivedValueSpy)).toHaveLength(0); + expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); + }); + + it('should still end the compute span when the write is skipped', async () => { + // Given the compute span is observed + const endSpanSpy = jest.spyOn(ActiveSpans, 'endSpan'); + + // When a recompute returns the current value and skips the write + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + await waitForBatchedUpdates(); + + // Then the early return still goes through finally, so no compute span is left open + expect(endSpanSpy).toHaveBeenCalledWith(`${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS}`); + }); + + it('should rewrite the value after Onyx.clear', async () => { + // Given the guide accountIDs are stored and Onyx is then cleared, as on sign-out + await Onyx.clear(); + await waitForBatchedUpdates(); + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // When the same personal details come back, as on signing in again + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + await waitForBatchedUpdates(); + + // Then the engine writes the value again instead of treating the pre-clear value as current + expect(getGuideWrites(setDerivedValueSpy).length).toBeGreaterThan(0); + expect(await getOnyxValue(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS)).toEqual([GUIDE_ACCOUNT_ID]); + }); + }); + + describe('consumers without a selector on PERSONAL_DETAILS_LIST writes', () => { + beforeAll(async () => { + await IntlStore.load(CONST.LOCALES.EN); + }); + + beforeEach(async () => { + await act(async () => { + await Onyx.set(ONYXKEYS.RAM_ONLY_ARE_TRANSLATIONS_LOADING, false); + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS); + }); + await waitForBatchedUpdatesWithAct(); + }); + + it.each(KEYS_READ_WITHOUT_SELECTOR)('should keep the %s reference when an unrelated personal detail changes', async (key) => { + // Given a component that reads the derived value without a selector, the way 10 App sites do + const {hook, getRenderCount} = renderHookWithoutSelector(key); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + const rendersBefore = getRenderCount(); + + // When a member renames themselves, which leaves both derived values with the same content + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {[MEMBER_ACCOUNT_ID]: {displayName: 'Renamed member'}}); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the engine's fresh but equal result must not reach the consumer as a new reference or a re-render + expect(hook.result.current[0]).toBe(valueBefore); + expect(getRenderCount()).toBe(rendersBefore); + }); + + it('should return new guide accountIDs when a guide is added', async () => { + // Given a consumer of the guide accountIDs + const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.GUIDE_ACCOUNT_IDS); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + + // When a second guide's personal details arrive + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [NEW_GUIDE_ACCOUNT_ID]: {accountID: NEW_GUIDE_ACCOUNT_ID, login: `new.guide@${CONST.EMAIL.GUIDES_DOMAIN}`}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer sees the change, so the stability check above is not passing on a frozen value + expect(hook.result.current[0]).not.toBe(valueBefore); + expect(hook.result.current[0]).toEqual([GUIDE_ACCOUNT_ID, NEW_GUIDE_ACCOUNT_ID]); + }); + + it('should return a new login map when a login is added', async () => { + // Given a consumer of the login to accountID map + const {hook} = renderHookWithoutSelector(ONYXKEYS.DERIVED.LOGIN_TO_ACCOUNT_ID_MAP); + await waitForBatchedUpdatesWithAct(); + const [valueBefore] = hook.result.current; + + // When a new member's personal details arrive + await act(async () => { + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [NEW_MEMBER_ACCOUNT_ID]: {accountID: NEW_MEMBER_ACCOUNT_ID, login: 'new.member@example.com'}, + }); + }); + await waitForBatchedUpdatesWithAct(); + + // Then the consumer sees the new login + expect(hook.result.current[0]).not.toBe(valueBefore); + expect(hook.result.current[0]?.['new.member@example.com']).toBe(NEW_MEMBER_ACCOUNT_ID); + }); + }); }); From c70a92e2bbe3ea6c5ecc75fcafd3dd157128904e Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 8 Oct 2026 13:16:23 +0200 Subject: [PATCH 3/3] add shallow equal check to reportTransactionsAndViolations --- .../reportTransactionsAndViolations.ts | 4 ++- .../reportTransactionsAndViolationsTest.ts | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts b/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts index e56bd7d756ca..e1432f9013a4 100644 --- a/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts +++ b/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts @@ -5,6 +5,8 @@ import type {TransactionViolation} from '@src/types/onyx'; import type {OnyxCollection} from 'react-native-onyx'; +import {shallowEqual} from 'fast-equals'; + let previousViolations: OnyxCollection = {}; let transactionReportIDMapping: Record = {}; @@ -117,7 +119,7 @@ export default createOnyxDerivedValueConfig({ previousViolations = violations; - return reportTransactionsAndViolations; + return currentValue && shallowEqual(currentValue, reportTransactionsAndViolations) ? currentValue : reportTransactionsAndViolations; }, // On cache clear, drop the cross-compute state so the map is rebuilt from scratch (see the engine's resetForClear). onReset: () => { diff --git a/tests/unit/OnyxDerived/reportTransactionsAndViolationsTest.ts b/tests/unit/OnyxDerived/reportTransactionsAndViolationsTest.ts index 3f8609498f74..5013909e18e4 100644 --- a/tests/unit/OnyxDerived/reportTransactionsAndViolationsTest.ts +++ b/tests/unit/OnyxDerived/reportTransactionsAndViolationsTest.ts @@ -112,4 +112,37 @@ describe('reportTransactionsAndViolations compute', () => { expect(result.reportF?.transactions?.[txKey('8')]).toBeUndefined(); expect(result.reportF?.transactions?.[txKey('9')]).toBeTruthy(); }); + + it('should return the current value when the delta touches no stored transaction', () => { + // Given a derived value holding one transaction + const transactions: OnyxCollection = {[txKey('10')]: makeTx('10', 'reportG')}; + const currentValue: ReportTransactionsAndViolationsDerivedValue = {reportG: {transactions: {[txKey('10')]: makeTx('10', 'reportG')}, violations: {}}}; + + // When a violation lands for a transaction that isn't in the collection + const violations: OnyxCollection = {[violationsKey('unknown')]: [dupViolation]}; + const result = reportTransactionsAndViolationsConfig.compute([transactions, violations], { + sourceValues: {[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]: violations}, + currentValue, + }); + + // Then the same object comes back, so consumers comparing by reference don't re-render + expect(result).toBe(currentValue); + }); + + it('should return a new object when the delta changes a stored transaction', () => { + // Given a derived value holding one transaction + const transactions: OnyxCollection = {[txKey('11')]: makeTx('11', 'reportH')}; + const currentValue: ReportTransactionsAndViolationsDerivedValue = {reportH: {transactions: {[txKey('11')]: makeTx('11', 'reportH')}, violations: {}}}; + + // When that transaction gets a violation + const violations: OnyxCollection = {[violationsKey('11')]: [dupViolation]}; + const result = reportTransactionsAndViolationsConfig.compute([transactions, violations], { + sourceValues: {[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]: violations}, + currentValue, + }); + + // Then a new object carries the violation instead of the current value being reused + expect(result).not.toBe(currentValue); + expect(result.reportH?.violations?.[violationsKey('11')]).toHaveLength(1); + }); });