From fa7a8d54eb66dfa38035dea3e72a51b1f5de8708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Jun 2026 15:40:20 +0100 Subject: [PATCH] Remove the sourceValue callback parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the optional 3rd `sourceValue` argument from collection connect callbacks and the `sourceValue` field from useOnyx's result metadata. `ResultMetadata` becomes non-generic (`{status}`); `UseOnyxResult` and the perf-test usages follow. Consumers that need "what changed" should diff against a previous value (e.g. usePrevious) — structural sharing makes that O(1) per member. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/OnyxConnectionManager.ts | 10 ++----- lib/OnyxUtils.ts | 4 +-- lib/types.ts | 2 +- lib/useOnyx.ts | 15 ++-------- .../perf-test/OnyxSnapshotCache.perf-test.ts | 18 ++---------- tests/perf-test/useOnyx.perf-test.tsx | 2 +- tests/unit/OnyxConnectionManagerTest.ts | 13 ++++----- tests/unit/onyxClearWebStorageTest.ts | 15 ++-------- tests/unit/onyxTest.ts | 29 +++++++------------ tests/unit/onyxUtilsTest.ts | 5 ++-- 10 files changed, 32 insertions(+), 81 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 5b0f32d01..594d62467 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -44,11 +44,6 @@ type ConnectionMetadata = { */ cachedCallbackKey?: OnyxKey; - /** - * The value that triggered the last update - */ - sourceValue?: OnyxValue; - /** * Whether the subscriber is waiting for the collection callback to be fired. */ @@ -143,7 +138,7 @@ 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); + (callback as CollectionConnectCallback)(connection.cachedCallbackValue as Record, connection.cachedCallbackKey as OnyxKey); } else { (callback as DefaultConnectCallback)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); } @@ -165,7 +160,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 @@ -173,7 +168,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 c88b2e170..bb5f52ac4 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -600,7 +600,7 @@ function keysChanged( lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key, partialCollection); + subscriber.callback(cachedCollection, subscriber.key); continue; } @@ -710,7 +710,7 @@ function keyChanged( cachedCollections[subscriber.key] = cachedCollection; } lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); + subscriber.callback(cachedCollection, subscriber.key); continue; } diff --git a/lib/types.ts b/lib/types.ts index 039130df9..552963132 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -223,7 +223,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, sourceValue?: OnyxValue) => void; +type CollectionConnectCallback = (value: NonUndefined>, key: TKey) => 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/lib/useOnyx.ts b/lib/useOnyx.ts index 984c25254..4ea6e19c3 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -31,12 +31,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, @@ -118,9 +117,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( () => @@ -227,7 +223,6 @@ function useOnyx>( previousValueRef.current ?? undefined, { status: newFetchStatus, - sourceValue: sourceValueRef.current, }, ]; } @@ -247,7 +242,6 @@ function useOnyx>( if (hasMountedRef.current) { previousValueRef.current = null; newValueRef.current = null; - sourceValueRef.current = undefined; resultRef.current = [undefined, {status: 'loading'}]; shouldGetCachedValueRef.current = true; } @@ -258,7 +252,7 @@ function useOnyx>( connectionRef.current = connectionManager.connect({ key, - callback: (value, callbackKey, sourceValue) => { + callback: () => { isConnectingRef.current = false; onStoreChangeFnRef.current = onStoreChange; @@ -269,9 +263,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 543dc23c1..24a64c5f4 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -139,7 +139,7 @@ describe('OnyxConnectionManager', () => { expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); expect(callback2).toHaveBeenCalledTimes(1); - expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined); + expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY); connectionManager.disconnect(connection1); connectionManager.disconnect(connection2); @@ -412,8 +412,8 @@ describe('OnyxConnectionManager', () => { }); }); - describe('sourceValue parameter', () => { - it('should pass the sourceValue parameter to collection callbacks when waitForCollectionCallback is true', async () => { + describe('collection callback arguments', () => { + it('should call collection callbacks with only value and key when waitForCollectionCallback is true', async () => { const obj1 = {id: 'entry1_id', name: 'entry1_name'}; const obj2 = {id: 'entry2_id', name: 'entry2_name'}; @@ -428,7 +428,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(); @@ -437,7 +437,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(); @@ -452,13 +452,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 callbacks when waitForCollectionCallback is false', async () => { + it('should call regular callbacks with only value and key when waitForCollectionCallback is false', 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 a9629ab3c..6cb4868e2 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -240,7 +240,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, { @@ -250,19 +250,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 5ff5d6f96..63589d7ab 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1065,7 +1065,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); }); }); @@ -1091,10 +1091,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); }) ); }); @@ -1149,10 +1149,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); }) ); }); @@ -1187,7 +1185,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 @@ -1224,8 +1222,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); @@ -1483,8 +1481,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); @@ -1660,10 +1658,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)); @@ -1734,7 +1728,6 @@ describe('Onyx', () => { [cat]: {age: 3, sound: 'meow'}, }, ONYX_KEYS.COLLECTION.ANIMALS, - {[cat]: {age: 3, sound: 'meow'}}, ); expect(animalsCollectionCallback).toHaveBeenNthCalledWith( 2, @@ -1743,7 +1736,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); @@ -1755,7 +1747,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 8767dca82..27f899bee 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -553,11 +553,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); });