diff --git a/README.md b/README.md index e8f25ac70..56820a456 100644 --- a/README.md +++ b/README.md @@ -257,10 +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: (allReports, collectionKey, sourceValue) => {...}); +Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (allReports, collectionKey) => {...}); ``` -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. +This will fire the callback once with the entire collection initially and later with an updated version of the collection when individual keys update. ### Performance Considerations When Using Collections diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 3be160cc5..1cc245e32 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -43,11 +43,6 @@ type ConnectionMetadata = { * The last callback key returned by `OnyxUtils.subscribeToKey()`'s callback. */ cachedCallbackKey?: OnyxKey; - - /** - * The value that triggered the last update - */ - sourceValue?: OnyxValue; }; /** @@ -138,11 +133,7 @@ class OnyxConnectionManager { for (const callback of connection.callbacks.values()) { try { if (OnyxKeys.isCollectionKey(connection.onyxKey)) { - (callback as CollectionConnectCallback)( - connection.cachedCallbackValue as Record, - connection.cachedCallbackKey as OnyxKey, - connection.sourceValue, - ); + (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey); } else { (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); } @@ -167,7 +158,7 @@ class OnyxConnectionManager { // If there is no connection yet for that connection ID, we create a new one. if (!connectionMetadata) { - const callback: ConnectCallback = (value, key, sourceValue) => { + const callback: ConnectCallback = (value: OnyxValue, key: OnyxKey) => { const createdConnection = this.connectionsMap.get(connectionID); if (createdConnection) { // We signal that the first connection was made and now any new subscribers @@ -175,7 +166,6 @@ class OnyxConnectionManager { createdConnection.isConnectionMade = true; createdConnection.cachedCallbackValue = value; createdConnection.cachedCallbackKey = key; - createdConnection.sourceValue = sourceValue; this.fireCallbacks(connectionID); } }; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 7370ec78b..4dc1ba2ce 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -10,7 +10,6 @@ import StorageCircuitBreaker from './StorageCircuitBreaker'; import Storage from './storage'; import {StorageErrorClass} from './storage/errors'; import type { - CollectionConnectCallback, CollectionKeyBase, ConnectOptions, DeepRecord, @@ -581,7 +580,7 @@ function keysChanged( try { lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - subscriber.callback(cachedCollection, subscriber.key, partialCollection); + subscriber.callback(cachedCollection, subscriber.key); } catch (error) { Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); } @@ -676,7 +675,7 @@ function keyChanged( cachedCollections[subscriber.key] = cachedCollection; } lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - (subscriber.callback as CollectionConnectCallback)(cachedCollection, subscriber.key, {[key]: value}); + subscriber.callback(cachedCollection, subscriber.key); continue; } diff --git a/lib/types.ts b/lib/types.ts index 2d5be9acf..f2b5b8014 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -223,14 +223,14 @@ 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, sourceValue?: OnyxValue) => void; +type CollectionConnectCallback = (value: NonUndefined>, key: TKey) => void; /** * 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 + * `(collection, key)`). 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! @@ -239,11 +239,7 @@ type ConnectOptions = BaseConnectOptions & { key: TKey; /** A function that will be called when the Onyx data we are subscribed changes. */ - callback?: ( - value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, - key: TKey, - sourceValue?: TKey extends CollectionKeyBase ? NonUndefined> : never, - ) => void; + callback?: (value: TKey extends CollectionKeyBase ? NonUndefined> : OnyxEntry, key: TKey) => void; }; type CallbackToStateMapping = ConnectOptions & { diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 9ece8852c..6d4f8cd25 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -30,12 +30,11 @@ type UseOnyxOptions = { type FetchStatus = 'loading' | 'loaded'; -type ResultMetadata = { +type ResultMetadata = { status: FetchStatus; - sourceValue?: NonNullable | undefined; }; -type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; +type UseOnyxResult = [NonNullable | undefined, ResultMetadata]; function useOnyx>( key: TKey, @@ -117,9 +116,6 @@ function useOnyx>( // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. const shouldGetCachedValueRef = useRef(true); - // Inside useOnyx.ts, we need to track the sourceValue separately - const sourceValueRef = useRef | undefined>(undefined); - // Cache the options key to avoid regenerating it every getSnapshot call const cacheKey = useMemo( () => @@ -226,7 +222,6 @@ function useOnyx>( previousValueRef.current ?? undefined, { status: newFetchStatus, - sourceValue: sourceValueRef.current, }, ]; } @@ -246,7 +241,6 @@ function useOnyx>( if (hasMountedRef.current) { previousValueRef.current = null; newValueRef.current = null; - sourceValueRef.current = undefined; resultRef.current = [undefined, {status: 'loading'}]; shouldGetCachedValueRef.current = true; } @@ -257,7 +251,7 @@ function useOnyx>( connectionRef.current = connectionManager.connect({ key, - callback: (value, callbackKey, sourceValue) => { + callback: () => { isConnectingRef.current = false; onStoreChangeFnRef.current = onStoreChange; @@ -268,9 +262,6 @@ function useOnyx>( // Signals that we want to get the newest cached value again in `getSnapshot()`. shouldGetCachedValueRef.current = true; - // sourceValue is unknown type, so we need to cast it to the correct type. - sourceValueRef.current = sourceValue as NonNullable; - // Invalidate snapshot cache for this key when data changes onyxSnapshotCache.invalidateForKey(key); diff --git a/tests/perf-test/OnyxSnapshotCache.perf-test.ts b/tests/perf-test/OnyxSnapshotCache.perf-test.ts index 151ce4668..02f8ed65d 100644 --- a/tests/perf-test/OnyxSnapshotCache.perf-test.ts +++ b/tests/perf-test/OnyxSnapshotCache.perf-test.ts @@ -44,18 +44,9 @@ const complexSelectorOptions: UseOnyxOptions = { }; // Mock results -const mockResult: UseOnyxResult = [ - {id: 1, name: 'Test', value: 42}, - {status: 'loaded', sourceValue: {id: 1, name: 'Test', value: 42}}, -]; +const mockResult: UseOnyxResult = [{id: 1, name: 'Test', value: 42}, {status: 'loaded'}]; -const mockResults = Array.from( - {length: 1000}, - (_, i): UseOnyxResult => [ - {id: i, name: `Test${i}`, value: i * 10}, - {status: 'loaded', sourceValue: {id: i, name: `Test${i}`, value: i * 10}}, - ], -); +const mockResults = Array.from({length: 1000}, (_, i): UseOnyxResult => [{id: i, name: `Test${i}`, value: i * 10}, {status: 'loaded'}]); describe('OnyxSnapshotCache', () => { let cache: OnyxSnapshotCache; @@ -153,10 +144,7 @@ describe('OnyxSnapshotCache', () => { test('getting cached result with complex selector (cache hit)', async () => { const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, complexSelectorOptions); - const complexResult: UseOnyxResult = [ - {id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}, - {status: 'loaded', sourceValue: {id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}}, - ]; + const complexResult: UseOnyxResult = [{id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}, {status: 'loaded'}]; await measureFunction( () => { cache.getCachedResult(ONYXKEYS.TEST_KEY, cacheKey); diff --git a/tests/perf-test/useOnyx.perf-test.tsx b/tests/perf-test/useOnyx.perf-test.tsx index c183beb81..ce5488567 100644 --- a/tests/perf-test/useOnyx.perf-test.tsx +++ b/tests/perf-test/useOnyx.perf-test.tsx @@ -20,7 +20,7 @@ const metadataStatusMatcher = (onyxKey: OnyxKey, expected: FetchStatus) => `meta type UseOnyxMatcherProps = { onyxKey: OnyxKey; data: OnyxValue; - metadata: ResultMetadata>; + metadata: ResultMetadata; }; function UseOnyxMatcher({onyxKey, data, metadata}: UseOnyxMatcherProps) { diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index d504353e3..664c96f28 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -135,8 +135,8 @@ describe('OnyxConnectionManager', () => { await act(async () => waitForPromisesToResolve()); // 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); + expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); + expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); connectionManager.disconnect(connection1); connectionManager.disconnect(connection2); @@ -232,7 +232,7 @@ describe('OnyxConnectionManager', () => { await act(async () => waitForPromisesToResolve()); - expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); + expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); const callback2 = jest.fn(); const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2}); @@ -402,8 +402,8 @@ describe('OnyxConnectionManager', () => { }); }); - describe('sourceValue parameter', () => { - it('should pass the sourceValue parameter to collection-root callbacks', async () => { + describe('collection callback arguments', () => { + it('should call collection-root callbacks with only the value and key', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const obj2 = {id: 'entry2_id', name: 'entry2_name'}; @@ -417,7 +417,7 @@ describe('OnyxConnectionManager', () => { // Initial callback with undefined values expect(callback).toHaveBeenCalledTimes(1); - expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY, undefined); + expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY); // Reset mock to test the next update callback.mockReset(); @@ -426,7 +426,7 @@ describe('OnyxConnectionManager', () => { 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}); + expect(callback).toHaveBeenCalledWith({[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}, ONYXKEYS.COLLECTION.TEST_KEY); // Reset mock to test the next update callback.mockReset(); @@ -441,13 +441,12 @@ describe('OnyxConnectionManager', () => { [`${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 (non-collection) key callbacks', async () => { + it('should call regular (non-collection) key callbacks with only the value and key', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const callback = jest.fn(); diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index a6a3962c7..bd699b2df 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -239,7 +239,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, ONYX_KEYS.COLLECTION.TEST, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST); expect(collectionCallback).toHaveBeenNthCalledWith( 2, { @@ -249,19 +249,8 @@ 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, { - test_1: undefined, - test_2: undefined, - test_3: undefined, - test_4: undefined, - }); + expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST); }) ); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index daa822176..2ea8c6c6c 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -711,7 +711,6 @@ describe('Onyx', () => { 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, @@ -724,7 +723,6 @@ describe('Onyx', () => { test_5: {ID: 567, value: 'one'}, }, ONYX_KEYS.COLLECTION.TEST_KEY, - expect.anything(), ); }); }); @@ -1050,7 +1048,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, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, undefined); + expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION); }); }); @@ -1075,10 +1073,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, ONYX_KEYS.COLLECTION.TEST_POLICY, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY); // 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, collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1132,10 +1130,8 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY, undefined); - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, { - [`${ONYX_KEYS.COLLECTION.TEST_POLICY}1`]: collectionUpdate.testPolicy_1, - }); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1169,7 +1165,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, {testPolicy_1: collectionUpdate.testPolicy_1}); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) // When merge is called again with the same collection not modified @@ -1206,8 +1202,8 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}} as GenericCollection}, ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_UPDATE, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE, {[itemKey]: {a: 'a'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_UPDATE); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE); expect(testCallback).toHaveBeenCalledTimes(2); expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); @@ -1463,8 +1459,8 @@ describe('Onyx', () => { }) .then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue}); - expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS); + expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1); @@ -1645,10 +1641,6 @@ 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)); @@ -1717,7 +1709,6 @@ describe('Onyx', () => { [cat]: {age: 3, sound: 'meow'}, }, ONYX_KEYS.COLLECTION.ANIMALS, - {[cat]: {age: 3, sound: 'meow'}}, ); expect(animalsCollectionCallback).toHaveBeenNthCalledWith( 2, @@ -1726,7 +1717,6 @@ 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); @@ -1738,7 +1728,6 @@ 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)); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index ac7630a1f..93899cfdc 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -549,11 +549,10 @@ describe('OnyxUtils', () => { OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[entryKey]: entryData}, {}); expect(collectionCallback).toHaveBeenCalledTimes(1); - // Collection subscriber receives the full cached collection, subscriber.key, and partial - const [receivedCollection, receivedKey, receivedPartial] = collectionCallback.mock.calls[0]; + // Collection subscriber receives the full cached collection and subscriber.key + const [receivedCollection, receivedKey] = collectionCallback.mock.calls[0]; expect(receivedKey).toBe(ONYXKEYS.COLLECTION.TEST_KEY); expect(receivedCollection[entryKey]).toEqual(entryData); - expect(receivedPartial).toEqual({[entryKey]: entryData}); Onyx.disconnect(connection); });