diff --git a/cspell.json b/cspell.json index 9fa7cb172c5d..027bbeed6f41 100644 --- a/cspell.json +++ b/cspell.json @@ -1047,7 +1047,9 @@ "Prefetch", "Prefetcher", "knip", - "lottiefiles" + "lottiefiles", + "macrotask", + "Macrotask" ], "ignorePaths": [ ".gitignore", diff --git a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts index 8536d8394e70..b1844c24d5ab 100644 --- a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts +++ b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts @@ -117,17 +117,21 @@ 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); - 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 = triggeredKeys?.size === 1; + if (!displayNamesChanged && personalDetailsIsOnlyTrigger) { return currentValue ?? {reports: {}, locale: null}; } } else if (!sourceValues) { @@ -139,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/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..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; } @@ -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 = triggeredKeys?.size === 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/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts index d3512c5415da..ca310ec14fc9 100644 --- a/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts +++ b/src/libs/actions/OnyxDerived/configs/visibleReportActions.ts @@ -6,25 +6,6 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {ReportAction, ReportActions} from '@src/types/onyx'; import type {VisibleReportActionsDerivedValue} from '@src/types/onyx/DerivedValues'; -import type {OnyxEntry} from 'react-native-onyx'; - -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. @@ -33,6 +14,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 @@ -40,89 +47,33 @@ 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]; - - // 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). 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; - 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 0ce1169a8b68..be8f4ed83d98 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -1,11 +1,16 @@ +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'; import IntlStore from '@src/languages/IntlStore'; +import type {OnyxKey} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; import ObjectUtils from '@src/types/utils/ObjectUtils'; +import type {OnyxCollection} from 'react-native-onyx'; + /** * This file contains logic for derived Onyx keys. The idea behind derived keys is that if there is a common computation * that we're doing in many places across the app to derive some value from multiple Onyx values, we can move that @@ -65,23 +70,21 @@ 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); - } + // Coalesce per-dependency recomputes from one logical change into a single compute on the next macrotask. + let flushScheduled = false; - // 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; - } + // 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, triggeredKeys: Set) => { context.currentValue = derivedValue; - context.sourceValues = sourceKey && sourceValue !== undefined ? {[sourceKey]: sourceValue} : undefined; + context.sourceValues = sourceValues as typeof context.sourceValues; + context.triggeredKeys = triggeredKeys; const spanId = `${CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE}_${key}`; startSpan(spanId, { @@ -102,6 +105,101 @@ 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 = () => { + flushScheduled = false; + + // 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]> = []; + + // 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]; + if (OnyxKeys.isCollectionKey(dependencyOnyxKey)) { + const currentValue = readCollectionDependency(index); + const delta = getCollectionDelta(currentValue, lastFlushedCollectionValues.at(index)); + stagedBaselines.push([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 { + // 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])) { + stagedBaselines.push([index, readCollectionDependency(index)]); + } + } + } + + try { + 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. + 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(); + }; + + const recomputeDerivedValue = (triggeredByIndex: number) => { + // If this recompute was triggered by a connection callback, check if it initializes the connection. + if (!areAllConnectionsSet) { + 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; + } + + pendingDependencyIndexes.add(triggeredByIndex); + if (flushScheduled) { + return; + } + flushScheduled = true; + scheduleMacrotask(flushRecompute); + }; + for (let i = 0; i < dependencies.length; i++) { const dependencyIndex = i; const dependencyOnyxKey = dependencies[dependencyIndex]; @@ -109,10 +207,10 @@ function init() { if (OnyxKeys.isCollectionKey(dependencyOnyxKey)) { Onyx.connectWithoutView({ key: dependencyOnyxKey, - 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) { @@ -132,7 +230,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 { @@ -141,8 +239,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/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/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/src/libs/scheduleMacrotask.ts b/src/libs/scheduleMacrotask.ts new file mode 100644 index 000000000000..e85ad45c4aba --- /dev/null +++ b/src/libs/scheduleMacrotask.ts @@ -0,0 +1,55 @@ +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 + * 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(() => runTask(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) { + runTask(task); + } + }; + + return (callback) => { + queue.push(callback); + if (queue.length === 1) { + channel.port2.postMessage(null); + } + }; +})(); + +export default scheduleMacrotask; +export type {ScheduleMacrotask}; diff --git a/tests/ui/components/HeaderViewTest.tsx b/tests/ui/components/HeaderViewTest.tsx index 95a7744818e4..b3d356b9705f 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 ComposeProviders from '@components/ComposeProviders'; import {LocaleContextProvider} from '@components/LocaleContextProvider'; @@ -99,7 +99,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'; @@ -112,7 +114,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/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/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); + }); +}); diff --git a/tests/unit/OnyxDerivedTest.tsx b/tests/unit/OnyxDerivedTest.tsx index 13d2e034fd72..89c51aa04433 100644 --- a/tests/unit/OnyxDerivedTest.tsx +++ b/tests/unit/OnyxDerivedTest.tsx @@ -1,14 +1,14 @@ 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'; -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} from 'react-native-onyx'; +import type {OnyxCollection, OnyxUpdate} from 'react-native-onyx'; /* eslint-disable @typescript-eslint/naming-convention */ import Onyx from 'react-native-onyx'; @@ -17,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 = () => { @@ -108,6 +109,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); @@ -116,6 +119,63 @@ 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}`]: {isOptimisticReport: 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}`, {isOptimisticReport: 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 @@ -408,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 diff --git a/tests/unit/SidebarOrderTest.ts b/tests/unit/SidebarOrderTest.ts index ca00cde5bd25..a93c91f99bef 100644 --- a/tests/unit/SidebarOrderTest.ts +++ b/tests/unit/SidebarOrderTest.ts @@ -20,6 +20,7 @@ import Onyx from 'react-native-onyx'; 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 @@ -514,9 +515,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, @@ -525,6 +524,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(() => { @@ -617,9 +619,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, @@ -629,6 +629,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(() => { @@ -873,9 +876,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, @@ -885,6 +886,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 @@ -931,9 +935,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, @@ -942,6 +944,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(() => { @@ -955,6 +960,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(() => { @@ -993,9 +999,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, @@ -1005,6 +1009,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(() => { @@ -1025,6 +1032,7 @@ describe('Sidebar', () => { ...reportCollectionDataSet, }), ) + .then(() => waitForBatchedUpdatesWithAct()) // Then they are still in alphabetical order .then(() => { @@ -1088,9 +1096,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, @@ -1101,6 +1107,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(() => { @@ -1157,9 +1166,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, @@ -1168,6 +1175,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(() => { @@ -1198,9 +1208,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, @@ -1208,6 +1216,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(() => { @@ -1221,6 +1232,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(() => { @@ -1259,9 +1271,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, @@ -1272,6 +1282,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..3e939bf6820c --- /dev/null +++ b/tests/unit/getCollectionDeltaTest.ts @@ -0,0 +1,88 @@ +import getCollectionDelta from '@libs/getCollectionDelta'; + +import type {OnyxCollection} from 'react-native-onyx'; + +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/reportAttributesTest.ts b/tests/unit/reportAttributesTest.ts index dc23784f3177..132fbf7b3a18 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, Transaction} from '@src/types/onyx'; import type {OnyxCollection} from 'react-native-onyx'; @@ -186,6 +187,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'); @@ -197,6 +199,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; @@ -219,6 +222,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 @@ -232,6 +236,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; @@ -251,6 +256,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 diff --git a/tests/unit/scheduleMacrotaskTest.ts b/tests/unit/scheduleMacrotaskTest.ts new file mode 100644 index 000000000000..b59097689113 --- /dev/null +++ b/tests/unit/scheduleMacrotaskTest.ts @@ -0,0 +1,185 @@ +/** + * 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'; + +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> = []; +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; + mockLogAlert.mockClear(); + }); + + 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('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[] = []; + + 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']); + }); + }); +});