From 01704211bf9363641843f2ecb826bc0633fdce07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Jun 2026 15:18:54 +0100 Subject: [PATCH] Remove the waitForCollectionCallback connect option Collection-root subscriptions (via Onyx.connect or useOnyx) now always deliver the whole collection snapshot to the callback. The legacy per-member delivery mode for collection-root subscribers is removed; consumers needing per-member processing subscribe to a single member key. Collapses the DefaultConnectOptions/CollectionConnectOptions discriminated union into one ConnectOptions shape (callback typed on TKey), simplifies generateConnectionID and the keyChanged/keysChanged dispatch, and drops the option from useOnyx's internal connect. Per-member-mode tests were migrated to assert snapshot delivery rather than deleted. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/Onyx.ts | 2 - lib/OnyxConnectionManager.ts | 29 ++--- lib/OnyxUtils.ts | 67 +++------- lib/types.ts | 61 ++++----- lib/useOnyx.ts | 2 - tests/perf-test/OnyxUtils.perf-test.ts | 1 - tests/unit/OnyxConnectionManagerTest.ts | 65 +++++----- tests/unit/collectionHydrationTest.ts | 75 +---------- tests/unit/onyxClearWebStorageTest.ts | 1 - tests/unit/onyxTest.ts | 158 ++++++++++-------------- tests/unit/onyxUtilsTest.ts | 37 ++---- 11 files changed, 159 insertions(+), 339 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a8f8f2d4f..16a0f6ec6 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -94,7 +94,6 @@ function init({ * @param connectOptions The options object that will define the behavior of the connection. * @param connectOptions.key The Onyx key to subscribe to. * @param connectOptions.callback A function that will be called when the Onyx data we are subscribed changes. - * @param connectOptions.waitForCollectionCallback If set to `true`, it will return the entire collection to the callback as a single object. * @param connectOptions.selector This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally @@ -119,7 +118,6 @@ function connect(connectOptions: ConnectOptions): Co * @param connectOptions The options object that will define the behavior of the connection. * @param connectOptions.key The Onyx key to subscribe to. * @param connectOptions.callback A function that will be called when the Onyx data we are subscribed changes. - * @param connectOptions.waitForCollectionCallback If set to `true`, it will return the entire collection to the callback as a single object. * @param connectOptions.selector This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook.** * Using this setting on `useOnyx()` can have very positive performance benefits because the component will only re-render * when the subset of data changes. Otherwise, any change of data on any property would normally diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 5b0f32d01..ca9d601a2 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -4,7 +4,7 @@ import type {ConnectOptions} from './Onyx'; import OnyxUtils from './OnyxUtils'; import OnyxKeys from './OnyxKeys'; import * as Str from './Str'; -import type {CollectionConnectCallback, DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; +import type {CollectionConnectCallback, DefaultConnectCallback, OnyxKey, OnyxValue} from './types'; import onyxSnapshotCache from './OnyxSnapshotCache'; type ConnectCallback = DefaultConnectCallback | CollectionConnectCallback; @@ -48,11 +48,6 @@ type ConnectionMetadata = { * The value that triggered the last update */ sourceValue?: OnyxValue; - - /** - * Whether the subscriber is waiting for the collection callback to be fired. - */ - waitForCollectionCallback?: boolean; }; /** @@ -115,21 +110,20 @@ class OnyxConnectionManager { * according to their purpose and effect they produce in the Onyx connection. */ private generateConnectionID(connectOptions: ConnectOptions): string { - const {key, reuseConnection, waitForCollectionCallback} = connectOptions; + const {key, reuseConnection} = connectOptions; // The current session ID is appended to the connection ID so we can have different connections // after an `Onyx.clear()` operation. let suffix = `,sessionID=${this.sessionID}`; - // We will generate a unique ID in any of the following situations: - // - `reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused. - // - `key` is a collection key AND `waitForCollectionCallback` is `undefined/false`. This combination needs a new connection at every subscription - // in order to send all the collection entries, so the connection can't be reused. - if (reuseConnection === false || (OnyxKeys.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false))) { + // We will generate a unique ID when `reuseConnection` is `false`, which means the subscriber + // explicitly wants the connection to not be reused. Collection-root subscriptions are now always + // snapshot mode, so they can be reused like any other connection. + if (reuseConnection === false) { suffix += `,uniqueID=${Str.guid()}`; } - return `onyxKey=${key},waitForCollectionCallback=${waitForCollectionCallback ?? false}${suffix}`; + return `onyxKey=${key}${suffix}`; } /** @@ -142,7 +136,7 @@ class OnyxConnectionManager { } for (const callback of connection.callbacks.values()) { - if (connection.waitForCollectionCallback) { + if (OnyxKeys.isCollectionKey(connection.onyxKey)) { (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey, connection.sourceValue); } else { (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); @@ -180,15 +174,14 @@ class OnyxConnectionManager { subscriptionID = OnyxUtils.subscribeToKey({ ...connectOptions, - callback: callback as DefaultConnectCallback, - }); + callback, + } as ConnectOptions); connectionMetadata = { subscriptionID, onyxKey: connectOptions.key, isConnectionMade: false, callbacks: new Map(), - waitForCollectionCallback: connectOptions.waitForCollectionCallback, }; this.connectionsMap.set(connectionID, connectionMetadata); @@ -204,7 +197,7 @@ class OnyxConnectionManager { // Defer the callback execution to the next tick of the event loop. // This ensures that the current execution flow completes and the result connection object is available when the callback fires. Promise.resolve().then(() => { - (connectOptions as DefaultConnectOptions).callback?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey); + (connectOptions.callback as DefaultConnectCallback | undefined)?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey); }); } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index c88b2e170..e78edb5b1 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -9,11 +9,11 @@ import OnyxKeys from './OnyxKeys'; import * as Str from './Str'; import Storage from './storage'; import type { + CollectionConnectCallback, CollectionKeyBase, ConnectOptions, DeepRecord, DefaultConnectCallback, - DefaultConnectOptions, KeyValueMapping, CallbackToStateMapping, MultiMergeReplaceNullPatches, @@ -597,28 +597,9 @@ function keysChanged( } try { + // Collection-root subscribers always receive the whole collection snapshot. lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - - if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key, partialCollection); - continue; - } - - // Not using waitForCollectionCallback — notify per changed key. - // Re-check the subscription on each iteration because the callback may - // synchronously disconnect itself (removing it from callbackToStateMapping), - // in which case we must stop firing further callbacks for this subscriber. - for (const dataKey of changedMemberKeys) { - const currentSubscriber = callbackToStateMapping[subID]; - if (!currentSubscriber || typeof currentSubscriber.callback !== 'function') { - break; - } - if (cachedCollection[dataKey] === previousCollection[dataKey]) { - continue; - } - const currentSubscriberCallback = currentSubscriber.callback as DefaultConnectCallback; - currentSubscriberCallback(cachedCollection[dataKey], dataKey as TKey); - } + subscriber.callback(cachedCollection, subscriber.key, partialCollection); } catch (error) { Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); } @@ -696,9 +677,10 @@ function keyChanged( continue; } - if (OnyxKeys.isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) { - // Skip individual key changes for collection callbacks during collection updates - // to prevent duplicate callbacks - the collection update will handle this properly + if (OnyxKeys.isCollectionKey(subscriber.key)) { + // Collection-root subscribers always receive the whole collection snapshot. + // Skip individual key changes during collection updates to prevent duplicate + // callbacks - the collection update will handle this properly. if (isProcessingCollectionUpdate) { continue; } @@ -710,7 +692,7 @@ function keyChanged( cachedCollections[subscriber.key] = cachedCollection; } lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); + (subscriber.callback as CollectionConnectCallback)(cachedCollection, subscriber.key, {[key]: value}); continue; } @@ -741,10 +723,10 @@ function sendDataToConnection(mapping: CallbackToStateMapp } // Always read the latest value from cache to avoid stale or duplicate data. - // For collection subscribers with waitForCollectionCallback, read the full collection. + // For collection-root subscribers, read the full collection (snapshot mode). // For individual key subscribers, read just that key's value. let value: OnyxValue | undefined; - if (OnyxKeys.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback) { + if (OnyxKeys.isCollectionKey(mapping.key)) { const collection = getCachedCollection(mapping.key); value = Object.keys(collection).length > 0 ? (collection as OnyxValue) : undefined; } else { @@ -762,7 +744,7 @@ function sendDataToConnection(mapping: CallbackToStateMapp return; } - (mapping as DefaultConnectOptions).callback?.(value, matchedKey as TKey); + (mapping.callback as DefaultConnectCallback | undefined)?.(value, matchedKey as TKey); } /** @@ -1143,7 +1125,7 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions { - for (const key of matchingKeys) { - sendDataToConnection(mapping, key as TKey); - } - }); + getCollectionDataAndSendAsObject(matchingKeys, mapping); return; } @@ -1416,7 +1387,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom // and subscriber state, matching the original per-key notification semantics. cache.set(key, value); // Skip subscriber notification on retry — already notified on attempt 0. - // waitForCollectionCallback subscribers re-fire on every keyChanged by contract. + // Collection-root subscribers re-fire on every keyChanged by contract. if (!retryAttempt) { keyChanged(key, value); } @@ -1506,7 +1477,7 @@ function setCollectionWithRetry({collectionKey, for (const [key, value] of keyValuePairs) cache.set(key, value); // Skip subscriber notification on retry — already notified on attempt 0. - // waitForCollectionCallback subscribers re-fire on every keysChanged by contract. + // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { keysChanged(collectionKey, mutableCollection, previousCollection); } @@ -1652,7 +1623,7 @@ function mergeCollectionWithPatches( const previousCollection = getCachedCollection(collectionKey, existingKeys); cache.merge(finalMergedCollection); // Skip subscriber notification on retry — already notified on attempt 0. - // waitForCollectionCallback subscribers re-fire on every keysChanged by contract. + // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { keysChanged(collectionKey, finalMergedCollection, previousCollection); } @@ -1739,7 +1710,7 @@ function partialSetCollection({collectionKey, co for (const [key, value] of keyValuePairs) cache.set(key, value); // Skip subscriber notification on retry — already notified on attempt 0. - // waitForCollectionCallback subscribers re-fire on every keysChanged by contract. + // Collection-root subscribers re-fire on every keysChanged by contract. if (!retryAttempt) { keysChanged(collectionKey, mutableCollection, previousCollection); } diff --git a/lib/types.ts b/lib/types.ts index 039130df9..aea010d1c 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -225,45 +225,38 @@ type DefaultConnectCallback = (value: OnyxEntry = (value: NonUndefined>, key: TKey, sourceValue?: OnyxValue) => void; -/** Represents the options used in `Onyx.connect()` method with a regular key. */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type DefaultConnectOptions = BaseConnectOptions & { - /** The Onyx key to subscribe to. */ - key: TKey; - - /** A function that will be called when the Onyx data we are subscribed changes. */ - callback?: DefaultConnectCallback; - - /** If set to `true`, it will return the entire collection to the callback as a single object. */ - waitForCollectionCallback?: false; -}; - -/** Represents the options used in `Onyx.connect()` method with a collection key. */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type CollectionConnectOptions = BaseConnectOptions & { - /** The Onyx key to subscribe to. */ - key: TKey extends CollectionKeyBase ? TKey : never; - - /** A function that will be called when the Onyx data we are subscribed changes. */ - callback?: CollectionConnectCallback; - - /** If set to `true`, it will return the entire collection to the callback as a single object. */ - waitForCollectionCallback: true; -}; - /** * Represents the options used in `Onyx.connect()` method. - * The type is built from `DefaultConnectOptions`/`CollectionConnectOptions` depending on the `waitForCollectionCallback` property. - * It includes two different forms, depending on whether we are waiting for a collection callback or not. * - * If `waitForCollectionCallback` is `true`, it expects `key` to be a Onyx collection key and `callback` will be triggered with the whole collection - * and will pass `value` as an `OnyxCollection`. + * For a collection root key (e.g. `ONYXKEYS.COLLECTION.REPORT`), the callback fires + * with the entire collection snapshot whenever any member changes (signature + * `(collection, key)`). For any other key, the callback fires with the value at + * that key (signature `(value, key)`). * - * If `waitForCollectionCallback` is `false` or not specified, the `key` can be any Onyx key and `callback` will be triggered with updates of each collection item - * and will pass `value` as an `OnyxEntry`. + * The legacy `waitForCollectionCallback` flag has been removed — collection-root + * subscriptions always deliver snapshots. Per-member dispatch (the old default + * for collection-root subscribers) is no longer supported; consumers that need + * per-member processing should subscribe to a single collection member key. */ // NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type ConnectOptions = DefaultConnectOptions | CollectionConnectOptions; +type ConnectOptions = BaseConnectOptions & { + /** The Onyx key to subscribe to. */ + key: TKey; + + /** + * A function that will be called when the Onyx data we are subscribed changes. + * + * The value is a conditional *parameter* (collection snapshot vs. entry) inside a single + * function type — rather than a union of two distinct callback types — so that callers using a + * generic or union `TKey` still get an assignable, non-`any` callback. Collection snapshots stay + * `NonUndefined`. + */ + callback?: ( + value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, + key: TKey, + sourceValue?: OnyxValue, + ) => void; +}; type CallbackToStateMapping = ConnectOptions & { subscriptionID: number; @@ -448,14 +441,12 @@ export type { BaseConnectOptions, Collection, CollectionConnectCallback, - CollectionConnectOptions, CollectionKey, CollectionKeyBase, ConnectOptions, CustomTypeOptions, DeepRecord, DefaultConnectCallback, - DefaultConnectOptions, ExtractOnyxCollectionValue, GenericFunction, InitOptions, diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 984c25254..9ece8852c 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -5,7 +5,6 @@ import OnyxCache, {TASK} from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; -import OnyxKeys from './OnyxKeys'; import type {CollectionKeyBase, OnyxKey, OnyxValue} from './types'; import onyxSnapshotCache from './OnyxSnapshotCache'; import useLiveRef from './useLiveRef'; @@ -278,7 +277,6 @@ function useOnyx>( // Finally, we signal that the store changed, making `getSnapshot()` be called again. onStoreChange(); }, - waitForCollectionCallback: OnyxKeys.isCollectionKey(key) as true, reuseConnection: options?.reuseConnection, }); diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index ae0df52e2..87c7dd7cb 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -623,7 +623,6 @@ describe('OnyxUtils', () => { callback: () => { callback.resolve?.(); }, - waitForCollectionCallback: true, }); return callback.promise; }, diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 543dc23c1..184e1d197 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -39,19 +39,19 @@ describe('OnyxConnectionManager', () => { describe('generateConnectionID', () => { it('should generate a stable connection ID', async () => { const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY}); - expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=false,sessionID=${getSessionID()}`); + expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()}`); }); - it("should generate a stable connection ID regardless of the order which the option's properties were passed", async () => { - const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY, waitForCollectionCallback: true}); - expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=true,sessionID=${getSessionID()}`); + it('should generate a stable, reusable connection ID for collection keys', async () => { + const connectionID = generateConnectionID({key: ONYXKEYS.COLLECTION.TEST_KEY}); + expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.COLLECTION.TEST_KEY},sessionID=${getSessionID()}`); }); it('should generate unique connection IDs if certain options are passed', async () => { const connectionID1 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); const connectionID2 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); - expect(connectionID1.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=false,sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); - expect(connectionID2.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},waitForCollectionCallback=false,sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); + expect(connectionID1.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); + expect(connectionID2.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},sessionID=${getSessionID()},uniqueID=`)).toBeTruthy(); expect(connectionID1).not.toEqual(connectionID2); }); @@ -109,7 +109,7 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.size).toEqual(0); }); - it('should connect two times to the same key but with different options, and fire the callbacks differently', async () => { + it('should connect two times to the same collection key, reuse the connection, and fire both callbacks with the whole collection snapshot', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const obj2 = {id: 'entry2_id', name: 'entry2_name'}; const collection = { @@ -125,20 +125,17 @@ describe('OnyxConnectionManager', () => { const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); + const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2}); - expect(connection1.id).not.toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); + // Collection-root connections are now always snapshot mode and are reused. + expect(connection1.id).toEqual(connection2.id); + expect(connectionsMap.size).toEqual(1); expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); - expect(callback1).toHaveBeenCalledTimes(2); - expect(callback1).toHaveBeenNthCalledWith(1, obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - - expect(callback2).toHaveBeenCalledTimes(1); + // Both subscribers share the connection and receive the whole collection snapshot. + expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); connectionManager.disconnect(connection1); @@ -219,7 +216,7 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.has(connection2.id)).toBeTruthy(); }); - it("should create a separate connection to the same key when it's a collection one and waitForCollectionCallback is undefined/false", async () => { + it('should reuse the connection to the same collection key and deliver the whole collection snapshot to all subscribers', async () => { const collection = { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, @@ -231,29 +228,25 @@ describe('OnyxConnectionManager', () => { await act(async () => waitForPromisesToResolve()); const callback1 = jest.fn(); - const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: undefined, callback: callback1}); + const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); await act(async () => waitForPromisesToResolve()); - expect(callback1).toHaveBeenCalledTimes(3); - expect(callback1).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback1).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - expect(callback1).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`); + expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); const callback2 = jest.fn(); - const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: false, callback: callback2}); + const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2}); - expect(connection1.id).not.toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); + // Collection-root connections are now always snapshot mode and are reused. + expect(connection1.id).toEqual(connection2.id); + expect(connectionsMap.size).toEqual(1); expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); - expect(callback2).toHaveBeenCalledTimes(3); - expect(callback2).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); - expect(callback2).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); - expect(callback2).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`); + // The second subscriber reuses the connection and fires immediately with the cached + // snapshot (no sourceValue is forwarded on the immediate fire). + expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); }); it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => { @@ -413,7 +406,7 @@ describe('OnyxConnectionManager', () => { }); describe('sourceValue parameter', () => { - it('should pass the sourceValue parameter to collection callbacks when waitForCollectionCallback is true', async () => { + it('should pass the sourceValue parameter to collection-root callbacks', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const obj2 = {id: 'entry2_id', name: 'entry2_name'}; @@ -421,7 +414,6 @@ describe('OnyxConnectionManager', () => { const connection = connectionManager.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback, - waitForCollectionCallback: true, }); await act(async () => waitForPromisesToResolve()); @@ -458,22 +450,21 @@ describe('OnyxConnectionManager', () => { connectionManager.disconnect(connection); }); - it('should not pass sourceValue to regular callbacks when waitForCollectionCallback is false', async () => { + it('should not pass sourceValue to regular (non-collection) key callbacks', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const callback = jest.fn(); const connection = connectionManager.connect({ - key: ONYXKEYS.COLLECTION.TEST_KEY, + key: ONYXKEYS.TEST_KEY, callback, - waitForCollectionCallback: false, }); await act(async () => waitForPromisesToResolve()); // Update with object - await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1); + await Onyx.merge(ONYXKEYS.TEST_KEY, obj1); - expect(callback).toHaveBeenCalledWith(obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); + expect(callback).toHaveBeenCalledWith(obj1, ONYXKEYS.TEST_KEY); connectionManager.disconnect(connection); }); diff --git a/tests/unit/collectionHydrationTest.ts b/tests/unit/collectionHydrationTest.ts index 64de44412..9a77cc9db 100644 --- a/tests/unit/collectionHydrationTest.ts +++ b/tests/unit/collectionHydrationTest.ts @@ -26,13 +26,12 @@ describe('Collection hydration with connect() followed by immediate set()', () = afterEach(() => Onyx.clear()); - test('waitForCollectionCallback=true should deliver full collection from storage', async () => { + test('collection connect 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, }); @@ -51,43 +50,6 @@ describe('Collection hydration with connect() followed by immediate set()', () = expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({id: 1, title: 'Updated Test One'}); }); - test('waitForCollectionCallback=false should deliver all shallow-equal collection members when set() races with hydration', async () => { - // Clear existing storage and set up shallow-equal values for all members - await StorageMock.clear(); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {status: 'active'}); - await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {status: 'active'}); - // Re-init so Onyx picks up the new storage keys - Onyx.init({keys: ONYX_KEYS}); - - const mockCallback = jest.fn(); - - Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: false, - callback: mockCallback, - }); - - // set() with the same shallow-equal value — this fires keyChanged synchronously, - // populating lastConnectionCallbackData before the hydration multiGet resolves. - Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); - - await waitForPromisesToResolve(); - - const deliveredKeys = new Set(); - for (const call of mockCallback.mock.calls) { - const [, key] = call; - if (key) { - deliveredKeys.add(key); - } - } - - // ALL three members must be delivered, even though their values are shallow-equal. - 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`); - }); - test('single key: set() with non-shallow-equal value should not be overwritten by stale hydration', async () => { const mockCallback = jest.fn(); @@ -111,7 +73,6 @@ describe('Collection hydration with connect() followed by immediate set()', () = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: mockCallback, }); @@ -128,38 +89,4 @@ describe('Collection hydration with connect() followed by immediate set()', () = expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]).toEqual({id: 2, title: 'Test Two'}); expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]).toEqual({id: 3, title: 'Test Three'}); }); - - 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/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index a9629ab3c..a6a3962c7 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -213,7 +213,6 @@ describe('Set data while storage is clearing', () => { const collectionCallback = jest.fn(); const testConnection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST, - waitForCollectionCallback: true, callback: collectionCallback, }); return ( diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 5ff5d6f96..06c8c0297 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -215,7 +215,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -599,7 +598,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, callback: (value) => (result = value), - waitForCollectionCallback: true, }); return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -643,23 +641,22 @@ describe('Onyx', () => { }); }); - it('should overwrite an array key nested inside an object when using merge on a collection', () => { + it('should overwrite an array key nested inside an object', () => { let testKeyValue: unknown; connection = Onyx.connect({ - key: ONYX_KEYS.COLLECTION.TEST_KEY, + key: ONYX_KEYS.TEST_KEY, callback: (value) => { testKeyValue = value; }, }); - Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [1, 2, 3]}}); - return Onyx.merge(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {something: [4]}}).then(() => { - expect(testKeyValue).toEqual({test_1: {something: [4]}}); + Onyx.merge(ONYX_KEYS.TEST_KEY, {something: [1, 2, 3]}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {something: [4]}).then(() => { + expect(testKeyValue).toEqual({something: [4]}); }); }); it('should properly set and merge when using mergeCollection', async () => { - const valuesReceived: Record = {}; const mockCallback = jest.fn(); connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, @@ -668,7 +665,6 @@ describe('Onyx', () => { await waitForPromisesToResolve(); mockCallback.mockReset(); - mockCallback.mockImplementation((data) => (valuesReceived[data.ID] = data.value)); // The first time we call mergeCollection we'll be doing a multiSet internally return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -709,20 +705,27 @@ describe('Onyx', () => { } as GenericCollection), ) .then(() => { - // 3 items on the first mergeCollection + 4 items the next mergeCollection - expect(mockCallback).toHaveBeenCalledTimes(7); - expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); - expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); - expect(valuesReceived[123]).toEqual('five'); - expect(valuesReceived[234]).toEqual('four'); - expect(valuesReceived[345]).toEqual('three'); - expect(valuesReceived[456]).toEqual('two'); - expect(valuesReceived[567]).toEqual('one'); + // Snapshot mode: callback fires once per mergeCollection with the full snapshot. + expect(mockCallback).toHaveBeenCalledTimes(2); + expect(mockCallback).toHaveBeenNthCalledWith( + 1, + {test_1: {ID: 123, value: 'one'}, test_2: {ID: 234, value: 'two'}, test_3: {ID: 345, value: 'three'}}, + ONYX_KEYS.COLLECTION.TEST_KEY, + expect.anything(), + ); + expect(mockCallback).toHaveBeenNthCalledWith( + 2, + { + test_1: {ID: 123, value: 'five'}, + test_2: {ID: 234, value: 'four'}, + // test_3 unchanged (incompatible array merge rejected) + test_3: {ID: 345, value: 'three'}, + test_4: {ID: 456, value: 'two'}, + test_5: {ID: 567, value: 'one'}, + }, + ONYX_KEYS.COLLECTION.TEST_KEY, + expect.anything(), + ); }); }); @@ -739,10 +742,10 @@ describe('Onyx', () => { }); it('should return full object to callback when calling mergeCollection()', () => { - const valuesReceived: Record = {}; + let lastSnapshot: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (snapshot) => (lastSnapshot = snapshot), }); return Onyx.multiSet({ @@ -766,7 +769,7 @@ describe('Onyx', () => { } as GenericCollection), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastSnapshot).toEqual({ test_1: { ID: 123, value: 'one', @@ -789,7 +792,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: mockCallback, }); @@ -833,7 +835,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: mockCallback, }); @@ -912,7 +913,6 @@ describe('Onyx', () => { }); it('should use update data object to merge a collection of keys', () => { - const valuesReceived: Record = {}; const mockCallback = jest.fn(); connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, @@ -922,7 +922,6 @@ describe('Onyx', () => { return waitForPromisesToResolve() .then(() => { mockCallback.mockReset(); - mockCallback.mockImplementation((data) => (valuesReceived[data.ID] = data.value)); // Given the initial Onyx state: {test_1: {existingData: 'test',}, test_2: {existingData: 'test',}} Onyx.multiSet({ @@ -936,8 +935,9 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(mockCallback).toHaveBeenNthCalledWith(1, {existingData: 'test'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(2, {existingData: 'test'}, 'test_2'); + // Snapshot mode: the collection callback receives the whole collection snapshot. + expect(mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]).toEqual({test_1: {existingData: 'test'}, test_2: {existingData: 'test'}}); + mockCallback.mockReset(); // When we pass a mergeCollection data object to Onyx.update return Onyx.update([ @@ -962,36 +962,22 @@ describe('Onyx', () => { ]); }) .then(() => { - /* Then the final Onyx state should be: - { - test_1: { - existingData: 'test' - ID: 123, - value: 'one', - }, - test_2: { - existingData: 'test' - ID: 234, - value: 'two', - }, - test_3: { - ID: 345, - value: 'three', - }, - } - */ - - expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 123, value: 'one', existingData: 'test'}, 'test_1'); - expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 234, value: 'two', existingData: 'test'}, 'test_2'); - expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 345, value: 'three'}, 'test_3'); + // mergeCollection fires the collection snapshot once with all 3 merged members. + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback.mock.calls[0][0]).toEqual({ + test_1: {ID: 123, value: 'one', existingData: 'test'}, + test_2: {ID: 234, value: 'two', existingData: 'test'}, + test_3: {ID: 345, value: 'three'}, + }); + expect(mockCallback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.TEST_KEY); }); }); it('should properly set all keys provided in a multiSet called via update', () => { - const valuesReceived: Record = {}; + let lastSnapshot: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (snapshot) => (lastSnapshot = snapshot), }); return Onyx.multiSet({ @@ -1020,7 +1006,7 @@ describe('Onyx', () => { ] as unknown as Array>), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastSnapshot).toEqual({ test_1: { ID: 123, value: 'one', @@ -1033,7 +1019,7 @@ describe('Onyx', () => { }); }); - it('should return all collection keys as a single object when waitForCollectionCallback = true', () => { + it('should return all collection keys as a single object', () => { const mockCallback = jest.fn(); // Given some initial collection data @@ -1054,10 +1040,9 @@ describe('Onyx', () => { return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, initialCollectionData as GenericCollection) .then(() => { - // When we connect to that collection with waitForCollectionCallback = true + // When we connect to that collection connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, - waitForCollectionCallback: true, callback: mockCallback, }); return waitForPromisesToResolve(); @@ -1069,17 +1054,16 @@ describe('Onyx', () => { }); }); - it('should return all collection keys as a single object when updating a collection key with waitForCollectionCallback = true', () => { + it('should return all collection keys as a single object when updating a collection key', () => { const mockCallback = jest.fn(); const collectionUpdate = { testPolicy_1: {ID: 234, value: 'one'}, testPolicy_2: {ID: 123, value: 'two'}, }; - // Given an Onyx.connect call with waitForCollectionCallback=true + // Given an Onyx.connect call to a collection key connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, - waitForCollectionCallback: true, callback: mockCallback, }); return ( @@ -1106,7 +1090,7 @@ describe('Onyx', () => { testPolicy_2: {ID: 123, value: 'two'}, }; - // Given an Onyx.connect call with waitForCollectionCallback=false + // Given an Onyx.connect call subscribing to a single collection member key connection = Onyx.connect({ key: `${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, callback: mockCallback, @@ -1128,16 +1112,15 @@ describe('Onyx', () => { ); }); - it('should return all collection keys as a single object for subscriber using waitForCollectionCallback when a single collection member key is updated', () => { + it('should return all collection keys as a single object when a single collection member key is updated', () => { const mockCallback = jest.fn(); const collectionUpdate = { testPolicy_1: {ID: 234, value: 'one'}, }; - // Given an Onyx.connect call with waitForCollectionCallback=true + // Given an Onyx.connect call to a collection key connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, - waitForCollectionCallback: true, callback: mockCallback, }); return ( @@ -1172,10 +1155,9 @@ describe('Onyx', () => { testPolicy_1: {ID: 234, value: 'one'}, }; - // Given an Onyx.connect call with waitForCollectionCallback=true + // Given an Onyx.connect call to a collection key connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, - waitForCollectionCallback: true, callback: mockCallback, }); return ( @@ -1216,7 +1198,7 @@ describe('Onyx', () => { connections.push(Onyx.connect({key: ONYX_KEYS.TEST_KEY, callback: testCallback})); connections.push(Onyx.connect({key: ONYX_KEYS.OTHER_TEST, callback: otherTestCallback})); - connections.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST_UPDATE, callback: collectionCallback, waitForCollectionCallback: true})); + connections.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST_UPDATE, callback: collectionCallback})); return waitForPromisesToResolve().then(() => Onyx.update([ {onyxMethod: Onyx.METHOD.SET, key: ONYX_KEYS.TEST_KEY, value: 'taco'}, @@ -1412,7 +1394,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, callback: (value) => (result = value), - waitForCollectionCallback: true, }); return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -1463,7 +1444,6 @@ describe('Onyx', () => { Onyx.connect({ key: ONYX_KEYS.COLLECTION.ANIMALS, callback: collectionCallback, - waitForCollectionCallback: true, }), ); connections.push(Onyx.connect({key: cat, callback: catCallback})); @@ -1517,9 +1497,12 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); + // Snapshot mode: the SNAPSHOT collection-root subscriber receives the whole collection. expect(callback).toBeCalledTimes(2); - expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue}}, snapshot1); - expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: finalValue}}, snapshot1); + expect(callback.mock.calls[0][0]).toEqual({[snapshot1]: {data: {[cat]: initialValue}}}); + expect(callback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback.mock.calls[1][0]).toEqual({[snapshot1]: {data: {[cat]: finalValue}}}); + expect(callback.mock.calls[1][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); }); it('should merge allowlisted keys into Snapshot even if they were missing', async () => { @@ -1548,9 +1531,12 @@ describe('Onyx', () => { await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]); + // Snapshot mode: the SNAPSHOT collection-root subscriber receives the whole collection. expect(callback).toBeCalledTimes(2); - expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue}}, snapshot1); - expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: {name: 'Kitty', pendingAction: 'delete', pendingFields: {preview: 'delete'}}}}, snapshot1); + expect(callback.mock.calls[0][0]).toEqual({[snapshot1]: {data: {[cat]: initialValue}}}); + expect(callback.mock.calls[0][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); + expect(callback.mock.calls[1][0]).toEqual({[snapshot1]: {data: {[cat]: {name: 'Kitty', pendingAction: 'delete', pendingFields: {preview: 'delete'}}}}}); + expect(callback.mock.calls[1][1]).toBe(ONYX_KEYS.COLLECTION.SNAPSHOT); }); describe('update', () => { @@ -1576,7 +1562,6 @@ describe('Onyx', () => { Onyx.connect({ key: ONYX_KEYS.COLLECTION.ROUTES, callback: routesCollectionCallback, - waitForCollectionCallback: true, }), ); @@ -1691,14 +1676,12 @@ describe('Onyx', () => { Onyx.connect({ key: ONYX_KEYS.COLLECTION.ANIMALS, callback: animalsCollectionCallback, - waitForCollectionCallback: true, }), ); connections.push( Onyx.connect({ key: ONYX_KEYS.COLLECTION.PEOPLE, callback: peopleCollectionCallback, - waitForCollectionCallback: true, }), ); connections.push(Onyx.connect({key: cat, callback: catCallback})); @@ -1798,7 +1781,6 @@ describe('Onyx', () => { let result: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, - waitForCollectionCallback: true, callback: (value) => { result = value; }, @@ -1841,7 +1823,6 @@ describe('Onyx', () => { beforeEach(() => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, - waitForCollectionCallback: true, callback: (value) => { result = value; }, @@ -2203,7 +2184,6 @@ describe('Onyx', () => { callback: (value) => { routesCollection = value; }, - waitForCollectionCallback: true, }); return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { @@ -2242,7 +2222,6 @@ describe('Onyx', () => { callback: (value) => { routesCollection = value; }, - waitForCollectionCallback: true, }); let testKeyValue: unknown; @@ -2296,7 +2275,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: collectionCallback, }); @@ -2428,7 +2406,6 @@ describe('Onyx', () => { let result: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, - waitForCollectionCallback: true, callback: (value) => { result = value; }, @@ -2462,7 +2439,6 @@ describe('Onyx', () => { beforeEach(() => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_UPDATE, - waitForCollectionCallback: true, callback: (value) => { result = value; }, @@ -2736,7 +2712,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); // Set initial collection state @@ -2768,7 +2743,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.ROUTES, callback, - waitForCollectionCallback: true, }); await waitForPromisesToResolve(); @@ -2794,7 +2768,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ROUTES, { @@ -2818,7 +2791,6 @@ describe('Onyx', () => { connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.ROUTES, - waitForCollectionCallback: true, callback: mockCallback, }); @@ -2864,7 +2836,6 @@ describe('Onyx', () => { let testKeyValue: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -2882,7 +2853,6 @@ describe('Onyx', () => { let testKeyValue: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -2900,7 +2870,6 @@ describe('Onyx', () => { let testKeyValue: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -2922,7 +2891,6 @@ describe('Onyx', () => { let testKeyValue: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -2944,7 +2912,6 @@ describe('Onyx', () => { let testKeyValue: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - waitForCollectionCallback: true, callback: (value) => { testKeyValue = value; }, @@ -3166,7 +3133,6 @@ describe('RAM-only keys should not read from storage', () => { callback: (value) => { receivedCollection = value; }, - waitForCollectionCallback: true, }); await act(async () => waitForPromisesToResolve()); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 8767dca82..c1e6db7a3 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -115,7 +115,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); // Set initial collection state @@ -151,7 +150,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); await Onyx.mergeCollection(ONYXKEYS.COLLECTION.ROUTES, { @@ -177,7 +175,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); await Onyx.mergeCollection(ONYXKEYS.COLLECTION.ROUTES, { @@ -205,7 +202,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); await waitForPromisesToResolve(); @@ -275,7 +271,6 @@ describe('OnyxUtils', () => { const connCollection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); const connSingle = Onyx.connect({ key: ONYXKEYS.TEST_KEY, @@ -309,12 +304,10 @@ describe('OnyxUtils', () => { const connTest = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: testCallback, - waitForCollectionCallback: true, }); const connRoutes = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: routesCallback, - waitForCollectionCallback: true, }); await waitForPromisesToResolve(); testCallback.mockClear(); @@ -377,15 +370,13 @@ describe('OnyxUtils', () => { Onyx.disconnect(conn2); }); - it('should stop firing callbacks for a collection subscriber that disconnects itself mid-batch', async () => { - // A collection subscriber (waitForCollectionCallback=false) disconnects itself when - // it receives the first member. Subsequent changed members in the same batch must NOT - // trigger further callbacks for this subscriber. + it('should not fire again for a collection subscriber that disconnects itself in its callback', async () => { + // A collection-root subscriber (snapshot mode) disconnects itself when it receives a + // collection snapshot. A subsequent collection change must NOT trigger another callback. const callback = jest.fn(); const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback, - waitForCollectionCallback: false, }); await waitForPromisesToResolve(); callback.mockReset(); @@ -393,13 +384,18 @@ describe('OnyxUtils', () => { Onyx.disconnect(connection); }); + // First batch fires the snapshot callback once, which disconnects the subscriber. await Onyx.multiSet({ [`${ONYXKEYS.COLLECTION.TEST_KEY}1`]: {id: 1}, [`${ONYXKEYS.COLLECTION.TEST_KEY}2`]: {id: 2}, [`${ONYXKEYS.COLLECTION.TEST_KEY}3`]: {id: 3}, }); - // Despite 3 changed members, callback should fire at most once before disconnect stops it + expect(callback).toHaveBeenCalledTimes(1); + + // A subsequent change must not fire the now-disconnected subscriber again. + await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: 11}); + expect(callback).toHaveBeenCalledTimes(1); }); @@ -535,7 +531,7 @@ describe('OnyxUtils', () => { await Onyx.disconnect(connection); }); - it('should notify collection-level subscribers with waitForCollectionCallback', async () => { + it('should notify collection-level subscribers with the whole collection snapshot', async () => { const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}789`; const entryData = {value: 'data'}; @@ -543,7 +539,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); await Onyx.set(entryKey, entryData); @@ -899,7 +894,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -941,7 +935,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -988,7 +981,7 @@ describe('OnyxUtils', () => { // re-enters the failing method on the next attempt. const transientError = new Error('Transient storage error'); - it('mergeCollection — waitForCollectionCallback subscriber fires once across retries', async () => { + it('mergeCollection — collection-root subscriber fires once across retries', async () => { const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; const existingMemberKey = `${collectionKey}1`; const newMemberKey = `${collectionKey}2`; @@ -998,7 +991,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1012,7 +1004,7 @@ describe('OnyxUtils', () => { } as GenericCollection); // Before this fix, every retry attempt re-fired keysChanged() — and - // waitForCollectionCallback subscribers fire on every keysChanged() call by contract. + // Collection-root subscribers fire on every keysChanged() call by contract. // After the fix, retries skip the keysChanged re-fire, so subscribers are notified // exactly once per logical operation. expect(collectionCallback).toHaveBeenCalledTimes(1); @@ -1026,7 +1018,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1050,7 +1041,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1074,7 +1064,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1212,7 +1201,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1280,7 +1268,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve();