From 90f77d8c697f0f8bb95f2bd65a9d8f49af5272aa Mon Sep 17 00:00:00 2001 From: Rafe Colton <1058475+rafecolton@users.noreply.github.com> Date: Fri, 20 Mar 2026 13:16:22 -0700 Subject: [PATCH] Revert "Remove macro/micro tasks during subscriber update" --- lib/Onyx.ts | 22 +++-- lib/OnyxMerge/index.native.ts | 5 +- lib/OnyxMerge/index.ts | 5 +- lib/OnyxMerge/types.ts | 1 + lib/OnyxUtils.ts | 131 +++++++++++++++++-------- tests/perf-test/OnyxUtils.perf-test.ts | 63 +++++++++++- tests/unit/collectionHydrationTest.ts | 85 ---------------- tests/unit/onyxTest.ts | 5 +- 8 files changed, 174 insertions(+), 143 deletions(-) delete mode 100644 tests/unit/collectionHydrationTest.ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3da6a4dee..42a3cd57c 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -266,8 +266,9 @@ function merge(key: TKey, changes: OnyxMergeInput): return Promise.resolve(); } - return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue}) => { + return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue, updatePromise}) => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue); + return updatePromise; }); } catch (error) { Logger.logAlert(`An error occurred while applying merge for key: ${key}, Error: ${error}`); @@ -379,6 +380,16 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { keysToBeClearedFromStorage.push(key); } + const updatePromises: Array> = []; + + // Notify the subscribers for each key/value group so they can receive the new values + for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { + updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value)); + } + for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { + updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value.newValues, value.oldValues)); + } + // Exclude RAM-only keys to prevent them from being saved to storage const defaultKeyValuePairs = Object.entries( Object.keys(defaultKeyStates) @@ -397,14 +408,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { .then(() => Storage.multiSet(defaultKeyValuePairs)) .then(() => { DevTools.clearState(keysToPreserve); - - // Notify the subscribers for each key/value group so they can receive the new values - for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { - OnyxUtils.keyChanged(key, value); - } - for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { - OnyxUtils.keysChanged(key, value.newValues, value.oldValues); - } + return Promise.all(updatePromises); }); }) .then(() => undefined); diff --git a/lib/OnyxMerge/index.native.ts b/lib/OnyxMerge/index.native.ts index 5e56bf49a..ec8c242e3 100644 --- a/lib/OnyxMerge/index.native.ts +++ b/lib/OnyxMerge/index.native.ts @@ -26,20 +26,21 @@ const applyMerge: ApplyMerge = , hasChanged); + const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); const shouldSkipStorageOperations = !hasChanged || OnyxUtils.isRamOnlyKey(key); // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. // If the key is marked as RAM-only, it should not be saved nor updated in the storage. if (shouldSkipStorageOperations) { - return Promise.resolve({mergedValue}); + return Promise.resolve({mergedValue, updatePromise}); } // For native platforms we use `mergeItem` that will take advantage of JSON_PATCH and JSON_REPLACE SQL operations to // merge the object in a performant way. return Storage.mergeItem(key, batchedChanges as OnyxValue, replaceNullPatches).then(() => ({ mergedValue, + updatePromise, })); }; diff --git a/lib/OnyxMerge/index.ts b/lib/OnyxMerge/index.ts index ef92293d3..7eac789cb 100644 --- a/lib/OnyxMerge/index.ts +++ b/lib/OnyxMerge/index.ts @@ -18,19 +18,20 @@ const applyMerge: ApplyMerge = , hasChanged); + const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); const shouldSkipStorageOperations = !hasChanged || OnyxUtils.isRamOnlyKey(key); // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. // If the key is marked as RAM-only, it should not be saved nor updated in the storage. if (shouldSkipStorageOperations) { - return Promise.resolve({mergedValue}); + return Promise.resolve({mergedValue, updatePromise}); } // For web platforms we use `setItem` since the object was already merged with its changes before. return Storage.setItem(key, mergedValue as OnyxValue).then(() => ({ mergedValue, + updatePromise, })); }; diff --git a/lib/OnyxMerge/types.ts b/lib/OnyxMerge/types.ts index e53d8ff32..c59b7892a 100644 --- a/lib/OnyxMerge/types.ts +++ b/lib/OnyxMerge/types.ts @@ -2,6 +2,7 @@ import type {OnyxInput, OnyxKey} from '../types'; type ApplyMergeResult = { mergedValue: TValue; + updatePromise: Promise; }; type ApplyMerge = | undefined, TChange extends OnyxInput | null>( diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 1a9b11bb8..f203870da 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1,4 +1,4 @@ -import {deepEqual, shallowEqual} from 'fast-equals'; +import {deepEqual} from 'fast-equals'; import type {ValueOf} from 'type-fest'; import _ from 'underscore'; import DevTools from './DevTools'; @@ -74,6 +74,9 @@ type OnyxMethod = ValueOf; let mergeQueue: Record>> = {}; let mergeQueuePromise: Record> = {}; +// Used to schedule subscriber update to the macro task queue +let nextMacrotaskPromise: Promise | null = null; + // Holds a mapping of all the React components that want their state subscribed to a store key let callbackToStateMapping: Record> = {}; @@ -706,8 +709,6 @@ function keysChanged( // If they are subscribed to the collection key and using waitForCollectionCallback then we'll // send the whole cached collection. if (isSubscribedToCollectionKey) { - lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection); - if (subscriber.waitForCollectionCallback) { subscriber.callback(cachedCollection, subscriber.key, partialCollection); continue; @@ -807,7 +808,6 @@ function keyChanged( } cachedCollection[key] = value; - lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection); subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); continue; } @@ -826,34 +826,24 @@ function keyChanged( /** * Sends the data obtained from the keys to the connection. */ -function sendDataToConnection(mapping: CallbackToStateMapping, matchedKey: TKey | undefined): void { +function sendDataToConnection(mapping: CallbackToStateMapping, value: OnyxValue | null, matchedKey: TKey | undefined): void { // If the mapping no longer exists then we should not send any data. // This means our subscriber was disconnected. if (!callbackToStateMapping[mapping.subscriptionID]) { return; } - // Always read the latest value from cache to avoid stale or duplicate data. - // For collection subscribers with waitForCollectionCallback, read the full collection. - // For individual key subscribers, read just that key's value. - let value: OnyxValue | undefined; - if (isCollectionKey(mapping.key) && mapping.waitForCollectionCallback) { - const collection = getCachedCollection(mapping.key); - value = Object.keys(collection).length > 0 ? (collection as OnyxValue) : undefined; - } else { - value = cache.get(matchedKey ?? mapping.key) as OnyxValue; - } - // For regular callbacks, we never want to pass null values, but always just undefined if a value is not set in cache or storage. - value = value === null ? undefined : value; + const valueToPass = value === null ? undefined : value; const lastValue = lastConnectionCallbackData.get(mapping.subscriptionID); + lastConnectionCallbackData.get(mapping.subscriptionID); // If the value has not changed we do not need to trigger the callback - if (lastConnectionCallbackData.has(mapping.subscriptionID) && shallowEqual(lastValue, value)) { + if (lastConnectionCallbackData.has(mapping.subscriptionID) && valueToPass === lastValue) { return; } - (mapping as DefaultConnectOptions).callback?.(value, matchedKey as TKey); + (mapping as DefaultConnectOptions).callback?.(valueToPass, matchedKey as TKey); } /** @@ -876,17 +866,63 @@ function addKeyToRecentlyAccessedIfNeeded(key: TKey): void * Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. */ function getCollectionDataAndSendAsObject(matchingKeys: CollectionKeyBase[], mapping: CallbackToStateMapping): void { - multiGet(matchingKeys).then(() => { - sendDataToConnection(mapping, mapping.key); + multiGet(matchingKeys).then((dataMap) => { + const data = Object.fromEntries(dataMap.entries()) as OnyxValue; + sendDataToConnection(mapping, data, mapping.key); }); } +/** + * Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress. + * + * @param callback The keyChanged/keysChanged callback + * */ +function prepareSubscriberUpdate(callback: () => void): Promise { + if (!nextMacrotaskPromise) { + nextMacrotaskPromise = new Promise((resolve) => { + setTimeout(() => { + nextMacrotaskPromise = null; + resolve(); + }, 0); + }); + } + return Promise.all([nextMacrotaskPromise, Promise.resolve().then(callback)]).then(); +} + +/** + * Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately). + * + * @example + * scheduleSubscriberUpdate(key, value, subscriber => subscriber.initWithStoredValues === false) + */ +function scheduleSubscriberUpdate( + key: TKey, + value: OnyxValue, + canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, + isProcessingCollectionUpdate = false, +): Promise { + return prepareSubscriberUpdate(() => keyChanged(key, value, canUpdateSubscriber, isProcessingCollectionUpdate)); +} + +/** + * This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections + * so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the + * subscriber callbacks receive the data in a different format than they normally expect and it breaks code. + */ +function scheduleNotifyCollectionSubscribers( + key: TKey, + value: OnyxCollection, + previousValue?: OnyxCollection, +): Promise { + return prepareSubscriberUpdate(() => keysChanged(key, value, previousValue)); +} + /** * Remove a key from Onyx and update the subscribers */ function remove(key: TKey, isProcessingCollectionUpdate?: boolean): Promise { cache.drop(key); - keyChanged(key, undefined as OnyxValue, undefined, isProcessingCollectionUpdate); + scheduleSubscriberUpdate(key, undefined as OnyxValue, undefined, isProcessingCollectionUpdate); if (isRamOnlyKey(key)) { return Promise.resolve(); @@ -957,7 +993,7 @@ function retryOperation(error: Error, on /** * Notifies subscribers and writes current value to cache */ -function broadcastUpdate(key: TKey, value: OnyxValue, hasChanged?: boolean): void { +function broadcastUpdate(key: TKey, value: OnyxValue, hasChanged?: boolean): Promise { // Update subscribers if the cached value has changed, or when the subscriber specifically requires // all updates regardless of value changes (indicated by initWithStoredValues set to false). if (hasChanged) { @@ -966,7 +1002,7 @@ function broadcastUpdate(key: TKey, value: OnyxValue cache.addToAccessedKeys(key); } - keyChanged(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false); + return scheduleSubscriberUpdate(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false).then(() => undefined); } function hasPendingMergeForKey(key: OnyxKey): boolean { @@ -1183,7 +1219,7 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions { - for (const key of matchingKeys) { - sendDataToConnection(mapping, key as TKey); + multiGet(matchingKeys).then((values) => { + for (const [key, val] of values.entries()) { + sendDataToConnection(mapping, val as OnyxValue, key as TKey); } }); return; } // If we are not subscribed to a collection key then there's only a single key to send an update for. - get(mapping.key).then(() => sendDataToConnection(mapping, mapping.key)); + get(mapping.key).then((val) => sendDataToConnection(mapping, val as OnyxValue, mapping.key)); return; } @@ -1369,23 +1405,24 @@ function setWithRetry({key, value, options}: SetParams OnyxUtils.retryOperation(error, setWithRetry, {key, value: valueWithoutNestedNullValues, options}, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues); + return updatePromise; }); } @@ -1419,17 +1456,17 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(newData, true); - for (const [key, value] of keyValuePairsToSet) { + const updatePromises = keyValuePairsToSet.map(([key, value]) => { // When we use multiSet to set a key we want to clear the current delta changes from Onyx.merge that were queued // before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes. if (OnyxUtils.hasPendingMergeForKey(key)) { delete OnyxUtils.getMergeQueue()[key]; } - // Update cache and optimistically inform subscribers + // Update cache and optimistically inform subscribers on the next tick cache.set(key, value); - keyChanged(key, value); - } + return OnyxUtils.scheduleSubscriberUpdate(key, value); + }); const keyValuePairsToStore = keyValuePairsToSet.filter((keyValuePair) => { const [key] = keyValuePair; @@ -1441,7 +1478,9 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom .catch((error) => OnyxUtils.retryOperation(error, multiSetWithRetry, newData, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData); - }); + return Promise.all(updatePromises); + }) + .then(() => undefined); } /** @@ -1502,18 +1541,19 @@ function setCollectionWithRetry({collectionKey, for (const [key, value] of keyValuePairs) cache.set(key, value); - keysChanged(collectionKey, mutableCollection, previousCollection); + const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); // RAM-only keys are not supposed to be saved to storage if (isRamOnlyKey(collectionKey)) { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - return; + return updatePromise; } return Storage.multiSet(keyValuePairs) .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); + return updatePromise; }); }); } @@ -1645,7 +1685,7 @@ function mergeCollectionWithPatches( // and update all subscribers const promiseUpdate = previousCollectionPromise.then((previousCollection) => { cache.merge(finalMergedCollection); - keysChanged(collectionKey, finalMergedCollection, previousCollection); + return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection); }); return Promise.all(promises) @@ -1711,17 +1751,18 @@ function partialSetCollection({collectionKey, co for (const [key, value] of keyValuePairs) cache.set(key, value); - keysChanged(collectionKey, mutableCollection, previousCollection); + const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); if (isRamOnlyKey(collectionKey)) { sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - return; + return updatePromise; } return Storage.multiSet(keyValuePairs) .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt)) .then(() => { sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); + return updatePromise; }); }); } @@ -1769,6 +1810,8 @@ const OnyxUtils = { sendDataToConnection, getCollectionKey, getCollectionDataAndSendAsObject, + scheduleSubscriberUpdate, + scheduleNotifyCollectionSubscribers, remove, reportStorageQuota, retryOperation, @@ -1825,6 +1868,10 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { // @ts-expect-error Reassign sendDataToConnection = decorateWithMetrics(sendDataToConnection, 'OnyxUtils.sendDataToConnection'); // @ts-expect-error Reassign + scheduleSubscriberUpdate = decorateWithMetrics(scheduleSubscriberUpdate, 'OnyxUtils.scheduleSubscriberUpdate'); + // @ts-expect-error Reassign + scheduleNotifyCollectionSubscribers = decorateWithMetrics(scheduleNotifyCollectionSubscribers, 'OnyxUtils.scheduleNotifyCollectionSubscribers'); + // @ts-expect-error Reassign remove = decorateWithMetrics(remove, 'OnyxUtils.remove'); // @ts-expect-error Reassign reportStorageQuota = decorateWithMetrics(reportStorageQuota, 'OnyxUtils.reportStorageQuota'); diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index 8677fc37d..5a00d910e 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -367,6 +367,7 @@ describe('OnyxUtils', () => { subscriptionID, callback: jest.fn(), }, + mockedReportActionsMap, undefined, ), { @@ -430,6 +431,66 @@ describe('OnyxUtils', () => { }); }); + describe('scheduleSubscriberUpdate', () => { + test('10k calls scheduling updates', async () => { + const subscriptionMap = new Map(); + + const changedReportActions = Object.fromEntries( + Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const), + ) as GenericCollection; + + await measureAsyncFunction(() => Promise.all(Object.entries(changedReportActions).map(([key, value]) => OnyxUtils.scheduleSubscriberUpdate(key, value))), { + beforeEach: async () => { + await Onyx.multiSet(mockedReportActionsMap); + for (const key of mockedReportActionsKeys) { + const id = OnyxUtils.subscribeToKey({key, callback: jest.fn(), initWithStoredValues: false}); + subscriptionMap.set(key, id); + } + }, + afterEach: async () => { + for (const key of mockedReportActionsKeys) { + const id = subscriptionMap.get(key); + if (id) { + OnyxUtils.unsubscribeFromKey(id); + } + } + subscriptionMap.clear(); + await clearOnyxAfterEachMeasure(); + }, + }); + }); + }); + + describe('scheduleNotifyCollectionSubscribers', () => { + test('one call with 10k heavy objects to update 10k subscribers', async () => { + const subscriptionMap = new Map(); + + const changedReportActions = Object.fromEntries( + Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const), + ) as GenericCollection; + + await measureAsyncFunction(() => OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, changedReportActions, mockedReportActionsMap), { + beforeEach: async () => { + await Onyx.multiSet(mockedReportActionsMap); + for (const key of mockedReportActionsKeys) { + const id = OnyxUtils.subscribeToKey({key, callback: jest.fn(), initWithStoredValues: false}); + subscriptionMap.set(key, id); + } + }, + afterEach: async () => { + for (const key of mockedReportActionsKeys) { + const id = subscriptionMap.get(key); + if (id) { + OnyxUtils.unsubscribeFromKey(id); + } + } + subscriptionMap.clear(); + await clearOnyxAfterEachMeasure(); + }, + }); + }); + }); + describe('remove', () => { test('10k calls', async () => { await measureAsyncFunction(() => Promise.all(mockedReportActionsKeys.map((key) => OnyxUtils.remove(key))), { @@ -473,7 +534,7 @@ describe('OnyxUtils', () => { const reportAction = mockedReportActionsMap[`${collectionKey}0`]; const changedReportAction = createRandomReportAction(Number(reportAction.reportActionID)); - await measureFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), { + await measureAsyncFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), { beforeEach: async () => { await Onyx.set(key, reportAction); }, diff --git a/tests/unit/collectionHydrationTest.ts b/tests/unit/collectionHydrationTest.ts deleted file mode 100644 index a36f53db1..000000000 --- a/tests/unit/collectionHydrationTest.ts +++ /dev/null @@ -1,85 +0,0 @@ -import StorageMock from '../../lib/storage'; -import Onyx from '../../lib'; -import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; - -const ONYX_KEYS = { - COLLECTION: { - TEST_KEY: 'test_', - }, -}; - -describe('Collection hydration with connect() followed by immediate set()', () => { - beforeEach(async () => { - // ===== Session 1 ===== - // Data is written to persistent storage (simulates a previous app session). - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Test One'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {id: 2, title: 'Test Two'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {id: 3, title: 'Test Three'}); - - // ===== Session 2 ===== - // App restarts. Onyx.init() calls getAllKeys() which populates storageKeys - // with all 3 keys, but their values are NOT read into cache yet. - Onyx.init({keys: ONYX_KEYS}); - }); - - afterEach(() => Onyx.clear()); - - test('waitForCollectionCallback=true should deliver full collection from storage', async () => { - const mockCallback = jest.fn(); - - // A component connects to the collection (starts async hydration via multiGet). - Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, - callback: mockCallback, - }); - - Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Updated Test One'}); - - await waitForPromisesToResolve(); - - // The subscriber should eventually receive ALL collection members. - // The async hydration reads test_2 and test_3 from storage. - const lastCall = mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]; - expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); - - // Verify the updated value is present (not stale) - expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({id: 1, title: 'Updated Test One'}); - }); - - test('waitForCollectionCallback=false should deliver all collection members from storage', async () => { - const mockCallback = jest.fn(); - - // A component connects to the collection (callback fires per key, not batched). - Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: false, - callback: mockCallback, - }); - - Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Updated Test One'}); - - await waitForPromisesToResolve(); - - // With waitForCollectionCallback=false, the callback fires per key individually. - // Collect all keys that were delivered across all calls. - const deliveredKeys = new Set(); - for (const call of mockCallback.mock.calls) { - const [, key] = call; - if (key) { - deliveredKeys.add(key); - } - } - - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); - expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); - - // Verify the updated value is present (not stale) by finding the last call for key 1 - const key1Calls = mockCallback.mock.calls.filter((call) => call[1] === `${ONYX_KEYS.COLLECTION.TEST_KEY}1`); - const lastKey1Value = key1Calls[key1Calls.length - 1][0]; - expect(lastKey1Value).toEqual({id: 1, title: 'Updated Test One'}); - }); -}); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index bfafd0768..390f1a165 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1552,9 +1552,10 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(collectionCallback).toHaveBeenCalledTimes(2); + expect(collectionCallback).toHaveBeenCalledTimes(3); expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue}); - expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(3, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1);