Skip to content

Commit b9ac99b

Browse files
authored
Merge pull request Expensify#801 from callstack-internal/feature/onyx-store-pr-2
Remove `sourceValue` from Onyx
2 parents f3fc3b6 + c154ed4 commit b9ac99b

11 files changed

Lines changed: 38 additions & 98 deletions

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,10 +257,10 @@ export default MyComponent;
257257
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.
258258

259259
```js
260-
Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (allReports, collectionKey, sourceValue) => {...});
260+
Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (allReports, collectionKey) => {...});
261261
```
262262

263-
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.
263+
This will fire the callback once with the entire collection initially and later with an updated version of the collection when individual keys update.
264264

265265
### Performance Considerations When Using Collections
266266

lib/OnyxConnectionManager.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,6 @@ type ConnectionMetadata = {
4343
* The last callback key returned by `OnyxUtils.subscribeToKey()`'s callback.
4444
*/
4545
cachedCallbackKey?: OnyxKey;
46-
47-
/**
48-
* The value that triggered the last update
49-
*/
50-
sourceValue?: OnyxValue<OnyxKey>;
5146
};
5247

5348
/**
@@ -138,11 +133,7 @@ class OnyxConnectionManager {
138133
for (const callback of connection.callbacks.values()) {
139134
try {
140135
if (OnyxKeys.isCollectionKey(connection.onyxKey)) {
141-
(callback as CollectionConnectCallback<OnyxKey>)(
142-
connection.cachedCallbackValue as Record<string, unknown>,
143-
connection.cachedCallbackKey as OnyxKey,
144-
connection.sourceValue,
145-
);
136+
(callback as CollectionConnectCallback<OnyxKey>)(connection.cachedCallbackValue as Record<string, unknown>, connection.cachedCallbackKey as OnyxKey);
146137
} else {
147138
(callback as DefaultConnectCallback<OnyxKey>)(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey);
148139
}
@@ -167,15 +158,14 @@ class OnyxConnectionManager {
167158

168159
// If there is no connection yet for that connection ID, we create a new one.
169160
if (!connectionMetadata) {
170-
const callback: ConnectCallback = (value, key, sourceValue) => {
161+
const callback: ConnectCallback = (value: OnyxValue<OnyxKey>, key: OnyxKey) => {
171162
const createdConnection = this.connectionsMap.get(connectionID);
172163
if (createdConnection) {
173164
// We signal that the first connection was made and now any new subscribers
174165
// can fire their callbacks immediately with the cached value when connecting.
175166
createdConnection.isConnectionMade = true;
176167
createdConnection.cachedCallbackValue = value;
177168
createdConnection.cachedCallbackKey = key;
178-
createdConnection.sourceValue = sourceValue;
179169
this.fireCallbacks(connectionID);
180170
}
181171
};

lib/OnyxUtils.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import StorageCircuitBreaker from './StorageCircuitBreaker';
1010
import Storage from './storage';
1111
import {StorageErrorClass} from './storage/errors';
1212
import type {
13-
CollectionConnectCallback,
1413
CollectionKeyBase,
1514
ConnectOptions,
1615
DeepRecord,
@@ -581,7 +580,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
581580

582581
try {
583582
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});
584-
subscriber.callback(cachedCollection, subscriber.key, partialCollection);
583+
subscriber.callback(cachedCollection, subscriber.key);
585584
} catch (error) {
586585
Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`);
587586
}
@@ -676,7 +675,7 @@ function keyChanged<TKey extends OnyxKey>(
676675
cachedCollections[subscriber.key] = cachedCollection;
677676
}
678677
lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key});
679-
(subscriber.callback as CollectionConnectCallback<OnyxKey>)(cachedCollection, subscriber.key, {[key]: value});
678+
subscriber.callback(cachedCollection, subscriber.key);
680679
continue;
681680
}
682681

lib/types.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,14 +223,14 @@ 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
/**
229229
* Represents the options used in `Onyx.connect()` method.
230230
*
231231
* For a collection root key (e.g. `ONYXKEYS.COLLECTION.REPORT`), the callback fires
232232
* with the entire collection object whenever any member changes (signature
233-
* `(collection, key, sourceValue)`). For any other key, the callback fires with the value at
233+
* `(collection, key)`). For any other key, the callback fires with the value at
234234
* that key (signature `(value, key)`).
235235
*/
236236
// 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<TKey extends OnyxKey> = BaseConnectOptions & {
239239
key: TKey;
240240

241241
/** A function that will be called when the Onyx data we are subscribed changes. */
242-
callback?: (
243-
value: TKey extends CollectionKeyBase ? NonUndefined<OnyxCollection<KeyValueMapping[TKey]>> : OnyxEntry<KeyValueMapping[TKey]>,
244-
key: TKey,
245-
sourceValue?: TKey extends CollectionKeyBase ? NonUndefined<OnyxCollection<KeyValueMapping[TKey]>> : never,
246-
) => void;
242+
callback?: (value: TKey extends CollectionKeyBase ? NonUndefined<OnyxCollection<KeyValueMapping[TKey]>> : OnyxEntry<KeyValueMapping[TKey]>, key: TKey) => void;
247243
};
248244

