Skip to content

Commit fa7a8d5

Browse files
fabioh8010claude
fabioh8010
and
claude
committed
Remove the sourceValue callback parameter
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) <noreply@anthropic.com>
1 parent d91ec72 commit fa7a8d5

10 files changed

Lines changed: 32 additions & 81 deletions

‎lib/OnyxConnectionManager.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,6 @@ type ConnectionMetadata = {
4444
*/
4545
cachedCallbackKey?: OnyxKey;
4646

47-
/**
48-
* The value that triggered the last update
49-
*/
50-
sourceValue?: OnyxValue<OnyxKey>;
51-
5247
/**
5348
* Whether the subscriber is waiting for the collection callback to be fired.
5449
*/
@@ -143,7 +138,7 @@ class OnyxConnectionManager {
143138

144139
for (const callback of connection.callbacks.values()) {
145140
if (connection.waitForCollectionCallback) {
146-
(callback as CollectionConnectCallback<OnyxKey>)(connection.cachedCallbackValue as Record<string, unknown>, connection.cachedCallbackKey as OnyxKey, connection.sourceValue);
141+
(callback as CollectionConnectCallback<OnyxKey>)(connection.cachedCallbackValue as Record<string, unknown>, connection.cachedCallbackKey as OnyxKey);
147142
} else {
148143
(callback as DefaultConnectCallback<OnyxKey>)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey);
149144
}
@@ -165,15 +160,14 @@ class OnyxConnectionManager {
165160

166161
// If there is no connection yet for that connection ID, we create a new one.
167162
if (!connectionMetadata) {
168-
const callback: ConnectCallback = (value, key, sourceValue) => {
163+
const callback: ConnectCallback = (value: OnyxValue<OnyxKey>, key: OnyxKey) => {
169164
const createdConnection = this.connectionsMap.get(connectionID);
170165
if (createdConnection) {
171166
// We signal that the first connection was made and now any new subscribers
172167
// can fire their callbacks immediately with the cached value when connecting.
173168
createdConnection.isConnectionMade = true;
174169
createdConnection.cachedCallbackValue = value;
175170
createdConnection.cachedCallbackKey = key;
176-
createdConnection.sourceValue = sourceValue;
177171
this.fireCallbacks(connectionID);
178172
}
179173
};

‎lib/OnyxUtils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
600600
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});
601601

602602
if (subscriber.waitForCollectionCallback) {
603-
subscriber.callback(cachedCollection, subscriber.key, partialCollection);
603+
subscriber.callback(cachedCollection, subscriber.key);
604604
continue;
605605
}
606606

@@ -710,7 +710,7 @@ function keyChanged<TKey extends OnyxKey>(
710710
cachedCollections[subscriber.key] = cachedCollection;
711711
}
712712
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});
713-
subscriber.callback(cachedCollection, subscriber.key, {[key]: value});
713+
subscriber.callback(cachedCollection, subscriber.key);
714714
continue;
715715
}
716716

‎lib/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ type BaseConnectOptions = {
223223
type DefaultConnectCallback<TKey extends OnyxKey> = (value: OnyxEntry<KeyValueMapping[TKey]>, key: TKey) => void;
224224

225225
/** Represents the callback function used in `Onyx.connect()` method with a collection key. */
226-
type CollectionConnectCallback<TKey extends OnyxKey> = (value: NonUndefined<OnyxCollection<KeyValueMapping[TKey]>>, key: TKey, sourceValue?: OnyxValue<TKey>) => void;
226+
type CollectionConnectCallback<TKey extends OnyxKey> = (value: NonUndefined<OnyxCollection<KeyValueMapping[TKey]>>, key: TKey) => void;
227227

228228
/** Represents the options used in `Onyx.connect()` method with a regular key. */
229229
// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method!

‎lib/useOnyx.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,11 @@ type UseOnyxOptions<TKey extends OnyxKey, TReturnValue> = {
3131

3232
type FetchStatus = 'loading' | 'loaded';
3333

34-
type ResultMetadata<TValue> = {
34+
type ResultMetadata = {
3535
status: FetchStatus;
36-
sourceValue?: NonNullable<TValue> | undefined;
3736
};
3837

39-
type UseOnyxResult<TValue> = [NonNullable<TValue> | undefined, ResultMetadata<TValue>];
38+
type UseOnyxResult<TValue> = [NonNullable<TValue> | undefined, ResultMetadata];
4039

4140
function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
4241
key: TKey,
@@ -118,9 +117,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
118117
// Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution.
119118
const shouldGetCachedValueRef = useRef(true);
120119

121-
// Inside useOnyx.ts, we need to track the sourceValue separately
122-
const sourceValueRef = useRef<NonNullable<TReturnValue> | undefined>(undefined);
123-
124120
// Cache the options key to avoid regenerating it every getSnapshot call
125121
const cacheKey = useMemo(
126122
() =>
@@ -227,7 +223,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
227223
previousValueRef.current ?? undefined,
228224
{
229225
status: newFetchStatus,
230-
sourceValue: sourceValueRef.current,
231226
},
232227
];
233228
}
@@ -247,7 +242,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
247242
if (hasMountedRef.current) {
248243
previousValueRef.current = null;
249244
newValueRef.current = null;
250-
sourceValueRef.current = undefined;
251245
resultRef.current = [undefined, {status: 'loading'}];
252246
shouldGetCachedValueRef.current = true;
253247
}
@@ -258,7 +252,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
258252

259253
connectionRef.current = connectionManager.connect<CollectionKeyBase>({
260254
key,
261-
callback: (value, callbackKey, sourceValue) => {
255+
callback: () => {
262256
isConnectingRef.current = false;
263257
onStoreChangeFnRef.current = onStoreChange;
264258

@@ -269,9 +263,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
269263
// Signals that we want to get the newest cached value again in `getSnapshot()`.
270264
shouldGetCachedValueRef.current = true;
271265

272-
// sourceValue is unknown type, so we need to cast it to the correct type.
273-
sourceValueRef.current = sourceValue as NonNullable<TReturnValue>;
274-
275266
// Invalidate snapshot cache for this key when data changes
276267
onyxSnapshotCache.invalidateForKey(key);
277268

‎tests/perf-test/OnyxSnapshotCache.perf-test.ts‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,18 +44,9 @@ const complexSelectorOptions: UseOnyxOptions<string, ComplexSelectorResult> = {
4444
};
4545

4646
// Mock results
47-
const mockResult: UseOnyxResult<MockData> = [
48-
{id: 1, name: 'Test', value: 42},
49-
{status: 'loaded', sourceValue: {id: 1, name: 'Test', value: 42}},
50-
];
47+
const mockResult: UseOnyxResult<MockData> = [{id: 1, name: 'Test', value: 42}, {status: 'loaded'}];
5148

52-
const mockResults = Array.from(
53-
{length: 1000},
54-
(_, i): UseOnyxResult<MockData> => [
55-
{id: i, name: `Test${i}`, value: i * 10},
56-
{status: 'loaded', sourceValue: {id: i, name: `Test${i}`, value: i * 10}},
57-
],
58-
);
49+
const mockResults = Array.from({length: 1000}, (_, i): UseOnyxResult<MockData> => [{id: i, name: `Test${i}`, value: i * 10}, {status: 'loaded'}]);
5950

6051
describe('OnyxSnapshotCache', () => {
6152
let cache: OnyxSnapshotCache;
@@ -153,10 +144,7 @@ describe('OnyxSnapshotCache', () => {
153144

154145
test('getting cached result with complex selector (cache hit)', async () => {
155146
const cacheKey = cache.registerConsumer(ONYXKEYS.TEST_KEY, complexSelectorOptions);
156-
const complexResult: UseOnyxResult<ComplexSelectorResult> = [
157-
{id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'},
158-
{status: 'loaded', sourceValue: {id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}},
159-
];
147+
const complexResult: UseOnyxResult<ComplexSelectorResult> = [{id: 1, name: 'Test', computed: 84, formatted: 'Test: 42'}, {status: 'loaded'}];
160148
await measureFunction(
161149
() => {
162150
cache.getCachedResult(ONYXKEYS.TEST_KEY, cacheKey);

‎tests/perf-test/useOnyx.perf-test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const metadataStatusMatcher = (onyxKey: OnyxKey, expected: FetchStatus) => `meta
2020
type UseOnyxMatcherProps = {
2121
onyxKey: OnyxKey;
2222
data: OnyxValue<OnyxKey>;
23-
metadata: ResultMetadata<OnyxValue<OnyxKey>>;
23+
metadata: ResultMetadata;
2424
};
2525

2626
function UseOnyxMatcher({onyxKey, data, metadata}: UseOnyxMatcherProps) {

‎tests/unit/OnyxConnectionManagerTest.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ describe('OnyxConnectionManager', () => {
139139
expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`);
140140

141141
expect(callback2).toHaveBeenCalledTimes(1);
142-
expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
142+
expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY);
143143

144144
connectionManager.disconnect(connection1);
145145
connectionManager.disconnect(connection2);
@@ -412,8 +412,8 @@ describe('OnyxConnectionManager', () => {
412412
});
413413
});
414414

415-
describe('sourceValue parameter', () => {
416-
it('should pass the sourceValue parameter to collection callbacks when waitForCollectionCallback is true', async () => {
415+
describe('collection callback arguments', () => {
416+
it('should call collection callbacks with only value and key when waitForCollectionCallback is true', async () => {
417417
const obj1 = {id: 'entry1_id', name: 'entry1_name'};
418418
const obj2 = {id: 'entry2_id', name: 'entry2_name'};
419419

@@ -428,7 +428,7 @@ describe('OnyxConnectionManager', () => {
428428

429429
// Initial callback with undefined values
430430
expect(callback).toHaveBeenCalledTimes(1);
431-
expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
431+
expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY);
432432

433433
// Reset mock to test the next update
434434
callback.mockReset();
@@ -437,7 +437,7 @@ describe('OnyxConnectionManager', () => {
437437
await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1);
438438

439439
expect(callback).toHaveBeenCalledTimes(1);
440-
expect(callback).toHaveBeenCalledWith({[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}, ONYXKEYS.COLLECTION.TEST_KEY, {[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1});
440+
expect(callback).toHaveBeenCalledWith({[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1}, ONYXKEYS.COLLECTION.TEST_KEY);
441441

442442
// Reset mock to test the next update
443443
callback.mockReset();
@@ -452,13 +452,12 @@ describe('OnyxConnectionManager', () => {
452452
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2,
453453
},
454454
ONYXKEYS.COLLECTION.TEST_KEY,
455-
{[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2},
456455
);
457456

458457
connectionManager.disconnect(connection);
459458
});
460459

461-
it('should not pass sourceValue to regular callbacks when waitForCollectionCallback is false', async () => {
460+
it('should call regular callbacks with only value and key when waitForCollectionCallback is false', async () => {
462461
const obj1 = {id: 'entry1_id', name: 'entry1_name'};
463462

464463
const callback = jest.fn();

‎tests/unit/onyxClearWebStorageTest.ts‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ describe('Set data while storage is clearing', () => {
240240
expect(collectionCallback).toHaveBeenCalledTimes(3);
241241

242242
// And it should be called with the expected parameters each time
243-
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST, undefined);
243+
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST);
244244
expect(collectionCallback).toHaveBeenNthCalledWith(
245245
2,
246246
{
@@ -250,19 +250,8 @@ describe('Set data while storage is clearing', () => {
250250
test_4: 4,
251251
},
252252
ONYX_KEYS.COLLECTION.TEST,
253-
{
254-
test_1: 1,
255-
test_2: 2,
256-
test_3: 3,
257-
test_4: 4,
258-
},
259253
);
260-
expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST, {
261-
test_1: undefined,
262-
test_2: undefined,
263-
test_3: undefined,
264-
test_4: undefined,
265-
});
254+
expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST);
266255
})
267256
);
268257
});

‎tests/unit/onyxTest.ts‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,7 +1065,7 @@ describe('Onyx', () => {
10651065
.then(() => {
10661066
// Then we expect the callback to be called only once and the initial stored value to be initialCollectionData
10671067
expect(mockCallback).toHaveBeenCalledTimes(1);
1068-
expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, undefined);
1068+
expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION);
10691069
});
10701070
});
10711071

@@ -1091,10 +1091,10 @@ describe('Onyx', () => {
10911091
expect(mockCallback).toHaveBeenCalledTimes(2);
10921092

10931093
// AND the value for the first call should be null since the collection was not initialized at that point
1094-
expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY, undefined);
1094+
expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY);
10951095

10961096
// AND the value for the second call should be collectionUpdate since the collection was updated
1097-
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate);
1097+
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY);
10981098
})
10991099
);
11001100
});
@@ -1149,10 +1149,8 @@ describe('Onyx', () => {
11491149
expect(mockCallback).toHaveBeenCalledTimes(2);
11501150

11511151
// AND the value for the second call should be collectionUpdate
1152-
expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY, undefined);
1153-
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, {
1154-
[`${ONYX_KEYS.COLLECTION.TEST_POLICY}1`]: collectionUpdate.testPolicy_1,
1155-
});
1152+
expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_POLICY);
1153+
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY);
11561154
})
11571155
);
11581156
});
@@ -1187,7 +1185,7 @@ describe('Onyx', () => {
11871185
expect(mockCallback).toHaveBeenCalledTimes(2);
11881186

11891187
// And the value for the second call should be collectionUpdate
1190-
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY, {testPolicy_1: collectionUpdate.testPolicy_1});
1188+
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY);
11911189
})
11921190

11931191
// When merge is called again with the same collection not modified
@@ -1224,8 +1222,8 @@ describe('Onyx', () => {
12241222
{onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}} as GenericCollection},
12251223
]).then(() => {
12261224
expect(collectionCallback).toHaveBeenCalledTimes(2);
1227-
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_UPDATE, undefined);
1228-
expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE, {[itemKey]: {a: 'a'}});
1225+
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST_UPDATE);
1226+
expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE);
12291227

12301228
expect(testCallback).toHaveBeenCalledTimes(2);
12311229
expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined);
@@ -1483,8 +1481,8 @@ describe('Onyx', () => {
14831481
})
14841482
.then(() => {
14851483
expect(collectionCallback).toHaveBeenCalledTimes(2);
1486-
expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue});
1487-
expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}});
1484+
expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS);
1485+
expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS);
14881486

14891487
// Cat hasn't changed from its original value, expect only the initial connect callback
14901488
expect(catCallback).toHaveBeenCalledTimes(1);
@@ -1660,10 +1658,6 @@ describe('Onyx', () => {
16601658
},
16611659
},
16621660
ONYX_KEYS.COLLECTION.ROUTES,
1663-
{
1664-
[holidayRoute]: {waypoints: {0: 'Bed', 1: 'Home', 2: 'Beach', 3: 'Restaurant', 4: 'Home'}},
1665-
[routineRoute]: {waypoints: {0: 'Bed', 1: 'Home', 2: 'Work', 3: 'Gym'}},
1666-
},
16671661
);
16681662

16691663
connections.map((id) => Onyx.disconnect(id));
@@ -1734,7 +1728,6 @@ describe('Onyx', () => {
17341728
[cat]: {age: 3, sound: 'meow'},
17351729
},
17361730
ONYX_KEYS.COLLECTION.ANIMALS,
1737-
{[cat]: {age: 3, sound: 'meow'}},
17381731
);
17391732
expect(animalsCollectionCallback).toHaveBeenNthCalledWith(
17401733
2,
@@ -1743,7 +1736,6 @@ describe('Onyx', () => {
17431736
[dog]: {size: 'M', sound: 'woof'},
17441737
},
17451738
ONYX_KEYS.COLLECTION.ANIMALS,
1746-
{[dog]: {size: 'M', sound: 'woof'}},
17471739
);
17481740

17491741
expect(catCallback).toHaveBeenNthCalledWith(1, {age: 3, sound: 'meow'}, cat);
@@ -1755,7 +1747,6 @@ describe('Onyx', () => {
17551747
[lisa]: {age: 21, car: 'SUV'},
17561748
},
17571749
ONYX_KEYS.COLLECTION.PEOPLE,
1758-
{[bob]: {age: 25, car: 'sedan'}, [lisa]: {age: 21, car: 'SUV'}},
17591750
);
17601751

17611752
connections.map((id) => Onyx.disconnect(id));

‎tests/unit/onyxUtilsTest.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -553,11 +553,10 @@ describe('OnyxUtils', () => {
553553
OnyxUtils.keysChanged(ONYXKEYS.COLLECTION.TEST_KEY, {[entryKey]: entryData}, {});
554554

555555
expect(collectionCallback).toHaveBeenCalledTimes(1);
556-
// Collection subscriber receives the full cached collection, subscriber.key, and partial
557-
const [receivedCollection, receivedKey, receivedPartial] = collectionCallback.mock.calls[0];
556+
// Collection subscriber receives the full cached collection and subscriber.key
557+
const [receivedCollection, receivedKey] = collectionCallback.mock.calls[0];
558558
expect(receivedKey).toBe(ONYXKEYS.COLLECTION.TEST_KEY);
559559
expect(receivedCollection[entryKey]).toEqual(entryData);
560-
expect(receivedPartial).toEqual({[entryKey]: entryData});
561560

562561
Onyx.disconnect(connection);
563562
});

0 commit comments

Comments
 (0)