From 28b09a0a58ff286dbcbe8f8445ae1cb486e8dd8d Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 25 Jun 2026 13:28:05 +0200 Subject: [PATCH 01/12] schedule onyx derived values as macrotask --- .../OnyxDerived/configs/reportAttributes.ts | 6 +- src/libs/actions/OnyxDerived/index.ts | 68 +++++++++++++++---- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts index afb7937405c8..d835684ff1b7 100644 --- a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts +++ b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts @@ -102,9 +102,13 @@ export default createOnyxDerivedValueConfig({ // Check if display names changed when personal details are updated let displayNamesChanged = false; if (hasKeyTriggeredCompute(ONYXKEYS.PERSONAL_DETAILS_LIST, sourceValues)) { + // Must run regardless — it updates the tracked previous display names. displayNamesChanged = checkDisplayNamesChanged(personalDetails); - if (!displayNamesChanged) { + // Only short-circuit when personal details were the sole trigger; coalescing can batch them + // with report/transaction changes, and returning early would drop those. + const personalDetailsIsOnlyTrigger = Object.keys(sourceValues ?? {}).length === 1; + if (!displayNamesChanged && personalDetailsIsOnlyTrigger) { return currentValue ?? {reports: {}, locale: null}; } } else if (!sourceValues) { diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 57950c582d5f..24eb9ceffb0a 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -61,23 +61,15 @@ function init() { sourceValues: undefined, }; - const recomputeDerivedValue = (sourceKey?: string, sourceValue?: unknown, triggeredByIndex?: number) => { - // If this recompute was triggered by a connection callback, check if it initializes the connection - if (!areAllConnectionsSet && triggeredByIndex !== undefined) { - checkAndMarkConnectionInitialized(triggeredByIndex); - } - - // Before all connections are established, don't write to Onyx. - // This prevents overwriting a valid disk-cached value with empty defaults, - // and avoids N-1 unnecessary Onyx writes during initialization. - // We still update dependencyValues via setDependencyValue so data accumulates correctly. - if (!areAllConnectionsSet) { - Log.info(`[OnyxDerived] not all connections set for ${key}, deferring Onyx write`); - return; - } + // Coalesce per-dependency recomputes from one logical change into a single compute on the + // next macrotask. setTimeout(0), not queueMicrotask: Onyx spreads an update's broadcasts + // across microtasks, so a microtask flush would split the batch. + let flushScheduled = false; + let pendingSourceValues: Record | undefined; + const runCompute = (sourceValues: Record | undefined) => { context.currentValue = derivedValue; - context.sourceValues = sourceKey && sourceValue !== undefined ? {[sourceKey]: sourceValue} : undefined; + context.sourceValues = sourceValues as typeof context.sourceValues; const spanId = `${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${key}`; startSpan(spanId, { @@ -98,6 +90,52 @@ function init() { } }; + const flushRecompute = () => { + flushScheduled = false; + const sourceValues = pendingSourceValues; + pendingSourceValues = undefined; + runCompute(sourceValues); + }; + + const accumulateSourceValue = (sourceKey?: string, sourceValue?: unknown) => { + // A trigger with no partial value (e.g. a cleared scalar dep) carries no incremental delta — + // the compute reads such state live. Skip it; the flush still runs. + if (sourceKey === undefined || sourceValue === undefined) { + return; + } + pendingSourceValues ??= {}; + const existing = pendingSourceValues[sourceKey]; + // Collection sourceValues are partial — merge members so a window doesn't drop changed keys. + if (existing && typeof existing === 'object' && typeof sourceValue === 'object') { + pendingSourceValues[sourceKey] = {...existing, ...sourceValue}; + } else { + pendingSourceValues[sourceKey] = sourceValue; + } + }; + + const recomputeDerivedValue = (sourceKey?: string, sourceValue?: unknown, triggeredByIndex?: number) => { + // If this recompute was triggered by a connection callback, check if it initializes the connection + if (!areAllConnectionsSet && triggeredByIndex !== undefined) { + checkAndMarkConnectionInitialized(triggeredByIndex); + } + + // Before all connections are established, don't write to Onyx. + // This prevents overwriting a valid disk-cached value with empty defaults, + // and avoids N-1 unnecessary Onyx writes during initialization. + // We still update dependencyValues via setDependencyValue so data accumulates correctly. + if (!areAllConnectionsSet) { + Log.info(`[OnyxDerived] not all connections set for ${key}, deferring Onyx write`); + return; + } + + accumulateSourceValue(sourceKey, sourceValue); + if (flushScheduled) { + return; + } + flushScheduled = true; + setTimeout(flushRecompute, 0); + }; + for (let i = 0; i < dependencies.length; i++) { const dependencyIndex = i; const dependencyOnyxKey = dependencies[dependencyIndex]; From fceec9a8918db24b3dca11e170c72cdf94c6ea2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Jul 2026 11:07:15 +0100 Subject: [PATCH 02/12] temp --- src/hooks/useCollectionDelta.ts | 16 +++ .../configs/visibleReportActions.ts | 120 ++++++------------ src/libs/actions/OnyxDerived/index.ts | 80 ++++++++---- src/libs/getCollectionDelta.ts | 45 +++++++ tests/ui/components/HeaderViewTest.tsx | 8 +- tests/unit/OnyxDerivedTest.tsx | 54 +++++++- tests/unit/SidebarOrderTest.ts | 67 ++++++---- tests/unit/getCollectionDeltaTest.ts | 87 +++++++++++++ tests/unit/useCollectionDeltaTest.ts | 60 +++++++++ 9 files changed, 397 insertions(+), 140 deletions(-) create mode 100644 src/hooks/useCollectionDelta.ts create mode 100644 src/libs/getCollectionDelta.ts create mode 100644 tests/unit/getCollectionDeltaTest.ts create mode 100644 tests/unit/useCollectionDeltaTest.ts diff --git a/src/hooks/useCollectionDelta.ts b/src/hooks/useCollectionDelta.ts new file mode 100644 index 000000000000..6da5c86a0cdc --- /dev/null +++ b/src/hooks/useCollectionDelta.ts @@ -0,0 +1,16 @@ +import {useMemo} from 'react'; +import type {OnyxCollection} from 'react-native-onyx'; +import getCollectionDelta from '@libs/getCollectionDelta'; +import usePrevious from './usePrevious'; + +/** + * Given the latest collection value from `useOnyx`, returns the subset of members that changed since + * the previous render (added, changed, or removed), or `undefined` when nothing changed. Because Onyx + * structurally shares unchanged members, the underlying diff is a cheap reference-equality scan. + */ +function useCollectionDelta(value: OnyxCollection): OnyxCollection | undefined { + const previous = usePrevious(value); + return useMemo(() => getCollectionDelta(value, previous), [value, previous]); +} + +export default useCollectionDelta; diff --git a/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts index e7b16841b5b1..1df956957511 100644 --- a/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts +++ b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts @@ -1,27 +1,9 @@ -import type {OnyxEntry} from 'react-native-onyx'; import {isActionableWhisperRequiringWritePermission, isConciergeCategoryOptions, shouldReportActionBeVisible} from '@libs/ReportActionsUtils'; import createOnyxDerivedValueConfig from '@userActions/OnyxDerived/createOnyxDerivedValueConfig'; import ONYXKEYS from '@src/ONYXKEYS'; import type {ReportAction, ReportActions} from '@src/types/onyx'; import type {VisibleReportActionsDerivedValue} from '@src/types/onyx/DerivedValues'; -function getOrCreateReportVisibilityRecord(result: VisibleReportActionsDerivedValue, reportID: string, clonedReportIDs: Set): Record { - if (!result[reportID]) { - // Parameter reassignment is necessary here because we are building up the derived value - // object incrementally as we process report actions. Creating a new object would break - // the reference chain and lose previously computed visibility data. - // eslint-disable-next-line no-param-reassign - result[reportID] = {}; - clonedReportIDs.add(reportID); - } else if (!clonedReportIDs.has(reportID)) { - // Clone the existing entry to avoid mutating the cached value - // eslint-disable-next-line no-param-reassign - result[reportID] = {...result[reportID]}; - clonedReportIDs.add(reportID); - } - return result[reportID]; -} - /** * Returns true if the action's visibility depends on runtime context that can't be cached, * such as write permissions or policy settings. @@ -30,6 +12,32 @@ function shouldSkipCachingAction(action: ReportAction): boolean { return isActionableWhisperRequiringWritePermission(action) || isConciergeCategoryOptions(action); } +/** + * Builds a report's action-visibility map (keyed by `reportActionID`) from its full set of report + * actions. Rebuilding the whole map rather than updating individual entries keeps deletions correct: + * a removed action is absent from `reportActions`, so it drops out of the result. + */ +function computeReportVisibility(reportActions: ReportActions): Record { + const reportVisibility: Record = {}; + + for (const [actionID, action] of Object.entries(reportActions)) { + if (!action) { + continue; + } + // Skip deprecated keys (e.g. sequenceNumber-keyed duplicates) so they + // cannot overwrite the canonical entry's visibility with false. + if (actionID !== action.reportActionID) { + continue; + } + if (shouldSkipCachingAction(action)) { + continue; + } + reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined); + } + + return reportVisibility; +} + export default createOnyxDerivedValueConfig({ key: ONYXKEYS.DERIVED.VISIBLE_REPORT_ACTIONS, // Note: REPORT dependency is needed both to trigger recompute when reports change @@ -45,81 +53,25 @@ export default createOnyxDerivedValueConfig({ const reportActionsUpdates = sourceValues?.[ONYXKEYS.COLLECTION.REPORT_ACTIONS]; const sessionUpdates = sourceValues?.[ONYXKEYS.SESSION]; - // Track which reportID entries have been cloned to avoid mutating cached nested objects. - const clonedReportIDs = new Set(); - - // Session change = user changed, need full recompute due to whisper targeting - if (sessionUpdates) { - const result: VisibleReportActionsDerivedValue = {}; + // Recompute only the reports whose actions changed when we have a usable delta. Otherwise + // recompute everything: on first load, when there's no delta, or on a SESSION change (the + // user changed, which affects whisper targeting for every report). + const isIncremental = !!reportActionsUpdates && !sessionUpdates && !!currentValue; - for (const [reportActionsKey, reportActions] of Object.entries(allReportActions)) { - if (!reportActions) { - continue; - } + const result: VisibleReportActionsDerivedValue = isIncremental ? {...currentValue} : {}; + const reportActionsKeysToProcess = isIncremental ? Object.keys(reportActionsUpdates) : Object.keys(allReportActions); - const reportID = reportActionsKey.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, ''); - const reportVisibility = getOrCreateReportVisibilityRecord(result, reportID, clonedReportIDs); - - for (const [actionID, action] of Object.entries(reportActions)) { - if (action) { - if (actionID !== action.reportActionID) { - continue; - } - if (shouldSkipCachingAction(action)) { - continue; - } - reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined); - } - } - } - - return result; - } - - const result: VisibleReportActionsDerivedValue = currentValue ? {...currentValue} : {}; - - const reportActionsToProcess = reportActionsUpdates ? Object.keys(reportActionsUpdates) : Object.keys(allReportActions); - - for (const reportActionsKey of reportActionsToProcess) { - const reportActions: OnyxEntry = allReportActions[reportActionsKey]; + for (const reportActionsKey of reportActionsKeysToProcess) { const reportID = reportActionsKey.replace(ONYXKEYS.COLLECTION.REPORT_ACTIONS, ''); + const reportActions = allReportActions[reportActionsKey]; + // The member was removed entirely — drop the report from the result. if (!reportActions) { delete result[reportID]; continue; } - const reportVisibility = getOrCreateReportVisibilityRecord(result, reportID, clonedReportIDs); - - const specificUpdates = reportActionsUpdates?.[reportActionsKey]; - const actionIDsToProcess = specificUpdates ? Object.keys(specificUpdates) : Object.keys(reportActions); - - for (const actionID of actionIDsToProcess) { - if (specificUpdates?.[actionID] === null) { - delete reportVisibility[actionID]; - continue; - } - - const action = reportActions[actionID]; - if (!action) { - delete reportVisibility[actionID]; - continue; - } - - // Skip deprecated keys (e.g. sequenceNumber-keyed duplicates) so they - // cannot overwrite the canonical entry's visibility with false. - if (actionID !== action.reportActionID) { - delete reportVisibility[actionID]; - continue; - } - - if (shouldSkipCachingAction(action)) { - delete reportVisibility[action.reportActionID]; - continue; - } - - reportVisibility[action.reportActionID] = shouldReportActionBeVisible(action, actionID, undefined); - } + result[reportID] = computeReportVisibility(reportActions); } return result; diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 24eb9ceffb0a..7336b2afefd1 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -6,8 +6,10 @@ * The primary purpose is to optimize performance by reducing redundant computations. More info can be found in the README. */ import Onyx from 'react-native-onyx'; +import type {OnyxCollection} from 'react-native-onyx'; import OnyxKeys from 'react-native-onyx/dist/OnyxKeys'; import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; +import getCollectionDelta from '@libs/getCollectionDelta'; import Log from '@libs/Log'; import {endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans'; import CONST from '@src/CONST'; @@ -65,7 +67,12 @@ function init() { // next macrotask. setTimeout(0), not queueMicrotask: Onyx spreads an update's broadcasts // across microtasks, so a microtask flush would split the batch. let flushScheduled = false; - let pendingSourceValues: Record | undefined; + // Dependency indexes that fired since the last flush; their deltas are reconstructed at flush time. + const pendingDependencyIndexes = new Set(); + // Snapshot of each collection dependency captured at the last flush. We diff the current snapshot + // against it to reconstruct the changed-member delta, instead of relying on Onyx's sourceValue. + const lastFlushedCollectionValues = new Array>(totalConnections); + let hasFlushedOnce = false; const runCompute = (sourceValues: Record | undefined) => { context.currentValue = derivedValue; @@ -90,32 +97,56 @@ function init() { } }; + // dependencyValues is a heterogeneous tuple typed to compute's params; reading a collection entry + // by runtime index yields a union, so we narrow it back to a collection in one place. + const readCollectionDependency = (index: number) => dependencyValues[index] as OnyxCollection; + const flushRecompute = () => { flushScheduled = false; - const sourceValues = pendingSourceValues; - pendingSourceValues = undefined; - runCompute(sourceValues); - }; - const accumulateSourceValue = (sourceKey?: string, sourceValue?: unknown) => { - // A trigger with no partial value (e.g. a cleared scalar dep) carries no incremental delta — - // the compute reads such state live. Skip it; the flush still runs. - if (sourceKey === undefined || sourceValue === undefined) { - return; - } - pendingSourceValues ??= {}; - const existing = pendingSourceValues[sourceKey]; - // Collection sourceValues are partial — merge members so a window doesn't drop changed keys. - if (existing && typeof existing === 'object' && typeof sourceValue === 'object') { - pendingSourceValues[sourceKey] = {...existing, ...sourceValue}; + // Reconstruct the source values at flush time by diffing each dependency that fired since the + // last flush against its last-flushed snapshot. On the very first flush we have no baselines, so + // we compute from scratch (undefined sourceValues) and capture snapshots for future diffs. + let sourceValues: Record | undefined; + if (hasFlushedOnce) { + for (const index of pendingDependencyIndexes) { + const dependencyOnyxKey = dependencies[index]; + if (OnyxKeys.isCollectionKey(dependencyOnyxKey)) { + const currentValue = readCollectionDependency(index); + // Structural sharing keeps unchanged members reference-equal, so this is a cheap scan. + const delta = getCollectionDelta(currentValue, lastFlushedCollectionValues.at(index)); + lastFlushedCollectionValues[index] = currentValue; + if (delta !== undefined) { + sourceValues ??= {}; + sourceValues[dependencyOnyxKey] = delta; + } + } else { + // Non-collection dependency: pass the entire value as the source value. A cleared value + // carries no incremental delta (the compute reads it live), so skip it. + const value = dependencyValues[index]; + if (value !== undefined) { + sourceValues ??= {}; + sourceValues[dependencyOnyxKey] = value; + } + } + } } else { - pendingSourceValues[sourceKey] = sourceValue; + // Capture baselines for every collection dependency so the next flush can diff against them. + for (let index = 0; index < totalConnections; index++) { + if (OnyxKeys.isCollectionKey(dependencies[index])) { + lastFlushedCollectionValues[index] = readCollectionDependency(index); + } + } + hasFlushedOnce = true; } + + pendingDependencyIndexes.clear(); + runCompute(sourceValues); }; - const recomputeDerivedValue = (sourceKey?: string, sourceValue?: unknown, triggeredByIndex?: number) => { + const recomputeDerivedValue = (triggeredByIndex: number) => { // If this recompute was triggered by a connection callback, check if it initializes the connection - if (!areAllConnectionsSet && triggeredByIndex !== undefined) { + if (!areAllConnectionsSet) { checkAndMarkConnectionInitialized(triggeredByIndex); } @@ -128,7 +159,7 @@ function init() { return; } - accumulateSourceValue(sourceKey, sourceValue); + pendingDependencyIndexes.add(triggeredByIndex); if (flushScheduled) { return; } @@ -144,10 +175,10 @@ function init() { Onyx.connectWithoutView({ key: dependencyOnyxKey, waitForCollectionCallback: true, - callback: (value, collectionKey, sourceValue) => { + callback: (value, collectionKey) => { Log.info(`[OnyxDerived] dependency ${collectionKey} for derived key ${key} changed, recomputing`); setDependencyValue(dependencyIndex, value as Parameters[0][typeof dependencyIndex]); - recomputeDerivedValue(dependencyOnyxKey, sourceValue, dependencyIndex); + recomputeDerivedValue(dependencyIndex); }, }); } else if (dependencyOnyxKey === ONYXKEYS.NVP_PREFERRED_LOCALE) { @@ -167,7 +198,7 @@ function init() { } Log.info(`[OnyxDerived] dependency ${dependencyOnyxKey} for derived key ${key} changed, recomputing`); setDependencyValue(dependencyIndex, localeValue as Parameters[0][typeof dependencyIndex]); - recomputeDerivedValue(dependencyOnyxKey, localeValue, dependencyIndex); + recomputeDerivedValue(dependencyIndex); }, }); } else { @@ -176,8 +207,7 @@ function init() { callback: (value) => { Log.info(`[OnyxDerived] dependency ${dependencyOnyxKey} for derived key ${key} changed, recomputing`); setDependencyValue(dependencyIndex, value as Parameters[0][typeof dependencyIndex]); - // if the dependency is not a collection, pass the entire value as the source value - recomputeDerivedValue(dependencyOnyxKey, value, dependencyIndex); + recomputeDerivedValue(dependencyIndex); }, }); } diff --git a/src/libs/getCollectionDelta.ts b/src/libs/getCollectionDelta.ts new file mode 100644 index 000000000000..79bdf0e0b5da --- /dev/null +++ b/src/libs/getCollectionDelta.ts @@ -0,0 +1,45 @@ +import type {OnyxCollection} from 'react-native-onyx'; + +/** + * Computes the subset of collection members that changed between two snapshots. + * Relies on Onyx structural sharing: unchanged members keep the same + * reference, so this is a cheap O(members) reference-equality scan. + * + * A missing (`undefined`) snapshot is treated as an empty collection, so members present on only one + * side count as added/removed. Returns `undefined` only when nothing changed (the two snapshots are + * reference-equal or no member differs). + */ +function getCollectionDelta(current: OnyxCollection, previous: OnyxCollection): OnyxCollection | undefined { + if (current === previous) { + return undefined; + } + + const delta: OnyxCollection | undefined = {}; + let hasChanges = false; + + // Added or changed members (reference differs from the previous snapshot). + if (current) { + for (const key of Object.keys(current)) { + if (current[key] === previous?.[key]) { + continue; + } + delta[key] = current[key]; + hasChanges = true; + } + } + + // Removed members (present in the previous snapshot, gone from the current one). + if (previous) { + for (const key of Object.keys(previous)) { + if (current && key in current) { + continue; + } + delta[key] = undefined; + hasChanges = true; + } + } + + return hasChanges ? delta : undefined; +} + +export default getCollectionDelta; diff --git a/tests/ui/components/HeaderViewTest.tsx b/tests/ui/components/HeaderViewTest.tsx index ba2affeef116..6e2255354d18 100644 --- a/tests/ui/components/HeaderViewTest.tsx +++ b/tests/ui/components/HeaderViewTest.tsx @@ -1,4 +1,4 @@ -import {act, fireEvent, render, screen} from '@testing-library/react-native'; +import {act, fireEvent, render, screen, waitFor} from '@testing-library/react-native'; import React from 'react'; import Onyx from 'react-native-onyx'; import type {KeyValueMapping} from 'react-native-onyx'; @@ -90,7 +90,9 @@ describe('HeaderView', () => { await waitForBatchedUpdatesWithAct(); - expect(screen.getByTestId('DisplayNames')).toHaveTextContent(displayName); + // Report attributes recompute is coalesced onto a macrotask, so the title can settle a tick after + // the initial flush; waitFor retries until the derived recompute + re-render lands. + await waitFor(() => expect(screen.getByTestId('DisplayNames')).toHaveTextContent(displayName)); // When the invoice receiver display name is updated displayName = 'test edit'; @@ -103,7 +105,7 @@ describe('HeaderView', () => { }); // Then the header title should be updated using the new display name - expect(screen.getByTestId('DisplayNames')).toHaveTextContent(displayName); + await waitFor(() => expect(screen.getByTestId('DisplayNames')).toHaveTextContent(displayName)); }); it('should display join button', async () => { diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index 34789f969a06..8a75a44f5a06 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -1,9 +1,10 @@ /* eslint-disable @typescript-eslint/naming-convention */ import Onyx from 'react-native-onyx'; -import type {OnyxCollection} from 'react-native-onyx'; +import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; import reportAttributes from '@libs/actions/OnyxDerived/configs/reportAttributes'; import initOnyxDerivedValues from '@userActions/OnyxDerived'; +import * as OnyxDerivedUtils from '@userActions/OnyxDerived/utils'; import CONST from '@src/CONST'; import IntlStore from '@src/languages/IntlStore'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -103,6 +104,8 @@ describe('OnyxDerived', () => { it('updates when locale changes', async () => { await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, mockReport); await IntlStore.load(CONST.LOCALES.ES); + // Derived recomputes are coalesced onto a macrotask; pump it so the locale change is applied. + await waitForBatchedUpdates(); const derivedReportAttributes = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES); @@ -111,6 +114,55 @@ describe('OnyxDerived', () => { }); }); + describe('coalescing', () => { + // Each flush/recompute writes the derived value exactly once via setDerivedValue, so counting + // its calls for a given derived key = counting how many times that value was recomputed. + const countRecomputes = (spy: jest.SpyInstance, derivedKey: string) => spy.mock.calls.filter(([calledKey]) => calledKey === derivedKey).length; + + it('recomputes a derived value only ONCE for a single Onyx.update touching several of its dependencies', async () => { + // Prime so the derived value is populated and connections are warm. + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, mockReport); + await waitForBatchedUpdates(); + + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // One logical update touching 3 different reportAttributes dependencies at once. + const updates: Array> = [ + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT, value: {[`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`]: {reportName: 'Renamed report'}}}, + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.TRANSACTION, value: {[`${ONYXKEYS.COLLECTION.TRANSACTION}1`]: createRandomTransaction(1)}}, + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT_METADATA, value: {[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`]: {isLoadingInitialReportActions: false}}}, + ]; + await Onyx.update(updates); + await waitForBatchedUpdates(); + + // Without coalescing this would be one recompute per changed dependency (3). + expect(countRecomputes(setDerivedValueSpy, ONYXKEYS.DERIVED.REPORT_ATTRIBUTES)).toBe(1); + + // And the coalesced compute must not drop any of the batched changes. + const derived = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES); + expect(derived?.reports?.[mockReport.reportID]?.reportName).toBe('Renamed report'); + + setDerivedValueSpy.mockRestore(); + }); + + it('coalesces several separate Onyx.merge calls in the same tick into a single recompute', async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, mockReport); + await waitForBatchedUpdates(); + + const setDerivedValueSpy = jest.spyOn(OnyxDerivedUtils, 'setDerivedValue'); + + // Separate merges to different dependencies, fired synchronously (not awaited between). + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, {reportName: 'Renamed again'}); + Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}1`, createRandomTransaction(1)); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`, {isLoadingInitialReportActions: false}); + await waitForBatchedUpdates(); + + expect(countRecomputes(setDerivedValueSpy, ONYXKEYS.DERIVED.REPORT_ATTRIBUTES)).toBe(1); + + setDerivedValueSpy.mockRestore(); + }); + }); + it('should contain both report attributes update when there are report and transaction updates', async () => { await waitForBatchedUpdates(); // Given 2 reports and 1 transaction diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index 05db8f768749..7a8a3dde3185 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -14,6 +14,7 @@ import type {ReportNameValuePairsCollectionDataSet} from '@src/types/onyx/Report import * as LHNTestUtils from '../utils/LHNTestUtils'; import * as TestHelper from '../utils/TestHelper'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates'; // Be sure to include the mocked Permissions libraries or else the beta tests won't work @@ -508,9 +509,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(iouReport.reportID)) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the report data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -519,6 +518,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so the LHN mounts once in its final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(iouReport.reportID)) + .then(() => waitForBatchedUpdatesWithAct()) // Then the order of the reports should be 4 > 3 > 2 > 1 .then(() => { @@ -611,9 +613,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(expenseReport.reportID)) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -623,6 +623,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(expenseReport.reportID)) + .then(() => waitForBatchedUpdatesWithAct()) // Then the order of the reports should be 4 > 3 > 2 > 1 .then(() => { @@ -867,9 +870,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId)) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -879,6 +880,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks(currentReportId)) + .then(() => waitForBatchedUpdatesWithAct()) // Then the reports are ordered by Pinned / GBR > Draft > Rest // there is a pencil icon @@ -925,9 +929,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -936,6 +938,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the reports are in alphabetical order .then(() => { @@ -949,6 +954,7 @@ describe('Sidebar', () => { // When a new report is added .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4)) + .then(() => waitForBatchedUpdatesWithAct()) // Then they are still in alphabetical order .then(() => { @@ -987,9 +993,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -999,6 +1003,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the reports are in alphabetical order .then(() => { @@ -1019,6 +1026,7 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + .then(() => waitForBatchedUpdatesWithAct()) // Then they are still in alphabetical order .then(() => { @@ -1082,9 +1090,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.BETAS]: betas, @@ -1095,6 +1101,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the first report is in last position .then(() => { @@ -1151,9 +1160,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.DEFAULT, @@ -1162,6 +1169,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the reports are ordered alphabetically since their lastVisibleActionCreated are the same .then(() => { @@ -1192,9 +1202,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, LHNTestUtils.fakePersonalDetails)) - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - // Given the sidebar is rendered in #focus mode (hides read chats) - // with all reports having unread comments + // Given the sidebar is in #focus mode (hides read chats) with all reports having unread comments .then(() => Onyx.multiSet({ [ONYXKEYS.NVP_PRIORITY_MODE]: CONST.PRIORITY_MODE.GSD, @@ -1202,6 +1210,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the reports are in alphabetical order .then(() => { @@ -1215,6 +1226,7 @@ describe('Sidebar', () => { // When a new report is added .then(() => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report4.reportID}`, report4)) + .then(() => waitForBatchedUpdatesWithAct()) // Then they are still in alphabetical order .then(() => { @@ -1253,9 +1265,7 @@ describe('Sidebar', () => { return ( waitForBatchedUpdates() - .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) - - // When Onyx is updated with the data and the sidebar re-renders + // When Onyx is updated with the data .then(() => Onyx.multiSet({ [ONYXKEYS.BETAS]: betas, @@ -1266,6 +1276,9 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + // Let the derived reportAttributes settle before the list mounts, so it mounts in final order. + .then(() => LHNTestUtils.getDefaultRenderedSidebarLinks('0')) + .then(() => waitForBatchedUpdatesWithAct()) // Then the first report is in last position .then(() => { diff --git a/tests/unit/getCollectionDeltaTest.ts b/tests/unit/getCollectionDeltaTest.ts new file mode 100644 index 000000000000..f28ce465e2e3 --- /dev/null +++ b/tests/unit/getCollectionDeltaTest.ts @@ -0,0 +1,87 @@ +import type {OnyxCollection} from 'react-native-onyx'; +import getCollectionDelta from '@libs/getCollectionDelta'; + +type Item = {v: number}; + +describe('getCollectionDelta', () => { + it('should return undefined when current and previous are the same reference', () => { + const collection: OnyxCollection = {a: {v: 1}}; + expect(getCollectionDelta(collection, collection)).toBeUndefined(); + }); + + it('should return undefined when both snapshots are undefined', () => { + expect(getCollectionDelta(undefined, undefined)).toBeUndefined(); + }); + + it('should return undefined when no member changed, even if the container objects differ (structural sharing)', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + // Different container objects, but every member keeps its reference. + expect(getCollectionDelta({a, b}, {a, b})).toBeUndefined(); + }); + + it('should report an added member and omit unchanged ones', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + const delta = getCollectionDelta({a, b}, {a}); + expect(delta).toStrictEqual({b}); + }); + + it('should report a changed member with its current value', () => { + const a: Item = {v: 1}; + const aNext: Item = {v: 99}; + const delta = getCollectionDelta({a: aNext}, {a}); + expect(delta).toStrictEqual({a: aNext}); + expect(delta?.a).toBe(aNext); + }); + + it('should report a removed member as undefined (key present)', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + const delta = getCollectionDelta({a}, {a, b}); + expect(delta).toStrictEqual({b: undefined}); + expect(delta).toHaveProperty('b'); + }); + + it('should treat a missing previous snapshot as empty, so every member is added', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + expect(getCollectionDelta({a, b}, undefined)).toStrictEqual({a, b}); + }); + + it('should treat a missing current snapshot as empty, so every member is removed', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + expect(getCollectionDelta(undefined, {a, b})).toStrictEqual({a: undefined, b: undefined}); + }); + + it('should include only added/changed/removed members in a mixed diff', () => { + const unchanged: Item = {v: 1}; + const before: Item = {v: 2}; + const after: Item = {v: 3}; + const removed: Item = {v: 4}; + const added: Item = {v: 5}; + + const delta = getCollectionDelta({unchanged, changed: after, added}, {unchanged, changed: before, removed}); + + expect(delta).toStrictEqual({changed: after, added, removed: undefined}); + expect(delta?.changed).toBe(after); + expect(delta).not.toHaveProperty('unchanged'); + }); + + it('should use reference equality, not deep equality (a re-created but deep-equal member counts as changed)', () => { + const delta = getCollectionDelta({a: {v: 1}}, {a: {v: 1}}); + expect(delta).toHaveProperty('a'); + expect(delta?.a).toEqual({v: 1}); + }); + + it('should report a member explicitly set to null, distinct from a removed member', () => { + const a: Item = {v: 1}; + const delta = getCollectionDelta({a: null}, {a}); + expect(delta).toStrictEqual({a: null}); + }); + + it('should omit a member that is null in both snapshots', () => { + expect(getCollectionDelta({a: null}, {a: null})).toBeUndefined(); + }); +}); diff --git a/tests/unit/useCollectionDeltaTest.ts b/tests/unit/useCollectionDeltaTest.ts new file mode 100644 index 000000000000..ec2768c51432 --- /dev/null +++ b/tests/unit/useCollectionDeltaTest.ts @@ -0,0 +1,60 @@ +import {renderHook} from '@testing-library/react-native'; +import type {OnyxCollection} from 'react-native-onyx'; +import useCollectionDelta from '@hooks/useCollectionDelta'; + +type Item = {v: number}; + +function renderUseCollectionDelta(initialProps: OnyxCollection) { + return renderHook((value: OnyxCollection) => useCollectionDelta(value), {initialProps}); +} + +describe('useCollectionDelta', () => { + it('should return undefined on the first render (nothing to diff against)', () => { + const {result} = renderUseCollectionDelta({a: {v: 1}}); + expect(result.current).toBeUndefined(); + }); + + it('should return the members added since the previous render', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + const {result, rerender} = renderUseCollectionDelta({a}); + + rerender({a, b}); + expect(result.current).toStrictEqual({b}); + }); + + it('should return a changed member with its current value', () => { + const a: Item = {v: 1}; + const aNext: Item = {v: 2}; + const {result, rerender} = renderUseCollectionDelta({a}); + + rerender({a: aNext}); + expect(result.current).toStrictEqual({a: aNext}); + }); + + it('should return a removed member as undefined', () => { + const a: Item = {v: 1}; + const b: Item = {v: 2}; + const {result, rerender} = renderUseCollectionDelta({a, b}); + + rerender({a}); + expect(result.current).toStrictEqual({b: undefined}); + }); + + it('should return undefined when the value reference is unchanged between renders', () => { + const collection: OnyxCollection = {a: {v: 1}}; + const {result, rerender} = renderUseCollectionDelta(collection); + + rerender(collection); + expect(result.current).toBeUndefined(); + }); + + it('should return undefined when members keep their references across renders (structural sharing)', () => { + const a: Item = {v: 1}; + const {result, rerender} = renderUseCollectionDelta({a}); + + // New container object, same member reference. + rerender({a}); + expect(result.current).toBeUndefined(); + }); +}); From 1100c6750a663cc6a731e9a9630782bfed642c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Jul 2026 15:44:22 +0100 Subject: [PATCH 03/12] Cleanup --- src/libs/actions/OnyxDerived/index.ts | 8 +++----- tests/unit/OnyxDerivedTest.tsx | 4 ++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 7336b2afefd1..469566d2727b 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -63,9 +63,7 @@ function init() { sourceValues: undefined, }; - // Coalesce per-dependency recomputes from one logical change into a single compute on the - // next macrotask. setTimeout(0), not queueMicrotask: Onyx spreads an update's broadcasts - // across microtasks, so a microtask flush would split the batch. + // Coalesce per-dependency recomputes from one logical change into a single compute on the next macrotask. let flushScheduled = false; // Dependency indexes that fired since the last flush; their deltas are reconstructed at flush time. const pendingDependencyIndexes = new Set(); @@ -99,6 +97,7 @@ function init() { // dependencyValues is a heterogeneous tuple typed to compute's params; reading a collection entry // by runtime index yields a union, so we narrow it back to a collection in one place. + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion const readCollectionDependency = (index: number) => dependencyValues[index] as OnyxCollection; const flushRecompute = () => { @@ -113,7 +112,6 @@ function init() { const dependencyOnyxKey = dependencies[index]; if (OnyxKeys.isCollectionKey(dependencyOnyxKey)) { const currentValue = readCollectionDependency(index); - // Structural sharing keeps unchanged members reference-equal, so this is a cheap scan. const delta = getCollectionDelta(currentValue, lastFlushedCollectionValues.at(index)); lastFlushedCollectionValues[index] = currentValue; if (delta !== undefined) { @@ -145,7 +143,7 @@ function init() { }; const recomputeDerivedValue = (triggeredByIndex: number) => { - // If this recompute was triggered by a connection callback, check if it initializes the connection + // If this recompute was triggered by a connection callback, check if it initializes the connection. if (!areAllConnectionsSet) { checkAndMarkConnectionInitialized(triggeredByIndex); } diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index 8a75a44f5a06..583999c1dc05 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -130,7 +130,7 @@ describe('OnyxDerived', () => { const updates: Array> = [ {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT, value: {[`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`]: {reportName: 'Renamed report'}}}, {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.TRANSACTION, value: {[`${ONYXKEYS.COLLECTION.TRANSACTION}1`]: createRandomTransaction(1)}}, - {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT_METADATA, value: {[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`]: {isLoadingInitialReportActions: false}}}, + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT_METADATA, value: {[`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`]: {isOptimisticReport: false}}}, ]; await Onyx.update(updates); await waitForBatchedUpdates(); @@ -154,7 +154,7 @@ describe('OnyxDerived', () => { // Separate merges to different dependencies, fired synchronously (not awaited between). Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${mockReport.reportID}`, {reportName: 'Renamed again'}); Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}1`, createRandomTransaction(1)); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`, {isLoadingInitialReportActions: false}); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${mockReport.reportID}`, {isOptimisticReport: false}); await waitForBatchedUpdates(); expect(countRecomputes(setDerivedValueSpy, ONYXKEYS.DERIVED.REPORT_ATTRIBUTES)).toBe(1); From 24222c6131116793965ced0404d05f9cc2bcea21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Jul 2026 16:04:36 +0100 Subject: [PATCH 04/12] Fix formatting --- src/hooks/useCollectionDelta.ts | 7 +++++-- tests/unit/getCollectionDeltaTest.ts | 3 ++- tests/unit/useCollectionDeltaTest.ts | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/hooks/useCollectionDelta.ts b/src/hooks/useCollectionDelta.ts index 6da5c86a0cdc..456ffe9b560f 100644 --- a/src/hooks/useCollectionDelta.ts +++ b/src/hooks/useCollectionDelta.ts @@ -1,6 +1,9 @@ -import {useMemo} from 'react'; -import type {OnyxCollection} from 'react-native-onyx'; import getCollectionDelta from '@libs/getCollectionDelta'; + +import type {OnyxCollection} from 'react-native-onyx'; + +import {useMemo} from 'react'; + import usePrevious from './usePrevious'; /** diff --git a/tests/unit/getCollectionDeltaTest.ts b/tests/unit/getCollectionDeltaTest.ts index f28ce465e2e3..3e939bf6820c 100644 --- a/tests/unit/getCollectionDeltaTest.ts +++ b/tests/unit/getCollectionDeltaTest.ts @@ -1,6 +1,7 @@ -import type {OnyxCollection} from 'react-native-onyx'; import getCollectionDelta from '@libs/getCollectionDelta'; +import type {OnyxCollection} from 'react-native-onyx'; + type Item = {v: number}; describe('getCollectionDelta', () => { diff --git a/tests/unit/useCollectionDeltaTest.ts b/tests/unit/useCollectionDeltaTest.ts index ec2768c51432..65afa9d5144f 100644 --- a/tests/unit/useCollectionDeltaTest.ts +++ b/tests/unit/useCollectionDeltaTest.ts @@ -1,7 +1,9 @@ import {renderHook} from '@testing-library/react-native'; -import type {OnyxCollection} from 'react-native-onyx'; + import useCollectionDelta from '@hooks/useCollectionDelta'; +import type {OnyxCollection} from 'react-native-onyx'; + type Item = {v: number}; function renderUseCollectionDelta(initialProps: OnyxCollection) { From 0842b78a81c39ea0a0c73f3a3bd862a7f3f1ff16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 3 Jul 2026 16:48:49 +0100 Subject: [PATCH 05/12] fix: honor all triggers in coalesced reportTransactionsAndViolations & sortedReportActions recomputes --- .../reportTransactionsAndViolations.ts | 18 ++-- .../configs/sortedReportActions.ts | 8 +- tests/unit/OnyxDerivedTest.tsx | 83 ++++++++++++++++++- 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts b/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts index 9f7af296d060..5b3b27f44724 100644 --- a/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts +++ b/src/libs/actions/OnyxDerived/configs/reportTransactionsAndViolations.ts @@ -23,12 +23,18 @@ export default createOnyxDerivedValueConfig({ const transactionsUpdates = sourceValues?.[ONYXKEYS.COLLECTION.TRANSACTION]; const transactionViolationsUpdates = sourceValues?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]; let transactionsToProcess = Object.keys(transactions); - if (transactionsUpdates) { - transactionsToProcess = Object.keys(transactionsUpdates); - } else if (transactionViolationsUpdates) { - transactionsToProcess = Object.keys(transactionViolationsUpdates).map((transactionViolation) => - transactionViolation.replace(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, ONYXKEYS.COLLECTION.TRANSACTION), - ); + // When we have a delta, process the union of transactions that changed directly and transactions + // whose violations changed. Coalescing can put both in the same flush, so an `if/else` would drop + // the second trigger (e.g. a transaction change for A batched with a violations change for B). + if (transactionsUpdates || transactionViolationsUpdates) { + const transactionKeys = new Set(); + for (const transactionKey of Object.keys(transactionsUpdates ?? {})) { + transactionKeys.add(transactionKey); + } + for (const transactionViolationKey of Object.keys(transactionViolationsUpdates ?? {})) { + transactionKeys.add(transactionViolationKey.replace(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, ONYXKEYS.COLLECTION.TRANSACTION)); + } + transactionsToProcess = Array.from(transactionKeys); } const reportTransactionsAndViolations = currentValue ? {...currentValue} : {}; diff --git a/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts b/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts index f63ffbaca945..6f44d62f0e9b 100644 --- a/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts +++ b/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts @@ -53,8 +53,14 @@ export default createOnyxDerivedValueConfig({ const reportActionsUpdates = sourceValues?.[ONYXKEYS.COLLECTION.REPORT_ACTIONS]; + // The incremental branch only knows how to react to report-action changes; REPORT/NETWORK changes are + // handled by the full recompute below. Coalescing can batch REPORT_ACTIONS with REPORT/NETWORK in a + // single flush, so only go incremental when report actions are the sole trigger — otherwise a batched + // REPORT change (e.g. chatReportID -> transactionThreadReportID) would be silently dropped. + const reportActionsIsOnlyTrigger = !!sourceValues && Object.keys(sourceValues).length === 1; + // Incremental update: only recompute reports whose actions changed - if (reportActionsUpdates && currentValue) { + if (reportActionsUpdates && currentValue && reportActionsIsOnlyTrigger) { const sortedActions = {...currentValue.sortedActions}; const lastActions = {...currentValue.lastActions}; const transactionThreadIDs = {...currentValue.transactionThreadIDs}; diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index c4eb7ad7a991..89c51aa04433 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -6,8 +6,7 @@ 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} from '@src/types/onyx'; -import type {ReportActions} from '@src/types/onyx/ReportAction'; +import type {Report, Transaction, TransactionViolation, ReportAction, ReportActions} from '@src/types/onyx'; import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; @@ -18,6 +17,7 @@ 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 {createMockReport, getFakeReportAction} from '../utils/ReportTestUtils'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; const onyxDerivedTestSetup = () => { @@ -468,6 +468,85 @@ describe('OnyxDerived', () => { }); }); + describe('reportTransactionsAndViolations', () => { + it('keeps a violations-only change for one transaction when coalesced with a transaction change for another', async () => { + const transactionA: Transaction = {...createRandomTransaction(1), transactionID: 'A', reportID: 'rA', amount: 100}; + const transactionB: Transaction = {...createRandomTransaction(2), transactionID: 'B', reportID: 'rB', amount: 200}; + + // Prime so both transactions are tracked and the connections are warm. + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.TRANSACTION}A` as const]: transactionA, + [`${ONYXKEYS.COLLECTION.TRANSACTION}B` as const]: transactionB, + }); + await waitForBatchedUpdates(); + + const violation: TransactionViolation = {type: CONST.VIOLATION_TYPES.VIOLATION, name: CONST.VIOLATIONS.MISSING_CATEGORY}; + + // One logical update: transaction A changes AND violations change for transaction B. + const updates: Array> = [ + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.TRANSACTION, value: {[`${ONYXKEYS.COLLECTION.TRANSACTION}A`]: {amount: 999}}}, + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, value: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}B`]: [violation]}}, + ]; + await Onyx.update(updates); + await waitForBatchedUpdates(); + + const derived = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_TRANSACTIONS_AND_VIOLATIONS); + + // The batched violations change for B must not be dropped... + expect(derived?.rB?.violations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}B`]).toEqual([violation]); + // ...and the transaction change for A must also land. + expect(derived?.rA?.transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}A`]?.amount).toBe(999); + }); + }); + + describe('sortedReportActions', () => { + it('applies a REPORT change that is coalesced with a REPORT_ACTIONS change for another report', async () => { + const chatReportID = '10'; + const expenseReportID = '20'; + const parentChatReportID = '30'; + const threadReportID = '40'; + + const iouAction = getFakeReportAction(100, { + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + childReportID: threadReportID, + reportID: expenseReportID, + originalMessage: {IOUTransactionID: 'txn1', type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, amount: 100, currency: 'USD'}, + } as Partial); + + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${chatReportID}` as const]: createMockReport({reportID: chatReportID, type: CONST.REPORT.TYPE.CHAT}), + // The expense report starts as a CHAT, so its one-transaction thread does not resolve yet. + [`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}` as const]: createMockReport({reportID: expenseReportID, type: CONST.REPORT.TYPE.CHAT, chatReportID: parentChatReportID}), + [`${ONYXKEYS.COLLECTION.REPORT}${parentChatReportID}` as const]: createMockReport({reportID: parentChatReportID, type: CONST.REPORT.TYPE.CHAT}), + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}` as const]: {'1': getFakeReportAction(1, {actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT})}, + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReportID}` as const]: {'100': iouAction}, + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${threadReportID}` as const]: {'200': getFakeReportAction(200, {actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT})}, + }); + await waitForBatchedUpdates(); + + // Precondition: while it is a CHAT, no transaction thread is resolved for the report. + let derived = await OnyxUtils.get(ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS); + expect(derived?.transactionThreadIDs?.[expenseReportID]).toBeUndefined(); + + // One logical update: flip the report to EXPENSE (a REPORT change that resolves its thread) batched + // with an unrelated REPORT_ACTIONS change to a different report. + const updates: Array> = [ + {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYXKEYS.COLLECTION.REPORT, value: {[`${ONYXKEYS.COLLECTION.REPORT}${expenseReportID}`]: {type: CONST.REPORT.TYPE.EXPENSE}}}, + { + onyxMethod: Onyx.METHOD.MERGE_COLLECTION, + key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, + value: {[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReportID}`]: {'2': getFakeReportAction(2, {actionName: CONST.REPORT.ACTIONS.TYPE.ADD_COMMENT})}}, + }, + ]; + await Onyx.update(updates); + await waitForBatchedUpdates(); + + // The batched REPORT change must be applied: the expense report's transaction thread now resolves. + derived = await OnyxUtils.get(ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS); + expect(derived?.transactionThreadIDs?.[expenseReportID]).toBe(threadReportID); + }); + }); + describe('nonPersonalAndWorkspaceCardList', () => { beforeAll(async () => { // Initialize dependency keys so Onyx.clear() in beforeEach triggers derived value recomputation From 3288505959dfe32e160f9a0500af61f25299020a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 6 Jul 2026 07:41:24 +0100 Subject: [PATCH 06/12] Implement scheduleMacrotask lib --- src/libs/actions/OnyxDerived/index.ts | 3 +- src/libs/scheduleMacrotask.ts | 41 +++++++ tests/unit/scheduleMacrotaskTest.ts | 161 ++++++++++++++++++++++++++ 3 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 src/libs/scheduleMacrotask.ts create mode 100644 tests/unit/scheduleMacrotaskTest.ts diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 357489aafd02..0413646e1413 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -1,5 +1,6 @@ import getCollectionDelta from '@libs/getCollectionDelta'; import Log from '@libs/Log'; +import scheduleMacrotask from '@libs/scheduleMacrotask'; import {endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans'; import CONST from '@src/CONST'; @@ -167,7 +168,7 @@ function init() { return; } flushScheduled = true; - setTimeout(flushRecompute, 0); + scheduleMacrotask(flushRecompute); }; for (let i = 0; i < dependencies.length; i++) { diff --git a/src/libs/scheduleMacrotask.ts b/src/libs/scheduleMacrotask.ts new file mode 100644 index 000000000000..ea4d55a61f02 --- /dev/null +++ b/src/libs/scheduleMacrotask.ts @@ -0,0 +1,41 @@ +type Task = () => void; + +type ScheduleMacrotask = (callback: Task) => void; + +/** + * Runs `callback` on the next macrotask — after the current task's microtasks drain (so it still + * coalesces an Onyx update's microtask-spread broadcasts), but without setTimeout's background-tab + * throttling (hidden tabs clamp timers to >= 1s, and to ~1/min after 5 minutes hidden). + * + * Uses MessageChannel — the same technique React's scheduler uses — when available. Falls back to + * setTimeout on React Native (no MessageChannel) and in tests, where timer-based flushing is what the + * test harness (waitForBatchedUpdates) drives. + */ +const scheduleMacrotask: ScheduleMacrotask = (() => { + const useTimeoutFallback = typeof MessageChannel === 'undefined' || process.env.NODE_ENV === 'test'; + if (useTimeoutFallback) { + return (callback) => { + setTimeout(callback, 0); + }; + } + + const queue: Task[] = []; + const channel = new MessageChannel(); + channel.port1.onmessage = () => { + // Drain everything queued for this tick; a task that schedules another lands in the next tick. + const tasks = queue.splice(0); + for (const task of tasks) { + task(); + } + }; + + return (callback) => { + queue.push(callback); + if (queue.length === 1) { + channel.port2.postMessage(null); + } + }; +})(); + +export default scheduleMacrotask; +export type {ScheduleMacrotask}; diff --git a/tests/unit/scheduleMacrotaskTest.ts b/tests/unit/scheduleMacrotaskTest.ts new file mode 100644 index 000000000000..11979c73e7d9 --- /dev/null +++ b/tests/unit/scheduleMacrotaskTest.ts @@ -0,0 +1,161 @@ +/** + * scheduleMacrotask picks its implementation once, at import time, based on `process.env.NODE_ENV` and + * whether `MessageChannel` exists. To exercise the MessageChannel branch we install a controllable fake + * global MessageChannel, clear the test guard, and re-import the module in isolation so its import-time + * selection re-runs under those conditions. + */ + +import type {ScheduleMacrotask} from '@libs/scheduleMacrotask'; + +// A fake MessageChannel we fully control: `postMessage` records a pending delivery instead of hopping the +// event loop, and `deliver()` fires the registered handler — so the macrotask boundary is deterministic. +let deliveries: Array<() => void> = []; +let postCount = 0; + +function createFakePort(postMessage: MessagePort['postMessage'] = () => {}): MessagePort { + return { + onmessage: null, + onmessageerror: null, + postMessage, + close: () => {}, + start: () => {}, + addEventListener: () => {}, + removeEventListener: () => {}, + dispatchEvent: () => false, + }; +} + +class FakeMessageChannel implements MessageChannel { + port1: MessagePort = createFakePort(); + + port2: MessagePort = createFakePort(() => { + postCount += 1; + deliveries.push(() => { + this.port1.onmessage?.(new MessageEvent('message')); + }); + }); +} + +const deliver = () => { + const pending = deliveries.splice(0); + for (const run of pending) { + run(); + } +}; + +const loadScheduler = (nodeEnv: string, messageChannelAvailable = true): ScheduleMacrotask => { + const originalNodeEnv = process.env.NODE_ENV; + const originalMessageChannel = global.MessageChannel; + process.env.NODE_ENV = nodeEnv; + if (messageChannelAvailable) { + global.MessageChannel = FakeMessageChannel; + } else { + // Simulate an environment without MessageChannel (e.g. React Native). + Reflect.deleteProperty(global, 'MessageChannel'); + } + + let scheduler: ScheduleMacrotask = () => {}; + jest.isolateModules(() => { + scheduler = require<{default: ScheduleMacrotask}>('@libs/scheduleMacrotask').default; + }); + + process.env.NODE_ENV = originalNodeEnv; + global.MessageChannel = originalMessageChannel; + return scheduler; +}; + +describe('scheduleMacrotask', () => { + beforeEach(() => { + deliveries = []; + postCount = 0; + }); + + describe('setTimeout fallback', () => { + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); + + it('uses setTimeout (not MessageChannel) when NODE_ENV is test, and runs the callback on a macrotask', () => { + const scheduleMacrotask = loadScheduler('test'); + const callback = jest.fn(); + + scheduleMacrotask(callback); + expect(callback).not.toHaveBeenCalled(); // not synchronous + expect(postCount).toBe(0); // MessageChannel was not used + + jest.runOnlyPendingTimers(); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('uses setTimeout (not MessageChannel) when MessageChannel is not available, even outside the test env', () => { + // NODE_ENV is not 'test' here, so the fallback is chosen purely because MessageChannel is absent. + const scheduleMacrotask = loadScheduler('development', false); + const callback = jest.fn(); + + scheduleMacrotask(callback); + expect(callback).not.toHaveBeenCalled(); // not synchronous + expect(postCount).toBe(0); // there was no MessageChannel to use + + jest.runOnlyPendingTimers(); + expect(callback).toHaveBeenCalledTimes(1); + }); + }); + + describe('MessageChannel path', () => { + it('does not run the callback synchronously', () => { + const scheduleMacrotask = loadScheduler('development'); + const callback = jest.fn(); + + scheduleMacrotask(callback); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('coalesces callbacks scheduled in one tick into a single postMessage and runs them FIFO', () => { + const scheduleMacrotask = loadScheduler('development'); + const order: number[] = []; + + scheduleMacrotask(() => order.push(1)); + scheduleMacrotask(() => order.push(2)); + scheduleMacrotask(() => order.push(3)); + + // One wake-up for the whole batch (the `queue.length === 1` guard). + expect(postCount).toBe(1); + expect(order).toEqual([]); + + deliver(); + expect(order).toEqual([1, 2, 3]); + }); + + it('re-arms for the next batch after a drain', () => { + const scheduleMacrotask = loadScheduler('development'); + const callback = jest.fn(); + + scheduleMacrotask(callback); + deliver(); + expect(callback).toHaveBeenCalledTimes(1); + expect(postCount).toBe(1); + + scheduleMacrotask(callback); + expect(postCount).toBe(2); // posted again because the queue went empty -> non-empty + deliver(); + expect(callback).toHaveBeenCalledTimes(2); + }); + + it('defers a callback scheduled during a drain to the next tick', () => { + const scheduleMacrotask = loadScheduler('development'); + const order: string[] = []; + + scheduleMacrotask(() => { + order.push('first'); + scheduleMacrotask(() => order.push('reentrant')); + }); + + deliver(); + expect(order).toEqual(['first']); // the re-entrant task did not run in this drain + expect(postCount).toBe(2); // ...it re-armed a fresh wake-up + + deliver(); + expect(order).toEqual(['first', 'reentrant']); + }); + }); +}); From 1f3ddeaa78989a52e5140434aeed8ef3ebe0ed8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 6 Jul 2026 10:58:47 +0100 Subject: [PATCH 07/12] Update cspell.json --- cspell.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cspell.json b/cspell.json index fc11acf9656e..8396dd7efadf 100644 --- a/cspell.json +++ b/cspell.json @@ -1045,7 +1045,9 @@ "Prefetch", "Prefetcher", "knip", - "lottiefiles" + "lottiefiles", + "macrotask", + "Macrotask" ], "ignorePaths": [ ".gitignore", From d9f95c629bbb82fc3329cb6d885c37ea96dd913a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 6 Jul 2026 13:06:48 +0100 Subject: [PATCH 08/12] Remove useCollectionDelta --- src/hooks/useCollectionDelta.ts | 19 --------- tests/unit/useCollectionDeltaTest.ts | 62 ---------------------------- 2 files changed, 81 deletions(-) delete mode 100644 src/hooks/useCollectionDelta.ts delete mode 100644 tests/unit/useCollectionDeltaTest.ts diff --git a/src/hooks/useCollectionDelta.ts b/src/hooks/useCollectionDelta.ts deleted file mode 100644 index 456ffe9b560f..000000000000 --- a/src/hooks/useCollectionDelta.ts +++ /dev/null @@ -1,19 +0,0 @@ -import getCollectionDelta from '@libs/getCollectionDelta'; - -import type {OnyxCollection} from 'react-native-onyx'; - -import {useMemo} from 'react'; - -import usePrevious from './usePrevious'; - -/** - * Given the latest collection value from `useOnyx`, returns the subset of members that changed since - * the previous render (added, changed, or removed), or `undefined` when nothing changed. Because Onyx - * structurally shares unchanged members, the underlying diff is a cheap reference-equality scan. - */ -function useCollectionDelta(value: OnyxCollection): OnyxCollection | undefined { - const previous = usePrevious(value); - return useMemo(() => getCollectionDelta(value, previous), [value, previous]); -} - -export default useCollectionDelta; diff --git a/tests/unit/useCollectionDeltaTest.ts b/tests/unit/useCollectionDeltaTest.ts deleted file mode 100644 index 65afa9d5144f..000000000000 --- a/tests/unit/useCollectionDeltaTest.ts +++ /dev/null @@ -1,62 +0,0 @@ -import {renderHook} from '@testing-library/react-native'; - -import useCollectionDelta from '@hooks/useCollectionDelta'; - -import type {OnyxCollection} from 'react-native-onyx'; - -type Item = {v: number}; - -function renderUseCollectionDelta(initialProps: OnyxCollection) { - return renderHook((value: OnyxCollection) => useCollectionDelta(value), {initialProps}); -} - -describe('useCollectionDelta', () => { - it('should return undefined on the first render (nothing to diff against)', () => { - const {result} = renderUseCollectionDelta({a: {v: 1}}); - expect(result.current).toBeUndefined(); - }); - - it('should return the members added since the previous render', () => { - const a: Item = {v: 1}; - const b: Item = {v: 2}; - const {result, rerender} = renderUseCollectionDelta({a}); - - rerender({a, b}); - expect(result.current).toStrictEqual({b}); - }); - - it('should return a changed member with its current value', () => { - const a: Item = {v: 1}; - const aNext: Item = {v: 2}; - const {result, rerender} = renderUseCollectionDelta({a}); - - rerender({a: aNext}); - expect(result.current).toStrictEqual({a: aNext}); - }); - - it('should return a removed member as undefined', () => { - const a: Item = {v: 1}; - const b: Item = {v: 2}; - const {result, rerender} = renderUseCollectionDelta({a, b}); - - rerender({a}); - expect(result.current).toStrictEqual({b: undefined}); - }); - - it('should return undefined when the value reference is unchanged between renders', () => { - const collection: OnyxCollection = {a: {v: 1}}; - const {result, rerender} = renderUseCollectionDelta(collection); - - rerender(collection); - expect(result.current).toBeUndefined(); - }); - - it('should return undefined when members keep their references across renders (structural sharing)', () => { - const a: Item = {v: 1}; - const {result, rerender} = renderUseCollectionDelta({a}); - - // New container object, same member reference. - rerender({a}); - expect(result.current).toBeUndefined(); - }); -}); From f28899c9f274cfa580069738f759cab8fab04d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 7 Jul 2026 16:09:36 +0100 Subject: [PATCH 09/12] Add try/catch to scheduleMacrotask --- src/libs/scheduleMacrotask.ts | 18 ++++++++++++++++-- tests/unit/scheduleMacrotaskTest.ts | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/libs/scheduleMacrotask.ts b/src/libs/scheduleMacrotask.ts index ea4d55a61f02..e85ad45c4aba 100644 --- a/src/libs/scheduleMacrotask.ts +++ b/src/libs/scheduleMacrotask.ts @@ -1,7 +1,21 @@ +import Log from './Log'; + type Task = () => void; type ScheduleMacrotask = (callback: Task) => void; +/** + * Run a scheduled task, isolating a throw so it neither aborts sibling tasks queued in the same tick + * nor escapes uncaught. + */ +function runTask(task: Task) { + try { + task(); + } catch (error) { + Log.alert('[scheduleMacrotask] scheduled task threw', {error}); + } +} + /** * Runs `callback` on the next macrotask — after the current task's microtasks drain (so it still * coalesces an Onyx update's microtask-spread broadcasts), but without setTimeout's background-tab @@ -15,7 +29,7 @@ const scheduleMacrotask: ScheduleMacrotask = (() => { const useTimeoutFallback = typeof MessageChannel === 'undefined' || process.env.NODE_ENV === 'test'; if (useTimeoutFallback) { return (callback) => { - setTimeout(callback, 0); + setTimeout(() => runTask(callback), 0); }; } @@ -25,7 +39,7 @@ const scheduleMacrotask: ScheduleMacrotask = (() => { // Drain everything queued for this tick; a task that schedules another lands in the next tick. const tasks = queue.splice(0); for (const task of tasks) { - task(); + runTask(task); } }; diff --git a/tests/unit/scheduleMacrotaskTest.ts b/tests/unit/scheduleMacrotaskTest.ts index 11979c73e7d9..b59097689113 100644 --- a/tests/unit/scheduleMacrotaskTest.ts +++ b/tests/unit/scheduleMacrotaskTest.ts @@ -7,6 +7,12 @@ import type {ScheduleMacrotask} from '@libs/scheduleMacrotask'; +const mockLogAlert = jest.fn(); +jest.mock('@libs/Log', () => ({ + __esModule: true, + default: {alert: mockLogAlert}, +})); + // A fake MessageChannel we fully control: `postMessage` records a pending delivery instead of hopping the // event loop, and `deliver()` fires the registered handler — so the macrotask boundary is deterministic. let deliveries: Array<() => void> = []; @@ -68,6 +74,7 @@ describe('scheduleMacrotask', () => { beforeEach(() => { deliveries = []; postCount = 0; + mockLogAlert.mockClear(); }); describe('setTimeout fallback', () => { @@ -141,6 +148,23 @@ describe('scheduleMacrotask', () => { expect(callback).toHaveBeenCalledTimes(2); }); + it('isolates a throwing task so sibling tasks queued in the same tick still run, and logs it', () => { + const scheduleMacrotask = loadScheduler('development'); + const order: string[] = []; + + scheduleMacrotask(() => order.push('before')); + scheduleMacrotask(() => { + throw new Error('boom'); + }); + scheduleMacrotask(() => order.push('after')); + + deliver(); + + // The throw did not abort the drain — sibling tasks still ran — and it was logged. + expect(order).toEqual(['before', 'after']); + expect(mockLogAlert).toHaveBeenCalledTimes(1); + }); + it('defers a callback scheduled during a drain to the next tick', () => { const scheduleMacrotask = loadScheduler('development'); const order: string[] = []; From 8735ed17b63c772c9ddf3a987bb7bb1db0fbc412 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 7 Jul 2026 16:32:27 +0100 Subject: [PATCH 10/12] Preserve OnyxDerived pending deltas when a coalesced compute throws --- src/libs/actions/OnyxDerived/index.ts | 25 +++++++-- tests/unit/OnyxDerivedSelfHealTest.ts | 76 +++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 tests/unit/OnyxDerivedSelfHealTest.ts diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 0413646e1413..1b6f2360ae09 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -112,14 +112,19 @@ function init() { // Reconstruct the source values at flush time by diffing each dependency that fired since the // last flush against its last-flushed snapshot. On the very first flush we have no baselines, so // we compute from scratch (undefined sourceValues) and capture snapshots for future diffs. + // + // We only STAGE the baseline advances / pending-set clear here and commit them after a + // successful compute. If the compute throws, we keep the baselines and pending set intact so the + // next dependency change re-diffs the accumulated delta and self-heals. let sourceValues: Record | undefined; + const stagedBaselines: Array<[number, OnyxCollection]> = []; if (hasFlushedOnce) { for (const index of pendingDependencyIndexes) { const dependencyOnyxKey = dependencies[index]; if (OnyxKeys.isCollectionKey(dependencyOnyxKey)) { const currentValue = readCollectionDependency(index); const delta = getCollectionDelta(currentValue, lastFlushedCollectionValues.at(index)); - lastFlushedCollectionValues[index] = currentValue; + stagedBaselines.push([index, currentValue]); if (delta !== undefined) { sourceValues ??= {}; sourceValues[dependencyOnyxKey] = delta; @@ -138,14 +143,26 @@ function init() { // Capture baselines for every collection dependency so the next flush can diff against them. for (let index = 0; index < totalConnections; index++) { if (OnyxKeys.isCollectionKey(dependencies[index])) { - lastFlushedCollectionValues[index] = readCollectionDependency(index); + stagedBaselines.push([index, readCollectionDependency(index)]); } } - hasFlushedOnce = true; } + try { + runCompute(sourceValues); + } catch (error) { + // Leave the baselines and pending set intact so the next dependency change re-diffs the + // accumulated delta and recomputes. flushScheduled is already false, so it will reschedule. + Log.alert(`[OnyxDerived] compute for ${key} threw; keeping pending deltas so the next dependency change recomputes them`, {error}); + return; + } + + // Commit only after a successful compute. + for (const [index, value] of stagedBaselines) { + lastFlushedCollectionValues[index] = value; + } + hasFlushedOnce = true; pendingDependencyIndexes.clear(); - runCompute(sourceValues); }; const recomputeDerivedValue = (triggeredByIndex: number) => { diff --git a/tests/unit/OnyxDerivedSelfHealTest.ts b/tests/unit/OnyxDerivedSelfHealTest.ts new file mode 100644 index 000000000000..a438a3ff302f --- /dev/null +++ b/tests/unit/OnyxDerivedSelfHealTest.ts @@ -0,0 +1,76 @@ +import type reportTransactionsAndViolationsConfig from '@libs/actions/OnyxDerived/configs/reportTransactionsAndViolations'; + +import initOnyxDerivedValues from '@userActions/OnyxDerived'; + +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Transaction} from '@src/types/onyx'; + +import Onyx from 'react-native-onyx'; +import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; + +import createRandomTransaction from '../utils/collections/transaction'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +// Force reportTransactionsAndViolations' compute to throw once, on demand, so we can verify the engine +// self-heals: a compute that throws must not lose the deltas that triggered the failed flush. It has to be +// the compute (not the Onyx write) that throws — a thrown write would still leave the change in the +// in-memory derived value, masking the bug. +let mockShouldThrowCompute = false; +jest.mock('@libs/actions/OnyxDerived/configs/reportTransactionsAndViolations', () => { + const actual = jest.requireActual<{default: typeof reportTransactionsAndViolationsConfig}>('@libs/actions/OnyxDerived/configs/reportTransactionsAndViolations'); + const actualCompute = actual.default.compute; + return { + __esModule: true, + default: { + ...actual.default, + compute: (dependencyValues: Parameters[0], context: Parameters[1]) => { + if (mockShouldThrowCompute) { + mockShouldThrowCompute = false; + throw new Error('compute boom'); + } + return actualCompute(dependencyValues, context); + }, + }, + }; +}); + +describe('OnyxDerived self-healing after a compute throws', () => { + beforeAll(async () => { + Onyx.init({keys: ONYXKEYS}); + initOnyxDerivedValues(); + await waitForBatchedUpdates(); + }); + + beforeEach(async () => { + mockShouldThrowCompute = false; + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + it('recovers the deltas from a failed flush on the next dependency change', async () => { + const transactionA: Transaction = {...createRandomTransaction(1), transactionID: 'A', reportID: 'rA', amount: 100}; + const transactionB: Transaction = {...createRandomTransaction(2), transactionID: 'B', reportID: 'rA', amount: 200}; + + // Establish a baseline: transaction A tracked for report rA. + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}A`, transactionA); + await waitForBatchedUpdates(); + + // Change A's amount, but make this flush's compute throw. The delta must not be lost. + mockShouldThrowCompute = true; + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}A`, {amount: 999}); + await waitForBatchedUpdates(); + + // The failed flush did not persist anything. + let derived = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_TRANSACTIONS_AND_VIOLATIONS); + expect(derived?.rA?.transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}A`]?.amount).toBe(100); + + // A later, unrelated change triggers a successful flush that must include the previously-failed delta. + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}B`, transactionB); + await waitForBatchedUpdates(); + + derived = await OnyxUtils.get(ONYXKEYS.DERIVED.REPORT_TRANSACTIONS_AND_VIOLATIONS); + // A's amount change (from the failed flush) is recovered, and B is added. + expect(derived?.rA?.transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}A`]?.amount).toBe(999); + expect(derived?.rA?.transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}B`]?.amount).toBe(200); + }); +}); From 291d0ffe73c13bd91fe8a7e060882ce33c536dfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 7 Jul 2026 17:47:51 +0100 Subject: [PATCH 11/12] Detect OnyxDerived triggers via triggeredKeys, not sourceValues --- .../OnyxDerived/configs/reportAttributes.ts | 14 ++++---- .../configs/sortedReportActions.ts | 4 +-- .../configs/visibleReportActions.ts | 8 ++--- src/libs/actions/OnyxDerived/index.ts | 14 ++++++-- src/libs/actions/OnyxDerived/types.ts | 4 +++ src/libs/actions/OnyxDerived/utils.ts | 16 ++++----- .../OnyxDerived/visibleReportActionsTest.ts | 34 +++++++++++++++++++ tests/unit/reportAttributesTest.ts | 6 ++++ 8 files changed, 75 insertions(+), 25 deletions(-) create mode 100644 tests/unit/OnyxDerived/visibleReportActionsTest.ts diff --git a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts index 189c057f8c5b..9be4b86ac477 100644 --- a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts +++ b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts @@ -117,20 +117,20 @@ export default createOnyxDerivedValueConfig({ conciergeReportID, introSelected, ], - {currentValue, sourceValues}, + {currentValue, sourceValues, triggeredKeys}, ) => { // Read the in-memory offline state directly (NETWORK is a dependency so recompute still fires when it changes). const isOffline = getIsOffline(); const translate: LocalizedTranslate = (path, ...parameters) => translateForLocale(preferredLocale, path, ...parameters); // Check if display names changed when personal details are updated let displayNamesChanged = false; - if (hasKeyTriggeredCompute(ONYXKEYS.PERSONAL_DETAILS_LIST, sourceValues)) { + if (hasKeyTriggeredCompute(ONYXKEYS.PERSONAL_DETAILS_LIST, triggeredKeys)) { // Must run regardless — it updates the tracked previous display names. displayNamesChanged = checkDisplayNamesChanged(personalDetails); // Only short-circuit when personal details were the sole trigger; coalescing can batch them // with report/transaction changes, and returning early would drop those. - const personalDetailsIsOnlyTrigger = Object.keys(sourceValues ?? {}).length === 1; + const personalDetailsIsOnlyTrigger = triggeredKeys?.size === 1; if (!displayNamesChanged && personalDetailsIsOnlyTrigger) { return currentValue ?? {reports: {}, locale: null}; } @@ -143,14 +143,14 @@ export default createOnyxDerivedValueConfig({ // We compare preferredLocale against currentValue?.locale so that the first locale load on startup // (where both equal the same persisted value) does not trigger an unnecessary full recompute. let needsFullRecompute = - (hasKeyTriggeredCompute(ONYXKEYS.NVP_PREFERRED_LOCALE, sourceValues) && preferredLocale !== currentValue?.locale) || + (hasKeyTriggeredCompute(ONYXKEYS.NVP_PREFERRED_LOCALE, triggeredKeys) && preferredLocale !== currentValue?.locale) || displayNamesChanged || - hasKeyTriggeredCompute(ONYXKEYS.CONCIERGE_REPORT_ID, sourceValues) || - hasKeyTriggeredCompute(ONYXKEYS.NVP_INTRO_SELECTED, sourceValues); + hasKeyTriggeredCompute(ONYXKEYS.CONCIERGE_REPORT_ID, triggeredKeys) || + hasKeyTriggeredCompute(ONYXKEYS.NVP_INTRO_SELECTED, triggeredKeys); // if policies are loaded first time, we need to recompute all report attributes to get correct action badge in LHN, such as Approve because it depends on policy's type (see canApproveIOU function) const policyChangedReportKeys: string[] = []; - if (hasKeyTriggeredCompute(ONYXKEYS.COLLECTION.POLICY, sourceValues)) { + if (hasKeyTriggeredCompute(ONYXKEYS.COLLECTION.POLICY, triggeredKeys)) { if (Object.keys(previousPolicies ?? {}).length === 0 && Object.keys(policies ?? {}).length > 0) { needsFullRecompute = true; } else if (!needsFullRecompute) { diff --git a/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts b/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts index 6f44d62f0e9b..5240d3ea5d9d 100644 --- a/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts +++ b/src/libs/actions/OnyxDerived/configs/sortedReportActions.ts @@ -43,7 +43,7 @@ function computeForReport( export default createOnyxDerivedValueConfig({ key: ONYXKEYS.DERIVED.RAM_ONLY_SORTED_REPORT_ACTIONS, dependencies: [ONYXKEYS.COLLECTION.REPORT_ACTIONS, ONYXKEYS.COLLECTION.REPORT, ONYXKEYS.NETWORK], - compute: ([allReportActions, allReports], {sourceValues, currentValue}): SortedReportActionsDerivedValue => { + compute: ([allReportActions, allReports], {sourceValues, currentValue, triggeredKeys}): SortedReportActionsDerivedValue => { if (!allReportActions) { return EMPTY_VALUE; } @@ -57,7 +57,7 @@ export default createOnyxDerivedValueConfig({ // handled by the full recompute below. Coalescing can batch REPORT_ACTIONS with REPORT/NETWORK in a // single flush, so only go incremental when report actions are the sole trigger — otherwise a batched // REPORT change (e.g. chatReportID -> transactionThreadReportID) would be silently dropped. - const reportActionsIsOnlyTrigger = !!sourceValues && Object.keys(sourceValues).length === 1; + const reportActionsIsOnlyTrigger = triggeredKeys?.size === 1; // Incremental update: only recompute reports whose actions changed if (reportActionsUpdates && currentValue && reportActionsIsOnlyTrigger) { diff --git a/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts index a7cd238eb87d..ca310ec14fc9 100644 --- a/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts +++ b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts @@ -47,18 +47,18 @@ export default createOnyxDerivedValueConfig({ // report collection to the visibility check, avoiding stale data from global connections. // SESSION dependency is needed for whisper targeting when user changes. dependencies: [ONYXKEYS.COLLECTION.REPORT_ACTIONS, ONYXKEYS.SESSION], - compute: ([allReportActions], {sourceValues, currentValue}): VisibleReportActionsDerivedValue => { + compute: ([allReportActions], {sourceValues, currentValue, triggeredKeys}): VisibleReportActionsDerivedValue => { if (!allReportActions) { return {}; } const reportActionsUpdates = sourceValues?.[ONYXKEYS.COLLECTION.REPORT_ACTIONS]; - const sessionUpdates = sourceValues?.[ONYXKEYS.SESSION]; // Recompute only the reports whose actions changed when we have a usable delta. Otherwise // recompute everything: on first load, when there's no delta, or on a SESSION change (the - // user changed, which affects whisper targeting for every report). - const isIncremental = !!reportActionsUpdates && !sessionUpdates && !!currentValue; + // user changed, which affects whisper targeting for every report). SESSION is checked via + // triggeredKeys, not sourceValues, so a session cleared to `undefined` still forces the full recompute. + const isIncremental = !!reportActionsUpdates && !triggeredKeys?.has(ONYXKEYS.SESSION) && !!currentValue; const result: VisibleReportActionsDerivedValue = isIncremental ? {...currentValue} : {}; const reportActionsKeysToProcess = isIncremental ? Object.keys(reportActionsUpdates) : Object.keys(allReportActions); diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 1b6f2360ae09..171c67671c38 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -5,6 +5,7 @@ import {endSpan, getSpan, startSpan} from '@libs/telemetry/activeSpans'; import CONST from '@src/CONST'; import IntlStore from '@src/languages/IntlStore'; +import type {OnyxKey} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; import ObjectUtils from '@src/types/utils/ObjectUtils'; @@ -78,9 +79,10 @@ function init() { const lastFlushedCollectionValues = new Array>(totalConnections); let hasFlushedOnce = false; - const runCompute = (sourceValues: Record | undefined) => { + const runCompute = (sourceValues: Record | undefined, triggeredKeys: Set) => { context.currentValue = derivedValue; context.sourceValues = sourceValues as typeof context.sourceValues; + context.triggeredKeys = triggeredKeys; const spanId = `${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${key}`; startSpan(spanId, { @@ -118,6 +120,14 @@ function init() { // next dependency change re-diffs the accumulated delta and self-heals. let sourceValues: Record | undefined; const stagedBaselines: Array<[number, OnyxCollection]> = []; + + // Every dependency that fired this flush, regardless of whether it produced a delta. Configs use + // this (not sourceValues) to detect which dependencies triggered — see hasKeyTriggeredCompute. + const triggeredKeys = new Set(); + for (const index of pendingDependencyIndexes) { + triggeredKeys.add(dependencies[index]); + } + if (hasFlushedOnce) { for (const index of pendingDependencyIndexes) { const dependencyOnyxKey = dependencies[index]; @@ -149,7 +159,7 @@ function init() { } try { - runCompute(sourceValues); + runCompute(sourceValues, triggeredKeys); } catch (error) { // Leave the baselines and pending set intact so the next dependency change re-diffs the // accumulated delta and recomputes. flushScheduled is already false, so it will reschedule. diff --git a/src/libs/actions/OnyxDerived/types.ts b/src/libs/actions/OnyxDerived/types.ts index f2d0a57a05b7..be7c71857a64 100644 --- a/src/libs/actions/OnyxDerived/types.ts +++ b/src/libs/actions/OnyxDerived/types.ts @@ -17,6 +17,10 @@ type DerivedSourceValues = Partial<{ type DerivedValueContext>> = { currentValue?: OnyxValue; sourceValues?: DerivedSourceValues; + // The dependency keys that fired since the last flush. Unlike `sourceValues` (which only holds + // non-empty deltas), this reflects every dependency that triggered — including a scalar cleared to + // `undefined` or a collection with no changed members — so trigger-detection can't miss a fire. + triggeredKeys?: Set; }; /** diff --git a/src/libs/actions/OnyxDerived/utils.ts b/src/libs/actions/OnyxDerived/utils.ts index 8cfff79d0269..8d37bbbaecb5 100644 --- a/src/libs/actions/OnyxDerived/utils.ts +++ b/src/libs/actions/OnyxDerived/utils.ts @@ -1,21 +1,17 @@ import type {OnyxDerivedKey, OnyxKey} from '@src/ONYXKEYS'; import type {OnyxInput} from 'react-native-onyx'; -import type {NonEmptyTuple} from 'type-fest'; import Onyx from 'react-native-onyx'; -import type {DerivedValueContext} from './types'; - /** - * Check if a specific key exists in sourceValue from OnyxDerived + * Check whether a specific dependency key triggered the current OnyxDerived compute. + * + * This reads `triggeredKeys` (every dependency that fired) rather than `sourceValues` (only the ones + * that produced a non-empty delta), so a dependency cleared to `undefined` — or a collection with no + * changed members — is still correctly reported as having triggered. */ -const hasKeyTriggeredCompute = >>(key: TKey, sourceValues: DerivedValueContext['sourceValues']) => { - if (!sourceValues) { - return false; - } - return Object.keys(sourceValues).some((sourceKey) => sourceKey === key); -}; +const hasKeyTriggeredCompute = (key: OnyxKey, triggeredKeys: Set | undefined): boolean => triggeredKeys?.has(key) ?? false; /** * Set a derived value in Onyx diff --git a/tests/unit/OnyxDerived/visibleReportActionsTest.ts b/tests/unit/OnyxDerived/visibleReportActionsTest.ts new file mode 100644 index 000000000000..b0a5f44d9ebe --- /dev/null +++ b/tests/unit/OnyxDerived/visibleReportActionsTest.ts @@ -0,0 +1,34 @@ +import visibleReportActionsConfig from '@libs/actions/OnyxDerived/configs/visibleReportActions'; + +import type {OnyxKey} from '@src/ONYXKEYS'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {ReportActions} from '@src/types/onyx'; +import type {VisibleReportActionsDerivedValue} from '@src/types/onyx/DerivedValues'; + +import type {OnyxCollection} from 'react-native-onyx'; + +describe('visibleReportActions', () => { + describe('SESSION detected via triggeredKeys', () => { + const reportActionsKeyA = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}A`; + // Only report A has report actions now; report B exists only in the previous derived value (currentValue). + const allReportActions: OnyxCollection = {[reportActionsKeyA]: {}}; + const args: Parameters[0] = [allReportActions, undefined]; + const currentValue: VisibleReportActionsDerivedValue = {A: {}, B: {}}; + const sourceValues: Parameters[1]['sourceValues'] = {[ONYXKEYS.COLLECTION.REPORT_ACTIONS]: {[reportActionsKeyA]: {}}}; + + it('recomputes incrementally (keeps a stale report) when only report actions triggered', () => { + const result = visibleReportActionsConfig.compute(args, {currentValue, sourceValues, triggeredKeys: new Set([ONYXKEYS.COLLECTION.REPORT_ACTIONS])}); + + // Incremental: report B (absent from allReportActions) is retained from the previous value. + expect(result.B).toBeDefined(); + }); + + it('does a full recompute (drops the stale report) when SESSION also triggered, even with no SESSION delta', () => { + const result = visibleReportActionsConfig.compute(args, {currentValue, sourceValues, triggeredKeys: new Set([ONYXKEYS.COLLECTION.REPORT_ACTIONS, ONYXKEYS.SESSION])}); + + // Full recompute: report B is not in allReportActions, so it is dropped. + expect(result.B).toBeUndefined(); + expect(result.A).toBeDefined(); + }); + }); +}); diff --git a/tests/unit/reportAttributesTest.ts b/tests/unit/reportAttributesTest.ts index f31388859e2d..0774a8f9188e 100644 --- a/tests/unit/reportAttributesTest.ts +++ b/tests/unit/reportAttributesTest.ts @@ -3,6 +3,7 @@ import {hasPolicyRelevantFieldChanged} from '@userActions/OnyxDerived/configs/re import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import type {OnyxKey} from '@src/ONYXKEYS'; import type {Policy, Report, ReportAttributesDerivedValue} from '@src/types/onyx'; import type {OnyxCollection} from 'react-native-onyx'; @@ -183,6 +184,7 @@ describe('reportAttributes compute — policy change code flow', () => { const result = config.compute(buildArgs(), { currentValue: undefined, sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies as never}, + triggeredKeys: new Set([ONYXKEYS.COLLECTION.POLICY]), }); expect(result?.reports).toHaveProperty('r1'); @@ -194,6 +196,7 @@ describe('reportAttributes compute — policy change code flow', () => { config.compute(buildArgs(), { currentValue: undefined, sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies as never}, + triggeredKeys: new Set([ONYXKEYS.COLLECTION.POLICY]), }); const policy1Changed = {...policy1, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL} as unknown as Policy; @@ -216,6 +219,7 @@ describe('reportAttributes compute — policy change code flow', () => { const result = config.compute(buildArgs(updatedPolicies), { currentValue: existingValue, sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1Changed} as never}, + triggeredKeys: new Set([ONYXKEYS.COLLECTION.POLICY]), }); // r1 (policy1 changed) should be recomputed with new name @@ -229,6 +233,7 @@ describe('reportAttributes compute — policy change code flow', () => { config.compute(buildArgs(), { currentValue: undefined, sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies as never}, + triggeredKeys: new Set([ONYXKEYS.COLLECTION.POLICY]), }); const policy1WithNameChange = {...policy1, name: 'New Policy Name'} as unknown as Policy; @@ -248,6 +253,7 @@ describe('reportAttributes compute — policy change code flow', () => { const result = config.compute(buildArgs(updatedPolicies), { currentValue: existingValue, sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1WithNameChange} as never}, + triggeredKeys: new Set([ONYXKEYS.COLLECTION.POLICY]), }); // No tracked fields changed → return currentValue unchanged From fe9d4e4f199cac1dd36527d6dfcc56eb172c2de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 8 Jul 2026 16:47:25 +0100 Subject: [PATCH 12/12] Address comment --- src/libs/actions/OnyxDerived/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index 171c67671c38..eb1e5eaeaa56 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -72,8 +72,10 @@ function init() { // Coalesce per-dependency recomputes from one logical change into a single compute on the next macrotask. let flushScheduled = false; + // Dependency indexes that fired since the last flush; their deltas are reconstructed at flush time. const pendingDependencyIndexes = new Set(); + // Snapshot of each collection dependency captured at the last flush. We diff the current snapshot // against it to reconstruct the changed-member delta, instead of relying on Onyx's sourceValue. const lastFlushedCollectionValues = new Array>(totalConnections);