249245
type CallbackToStateMapping<TKey extends OnyxKey> = ConnectOptions<TKey> & {

lib/useOnyx.ts

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

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

33-
type ResultMetadata<TValue> = {
33+
type ResultMetadata = {
3434
status: FetchStatus;
35-
sourceValue?: NonNullable<TValue> | undefined;
3635
};
3736

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

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

120-
// Inside useOnyx.ts, we need to track the sourceValue separately
121-
const sourceValueRef = useRef<NonNullable<TReturnValue> | undefined>(undefined);
122-
123119
// Cache the options key to avoid regenerating it every getSnapshot call
124120
const cacheKey = useMemo(
125121
() =>
@@ -226,7 +222,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
226222
previousValueRef.current ?? undefined,
227223
{
228224
status: newFetchStatus,
229-
sourceValue: sourceValueRef.current,
230225
},
231226
];
232227
}
@@ -246,7 +241,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
246241
if (hasMountedRef.current) {
247242
previousValueRef.current = null;
248243
newValueRef.current = null;
249-
sourceValueRef.current = undefined;
250244
resultRef.current = [undefined, {status: 'loading'}];
251245
shouldGetCachedValueRef.current = true;
252246
}
@@ -257,7 +251,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
257251

258252
connectionRef.current = connectionManager.connect<CollectionKeyBase>({
259253
key,
260-
callback: (value, callbackKey, sourceValue) => {
254+
callback: () => {
261255
isConnectingRef.current = false;
262256
onStoreChangeFnRef.current = onStoreChange;
263257

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

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

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: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ describe('OnyxConnectionManager', () => {
135135
await act(async () => waitForPromisesToResolve());
136136

137137
// Both subscribers share the connection and receive the whole collection object.
138-
expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
139-
expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
138+
expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY);
139+
expect(callback2).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY);
140140

141141
connectionManager.disconnect(connection1);
142142
connectionManager.disconnect(connection2);
@@ -232,7 +232,7 @@ describe('OnyxConnectionManager', () => {
232232

233233
await act(async () => waitForPromisesToResolve());
234234

235-
expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
235+
expect(callback1).toHaveBeenCalledWith(collection, ONYXKEYS.COLLECTION.TEST_KEY);
236236

237237
const callback2 = jest.fn();
238238
const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2});
@@ -402,8 +402,8 @@ describe('OnyxConnectionManager', () => {
402402
});
403403
});
404404

405-
describe('sourceValue parameter', () => {
406-
it('should pass the sourceValue parameter to collection-root callbacks', async () => {
405+
describe('collection callback arguments', () => {
406+
it('should call collection-root callbacks with only the value and key', async () => {
407407
const obj1 = {id: 'entry1_id', name: 'entry1_name'};
408408
const obj2 = {id: 'entry2_id', name: 'entry2_name'};
409409

@@ -417,7 +417,7 @@ describe('OnyxConnectionManager', () => {
417417

418418
// Initial callback with undefined values
419419
expect(callback).toHaveBeenCalledTimes(1);
420-
expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY, undefined);
420+
expect(callback).toHaveBeenCalledWith(undefined, ONYXKEYS.COLLECTION.TEST_KEY);
421421

422422
// Reset mock to test the next update
423423
callback.mockReset();
@@ -426,7 +426,7 @@ describe('OnyxConnectionManager', () => {
426426
await Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1);
427427

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

431431
// Reset mock to test the next update
432432
callback.mockReset();
@@ -441,13 +441,12 @@ describe('OnyxConnectionManager', () => {
441441
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2,
442442
},
443443
ONYXKEYS.COLLECTION.TEST_KEY,
444-
{[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2},
445444
);
446445

447446
connectionManager.disconnect(connection);
448447
});
449448

450-
it('should not pass sourceValue to regular (non-collection) key callbacks', async () => {
449+
it('should call regular (non-collection) key callbacks with only the value and key', async () => {
451450
const obj1 = {id: 'entry1_id', name: 'entry1_name'};
452451

453452
const callback = jest.fn();

tests/unit/onyxClearWebStorageTest.ts

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

241241
// And it should be called with the expected parameters each time
242-
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST, undefined);
242+
expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, ONYX_KEYS.COLLECTION.TEST);
243243
expect(collectionCallback).toHaveBeenNthCalledWith(
244244
2,
245245
{
@@ -249,19 +249,8 @@ describe('Set data while storage is clearing', () => {
249249
test_4: 4,
250250
},
251251
ONYX_KEYS.COLLECTION.TEST,
252-
{
253-
test_1: 1,
254-
test_2: 2,
255-
test_3: 3,
256-
test_4: 4,
257-
},
258252
);
259-
expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST, {
260-
test_1: undefined,
261-
test_2: undefined,
262-
test_3: undefined,
263-
test_4: undefined,
264-
});
253+
expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST);
265254
})
266255
);
267256
});

0 commit comments

Comments
 (0)