diff --git a/README.md b/README.md index f82919919..7eaeb2783 100644 --- a/README.md +++ b/README.md @@ -316,11 +316,11 @@ This will fire the callback once per member key depending on how many collection Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, waitForCollectionCallback: true, - callback: (allReports) => {...}, + 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. +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. ### Performance Considerations When Using Collections diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index bf50a487d..1835df8f5 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -3,10 +3,10 @@ import * as Logger from './Logger'; import type {ConnectOptions} from './Onyx'; import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; -import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; +import type {CollectionConnectCallback, DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; import utils from './utils'; -type ConnectCallback = DefaultConnectCallback; +type ConnectCallback = DefaultConnectCallback | CollectionConnectCallback; /** * Represents the connection's metadata that contains the necessary properties @@ -42,6 +42,16 @@ type ConnectionMetadata = { * The last callback key returned by `OnyxUtils.subscribeToKey()`'s callback. */ cachedCallbackKey?: OnyxKey; + + /** + * The value that triggered the last update + */ + sourceValue?: OnyxValue; + + /** + * Whether the subscriber is waiting for the collection callback to be fired. + */ + waitForCollectionCallback?: boolean; }; /** @@ -135,7 +145,11 @@ class OnyxConnectionManager { const connection = this.connectionsMap.get(connectionID); connection?.callbacks.forEach((callback) => { - callback(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); + 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); + } }); } @@ -159,7 +173,7 @@ class OnyxConnectionManager { // If the subscriber is a `withOnyx` HOC we don't define `callback` as the HOC will use // its own logic to handle the data. if (!utils.hasWithOnyxInstance(connectOptions)) { - callback = (value, key) => { + callback = (value, key, sourceValue) => { const createdConnection = this.connectionsMap.get(connectionID); if (createdConnection) { // We signal that the first connection was made and now any new subscribers @@ -167,15 +181,15 @@ class OnyxConnectionManager { createdConnection.isConnectionMade = true; createdConnection.cachedCallbackValue = value; createdConnection.cachedCallbackKey = key; - + createdConnection.sourceValue = sourceValue; this.fireCallbacks(connectionID); } }; } subscriptionID = OnyxUtils.subscribeToKey({ - ...(connectOptions as DefaultConnectOptions), - callback, + ...connectOptions, + callback: callback as DefaultConnectCallback, }); connectionMetadata = { @@ -183,6 +197,7 @@ class OnyxConnectionManager { onyxKey: connectOptions.key, isConnectionMade: false, callbacks: new Map(), + waitForCollectionCallback: connectOptions.waitForCollectionCallback, }; this.connectionsMap.set(connectionID, connectionMetadata); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index de1c4eada..5ec04ecb4 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -695,7 +695,7 @@ function keysChanged( // send the whole cached collection. if (isSubscribedToCollectionKey) { if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key); + subscriber.callback(cachedCollection, subscriber.key, partialCollection); continue; } @@ -905,7 +905,7 @@ function keyChanged( } cachedCollection[key] = value; - subscriber.callback(cachedCollection, subscriber.key); + subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); continue; } diff --git a/lib/types.ts b/lib/types.ts index b722fa41f..c492d869a 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -278,7 +278,7 @@ type BaseConnectOptions = { type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; /** Represents the callback function used in `Onyx.connect()` method with a collection key. */ -type CollectionConnectCallback = (value: NonUndefined>, key: TKey) => void; +type CollectionConnectCallback = (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! diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 91ab0da0f..7715b3beb 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -164,7 +164,7 @@ describe('OnyxConnectionManager', () => { expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith(collection, undefined); + expect(callback2).toHaveBeenCalledWith(collection, undefined, undefined); connectionManager.disconnect(connection1); connectionManager.disconnect(connection2); @@ -544,4 +544,71 @@ describe('OnyxConnectionManager', () => { }).not.toThrow(); }); }); + + describe('sourceValue parameter', () => { + it('should pass the sourceValue parameter to collection callbacks when waitForCollectionCallback is true', async () => { + const obj1 = {id: 'entry1_id', name: 'entry1_name'}; + const obj2 = {id: 'entry2_id', name: 'entry2_name'}; + + const callback = jest.fn(); + const connection = connectionManager.connect({ + key: ONYXKEYS.COLLECTION.TEST_KEY, + callback, + waitForCollectionCallback: true, + }); + + await act(async () => waitForPromisesToResolve()); + + // Initial callback with undefined values + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith(undefined, undefined, undefined); + + // Reset mock to test the next update + callback.mockReset(); + + // Update with first object + await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith({[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}, ONYXKEYS.COLLECTION.TEST_KEY, {[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}); + + // Reset mock to test the next update + callback.mockReset(); + + // Update with second object + await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, obj2); + + expect(callback).toHaveBeenCalledTimes(1); + expect(callback).toHaveBeenCalledWith( + { + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2, + }, + ONYXKEYS.COLLECTION.TEST_KEY, + {[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2}, + ); + + connectionManager.disconnect(connection); + }); + + it('should not pass sourceValue to regular callbacks when waitForCollectionCallback is false', async () => { + const obj1 = {id: 'entry1_id', name: 'entry1_name'}; + + const callback = jest.fn(); + const connection = connectionManager.connect({ + key: ONYXKEYS.COLLECTION.TEST_KEY, + callback, + waitForCollectionCallback: false, + }); + + await act(async () => waitForPromisesToResolve()); + + // Update with object + await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1); + + expect(callback).toHaveBeenCalledWith(obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); + + connectionManager.disconnect(connection); + }); + }); }); diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index d2894db69..3e6388364 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -161,7 +161,7 @@ describe('Set data while storage is clearing', () => { expect(collectionCallback).toHaveBeenCalledTimes(3); // And it should be called with the expected parameters each time - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined, undefined); expect(collectionCallback).toHaveBeenNthCalledWith( 2, { @@ -171,8 +171,19 @@ describe('Set data while storage is clearing', () => { test_4: 4, }, ONYX_KEYS.COLLECTION.TEST, + { + test_1: 1, + test_2: 2, + test_3: 3, + test_4: 4, + }, ); - expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST); + expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST, { + test_1: undefined, + test_2: undefined, + test_3: undefined, + test_4: undefined, + }); }) ); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 44400e715..a42737d5e 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -989,7 +989,7 @@ describe('Onyx', () => { .then(() => { // Then we expect the callback to be called only once and the initial stored value to be initialCollectionData expect(mockCallback).toHaveBeenCalledTimes(1); - expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, undefined); + expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, undefined, undefined); }); }); @@ -1015,10 +1015,10 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the first call should be null since the collection was not initialized at that point - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate); }) ); }); @@ -1073,7 +1073,10 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, { + [`${ONYX_KEYS.COLLECTION.TEST_POLICY}1`]: collectionUpdate.testPolicy_1, + }); }) ); }); @@ -1108,7 +1111,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, {testPolicy_1: collectionUpdate.testPolicy_1}); }) // When merge is called again with the same collection not modified @@ -1149,7 +1152,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(1); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, {testPolicy_1: collectionUpdate.testPolicy_1}); }) // When merge is called again with the same collection not modified @@ -1186,8 +1189,8 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}}}, ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE, {[itemKey]: {a: 'a'}}); expect(testCallback).toHaveBeenCalledTimes(2); expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); @@ -1426,7 +1429,9 @@ describe('Onyx', () => { }) .then(() => { expect(collectionCallback).toHaveBeenCalledTimes(3); - expect(collectionCallback).toHaveBeenCalledWith(collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS); + expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue}); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[cat]: initialValue}, undefined, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(3, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1); @@ -1558,6 +1563,10 @@ describe('Onyx', () => { }, }, ONYX_KEYS.COLLECTION.ROUTES, + { + [holidayRoute]: {waypoints: {0: 'Bed', 1: 'Home', 2: 'Beach', 3: 'Restaurant', 4: 'Home'}}, + [routineRoute]: {waypoints: {0: 'Bed', 1: 'Home', 2: 'Work', 3: 'Gym'}}, + }, ); connections.map((id) => Onyx.disconnect(id)); @@ -1628,6 +1637,7 @@ describe('Onyx', () => { [cat]: {age: 3, sound: 'meow'}, }, ONYX_KEYS.COLLECTION.ANIMALS, + {[cat]: {age: 3, sound: 'meow'}}, ); expect(animalsCollectionCallback).toHaveBeenNthCalledWith( 2, @@ -1636,6 +1646,7 @@ describe('Onyx', () => { [dog]: {size: 'M', sound: 'woof'}, }, ONYX_KEYS.COLLECTION.ANIMALS, + {[dog]: {size: 'M', sound: 'woof'}}, ); expect(catCallback).toHaveBeenNthCalledWith(1, {age: 3, sound: 'meow'}, cat); @@ -1647,6 +1658,7 @@ describe('Onyx', () => { [lisa]: {age: 21, car: 'SUV'}, }, ONYX_KEYS.COLLECTION.PEOPLE, + {[bob]: {age: 25, car: 'sedan'}, [lisa]: {age: 21, car: 'SUV'}}, ); connections.map((id) => Onyx.disconnect(id));