diff --git a/API.md b/API.md index 8870f7885..e766e551f 100644 --- a/API.md +++ b/API.md @@ -80,7 +80,6 @@ This method will be deprecated soon. Please use `Onyx.connectWithoutView()` inst | connectOptions | The options object that will define the behavior of the connection. | | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | -| connectOptions.waitForCollectionCallback | If set to `true`, it will return the entire collection to the callback as a single object. | | 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 cause the component to re-render (and that can be expensive from a performance standpoint). | **Example** @@ -103,7 +102,6 @@ Connects to an Onyx key given the options passed and listens to its changes. | connectOptions | The options object that will define the behavior of the connection. | | connectOptions.key | The Onyx key to subscribe to. | | connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | -| connectOptions.waitForCollectionCallback | If set to `true`, it will return the entire collection to the callback as a single object. | | 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 cause the component to re-render (and that can be expensive from a performance standpoint). | **Example** diff --git a/README.md b/README.md index 9d70edc9d..e8f25ac70 100644 --- a/README.md +++ b/README.md @@ -257,20 +257,10 @@ export default MyComponent; This will add a prop to the component called `allReports` which is an object of collection member key/values. Changes to the individual member keys will modify the entire object and new props will be passed with each individual key update. The prop doesn't update on the initial rendering of the component until the entire collection has been read out of Onyx. ```js -Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (memberValue, memberKey) => {...}); +Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (allReports, collectionKey, sourceValue) => {...}); ``` -This will fire the callback once per member key depending on how many collection member keys are currently stored. Changes to those keys after the initial callbacks fire will occur when each individual key is updated. - -```js -Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT, - waitForCollectionCallback: true, - callback: (allReports, collectionKey, sourceValue) => {...}, -}); -``` - -This final option forces `Onyx.connect()` to behave more like `useOnyx()` and only update the callback once with the entire collection initially and later with an updated version of the collection when individual keys update. The `sourceValue` parameter contains only the specific keys and values that triggered the current update, which can be useful for optimizing your code to only process what changed. This parameter is not available when `waitForCollectionCallback` is false or the key is not a collection key. +This will fire the callback once with the entire collection initially and later with an updated version of the collection when individual keys update. The `sourceValue` parameter contains only the specific keys and values that triggered the current update, which can be useful for optimizing your code to only process what changed. ### Performance Considerations When Using Collections 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..3be160cc5 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,10 +136,18 @@ class OnyxConnectionManager { } for (const callback of connection.callbacks.values()) { - if (connection.waitForCollectionCallback) { - (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey, connection.sourceValue); - } else { - (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); + try { + 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); + } + } catch (error) { + Logger.logAlert(`[ConnectionManager] Subscriber callback threw an error for key '${connection.onyxKey}': ${error}`); } } } @@ -180,15 +182,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 +205,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 a8c4e510d..7370ec78b 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -10,11 +10,11 @@ import StorageCircuitBreaker from './StorageCircuitBreaker'; import Storage from './storage'; import {StorageErrorClass} from './storage/errors'; import type { + CollectionConnectCallback, CollectionKeyBase, ConnectOptions, DeepRecord, DefaultConnectCallback, - DefaultConnectOptions, KeyValueMapping, CallbackToStateMapping, MultiMergeReplaceNullPatches, @@ -580,31 +580,8 @@ function keysChanged( } try { - 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); - } + lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); + subscriber.callback(cachedCollection, subscriber.key, partialCollection); } catch (error) { Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); } @@ -685,9 +662,9 @@ 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)) { + // Skip individual key changes during collection updates to prevent duplicate + // callbacks - the collection update will handle this properly. if (isProcessingCollectionUpdate) { continue; } @@ -698,13 +675,8 @@ function keyChanged( cachedCollection = getCachedCollection(subscriber.key); cachedCollections[subscriber.key] = cachedCollection; } - lastConnectionCallbackData.set(subscriber.subscriptionID, { - value: cachedCollection, - matchedKey: subscriber.key, - }); - subscriber.callback(cachedCollection, subscriber.key, { - [key]: value, - }); + lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); + (subscriber.callback as CollectionConnectCallback)(cachedCollection, subscriber.key, {[key]: value}); continue; } @@ -738,10 +710,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. // 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 { @@ -759,7 +731,7 @@ function sendDataToConnection(mapping: CallbackToStateMapp return; } - (mapping as DefaultConnectOptions).callback?.(value, matchedKey as TKey); + (mapping.callback as DefaultConnectCallback | undefined)?.(value, matchedKey as TKey); } /** @@ -1180,7 +1152,7 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions { - for (const key of matchingKeys) { - sendDataToConnection(mapping, key as TKey); - } - }); + getCollectionDataAndSendAsObject(matchingKeys, mapping); return; } @@ -1463,7 +1424,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); } @@ -1554,7 +1515,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); } @@ -1704,7 +1665,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); } @@ -1797,7 +1758,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..2d5be9acf 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -225,46 +225,27 @@ type DefaultConnectCallback = (value: OnyxEntry = (value: NonUndefined>, key: TKey, sourceValue?: OnyxValue) => void; -/** Represents the options used in `Onyx.connect()` method with a regular key. */ +/** + * Represents the options used in `Onyx.connect()` method. + * + * For a collection root key (e.g. `ONYXKEYS.COLLECTION.REPORT`), the callback fires + * with the entire collection object whenever any member changes (signature + * `(collection, key, sourceValue)`). For any other key, the callback fires with the value at + * that key (signature `(value, key)`). + */ // NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type DefaultConnectOptions = BaseConnectOptions & { +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. */ - callback?: DefaultConnectCallback; - - /** If set to `true`, it will return the entire collection to the callback as a single object. */ - waitForCollectionCallback?: false; + callback?: ( + value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, + key: TKey, + sourceValue?: TKey extends CollectionKeyBase ? NonUndefined> : never, + ) => void; }; -/** 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`. - * - * 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`. - */ -// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! -type ConnectOptions = DefaultConnectOptions | CollectionConnectOptions; - type CallbackToStateMapping = ConnectOptions & { subscriptionID: number; }; @@ -448,14 +429,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..d504353e3 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 object', 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 object. + 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 object 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,22 @@ 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); + 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`); + 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 +403,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 +411,6 @@ describe('OnyxConnectionManager', () => { const connection = connectionManager.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback, - waitForCollectionCallback: true, }); await act(async () => waitForPromisesToResolve()); @@ -458,22 +447,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..daa822176 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'); + // Callback fires once per mergeCollection with the full collection object. + 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 lastCollectionObject: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (value) => (lastCollectionObject = value), }); return Onyx.multiSet({ @@ -766,7 +769,7 @@ describe('Onyx', () => { } as GenericCollection), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastCollectionObject).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'); + // The collection callback receives the whole collection object. + 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 object 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 lastCollectionObject: unknown; connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, - callback: (data, key) => (valuesReceived[key] = data), + callback: (value) => (lastCollectionObject = value), }); return Onyx.multiSet({ @@ -1020,7 +1006,7 @@ describe('Onyx', () => { ] as unknown as Array>), ) .then(() => { - expect(valuesReceived).toEqual({ + expect(lastCollectionObject).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}]); + // 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}]); + // 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 a97d685bb..ac7630a1f 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -116,7 +116,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); // Set initial collection state @@ -152,7 +151,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); await Onyx.mergeCollection(ONYXKEYS.COLLECTION.ROUTES, { @@ -178,7 +176,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.ROUTES, callback: (value) => (result = value), - waitForCollectionCallback: true, }); await Onyx.mergeCollection(ONYXKEYS.COLLECTION.ROUTES, { @@ -206,7 +203,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); await waitForPromisesToResolve(); @@ -276,7 +272,6 @@ describe('OnyxUtils', () => { const connCollection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); const connSingle = Onyx.connect({ key: ONYXKEYS.TEST_KEY, @@ -310,12 +305,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(); @@ -378,15 +371,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 disconnects itself when it receives a + // collection object. 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(); @@ -394,13 +385,18 @@ describe('OnyxUtils', () => { Onyx.disconnect(connection); }); + // First batch fires the collection 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); }); @@ -536,7 +532,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 object', async () => { const entryKey = `${ONYXKEYS.COLLECTION.TEST_KEY}789`; const entryData = {value: 'data'}; @@ -544,7 +540,6 @@ describe('OnyxUtils', () => { const connection = Onyx.connect({ key: ONYXKEYS.COLLECTION.TEST_KEY, callback: collectionCallback, - waitForCollectionCallback: true, }); await Onyx.set(entryKey, entryData); @@ -965,7 +960,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1007,7 +1001,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1054,7 +1047,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`; @@ -1064,7 +1057,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1078,7 +1070,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); @@ -1092,7 +1084,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1116,7 +1107,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1140,7 +1130,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1278,7 +1267,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve(); @@ -1346,7 +1334,6 @@ describe('OnyxUtils', () => { const collectionCallback = jest.fn(); Onyx.connect({ key: collectionKey, - waitForCollectionCallback: true, callback: collectionCallback, }); await waitForPromisesToResolve();