Skip to content

Commit e3a8e47

Browse files
cristipavalcristipaval
authored
Merge pull request Expensify#680 from callstack-internal/VickyStash/refactor/revert-batch-updates
Revert "Merge pull request Expensify#669 from callstack-internal/VickyStash/refactor/batch-all-subscribers-updates"
2 parents 95eba05 + 63129ab commit e3a8e47

File tree

4 files changed

+49
-84
lines changed

4 files changed

+49
-84
lines changed

‎lib/OnyxUtils.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,6 @@ function keysChanged<TKey extends CollectionKeyBase>(
622622
partialPreviousCollection: OnyxCollection<KeyValueMapping[TKey]> | undefined,
623623
notifyConnectSubscribers = true,
624624
notifyWithOnyxSubscribers = true,
625-
notifyUseOnyxHookSubscribers = true,
626625
): void {
627626
// We prepare the "cached collection" which is the entire collection + the new partial data that
628627
// was merged in via mergeCollection().
@@ -658,8 +657,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
658657

659658
// Regular Onyx.connect() subscriber found.
660659
if (typeof subscriber.callback === 'function') {
661-
// Check if it's a useOnyx or a regular Onyx.connect() subscriber
662-
if ((subscriber.isUseOnyxSubscriber && !notifyUseOnyxHookSubscribers) || (!subscriber.isUseOnyxSubscriber && !notifyConnectSubscribers)) {
660+
if (!notifyConnectSubscribers) {
663661
continue;
664662
}
665663

@@ -818,7 +816,6 @@ function keyChanged<TKey extends OnyxKey>(
818816
canUpdateSubscriber: (subscriber?: Mapping<OnyxKey>) => boolean = () => true,
819817
notifyConnectSubscribers = true,
820818
notifyWithOnyxSubscribers = true,
821-
notifyUseOnyxHookSubscribers = true,
822819
): void {
823820
// Add or remove this key from the recentlyAccessedKeys lists
824821
if (value !== null) {
@@ -860,8 +857,7 @@ function keyChanged<TKey extends OnyxKey>(
860857

861858
// Subscriber is a regular call to connect() and provided a callback
862859
if (typeof subscriber.callback === 'function') {
863-
// Check if it's a useOnyx or a regular Onyx.connect() subscriber
864-
if ((subscriber.isUseOnyxSubscriber && !notifyUseOnyxHookSubscribers) || (!subscriber.isUseOnyxSubscriber && !notifyConnectSubscribers)) {
860+
if (!notifyConnectSubscribers) {
865861
continue;
866862
}
867863
if (lastConnectionCallbackData.has(subscriber.subscriptionID) && lastConnectionCallbackData.get(subscriber.subscriptionID) === value) {
@@ -1082,8 +1078,8 @@ function scheduleSubscriberUpdate<TKey extends OnyxKey>(
10821078
previousValue: OnyxValue<TKey>,
10831079
canUpdateSubscriber: (subscriber?: Mapping<OnyxKey>) => boolean = () => true,
10841080
): Promise<void> {
1085-
const promise = Promise.resolve().then(() => keyChanged(key, value, previousValue, canUpdateSubscriber, true, false, false));
1086-
batchUpdates(() => keyChanged(key, value, previousValue, canUpdateSubscriber, false, true, true));
1081+
const promise = Promise.resolve().then(() => keyChanged(key, value, previousValue, canUpdateSubscriber, true, false));
1082+
batchUpdates(() => keyChanged(key, value, previousValue, canUpdateSubscriber, false, true));
10871083
return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined);
10881084
}
10891085

@@ -1097,8 +1093,8 @@ function scheduleNotifyCollectionSubscribers<TKey extends OnyxKey>(
10971093
value: OnyxCollection<KeyValueMapping[TKey]>,
10981094
previousValue?: OnyxCollection<KeyValueMapping[TKey]>,
10991095
): Promise<void> {
1100-
const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue, true, false, false));
1101-
batchUpdates(() => keysChanged(key, value, previousValue, false, true, true));
1096+
const promise = Promise.resolve().then(() => keysChanged(key, value, previousValue, true, false));
1097+
batchUpdates(() => keysChanged(key, value, previousValue, false, true));
11021098
return Promise.all([maybeFlushBatchUpdates(), promise]).then(() => undefined);
11031099
}
11041100

‎lib/types.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,6 @@ type BaseConnectOptions = {
273273
* with the same connect configurations.
274274
*/
275275
reuseConnection?: boolean;
276-
277-
/** Indicates whether this subscriber is created from the useOnyx hook. */
278-
isUseOnyxSubscriber?: boolean;
279276
};
280277

281278
/** Represents the callback function used in `Onyx.connect()` method with a regular key. */

‎lib/useOnyx.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,6 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
359359
initWithStoredValues: options?.initWithStoredValues,
360360
waitForCollectionCallback: OnyxUtils.isCollectionKey(key) as true,
361361
reuseConnection: options?.reuseConnection,
362-
isUseOnyxSubscriber: true,
363362
});
364363

365364
checkEvictableKey();

‎tests/unit/useOnyxTest.ts‎

Lines changed: 43 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ describe('useOnyx', () => {
162162
});
163163

164164
it('should return value and loaded state when loading cached key', async () => {
165-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test'));
165+
Onyx.set(ONYXKEYS.TEST_KEY, 'test');
166166

167167
const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
168168

@@ -201,7 +201,7 @@ describe('useOnyx', () => {
201201
});
202202

203203
it('should return value from cache, and return updated value after a merge operation', async () => {
204-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, 'test1'));
204+
Onyx.set(ONYXKEYS.TEST_KEY, 'test1');
205205

206206
const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
207207

@@ -227,7 +227,8 @@ describe('useOnyx', () => {
227227
expect(result2.current[0]).toBeUndefined();
228228
expect(result2.current[1].status).toEqual('loaded');
229229

230-
await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'));
230+
Onyx.merge(ONYXKEYS.TEST_KEY, 'test2');
231+
await act(async () => waitForPromisesToResolve());
231232

232233
expect(result1.current[0]).toEqual('test2');
233234
expect(result1.current[1].status).toEqual('loaded');
@@ -259,7 +260,8 @@ describe('useOnyx', () => {
259260
expect(result3.current[0]).toBeUndefined();
260261
expect(result3.current[1].status).toEqual('loaded');
261262

262-
await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'));
263+
Onyx.merge(ONYXKEYS.TEST_KEY, 'test2');
264+
await act(async () => waitForPromisesToResolve());
263265

264266
expect(result1.current[0]).toEqual('test2');
265267
expect(result1.current[1].status).toEqual('loaded');
@@ -272,7 +274,7 @@ describe('useOnyx', () => {
272274

273275
describe('selector', () => {
274276
it('should return selected data from a non-collection key', async () => {
275-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}));
277+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
276278

277279
const {result} = renderHook(() =>
278280
useOnyx(ONYXKEYS.TEST_KEY, {
@@ -291,13 +293,11 @@ describe('useOnyx', () => {
291293
});
292294

293295
it('should return selected data from a collection key', async () => {
294-
await act(async () =>
295-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
296-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
297-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
298-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
299-
} as GenericCollection),
300-
);
296+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
297+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
298+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
299+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
300+
} as GenericCollection);
301301

302302
const {result} = renderHook(() =>
303303
useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {
@@ -320,9 +320,6 @@ describe('useOnyx', () => {
320320
expect(result.current[1].status).toEqual('loaded');
321321

322322
await act(async () => Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, null));
323-
// Onyx.merge() with null value resolves immediately, but notifies subscribers asynchronously.
324-
// waitForPromisesToResolve() ensures these async operations complete before test assertions run.
325-
await act(async () => waitForPromisesToResolve());
326323

327324
expect(result.current[0]).toEqual({
328325
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: 'entry1_id',
@@ -332,7 +329,7 @@ describe('useOnyx', () => {
332329
});
333330

334331
it('should not change selected data if a property outside that data was changed', async () => {
335-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}));
332+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
336333

337334
// primitive
338335
const {result: primitiveResult} = renderHook(() =>
@@ -373,13 +370,11 @@ describe('useOnyx', () => {
373370
});
374371

375372
it('should not change selected collection data if a property outside that data was changed', async () => {
376-
await act(async () =>
377-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
378-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
379-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
380-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
381-
} as GenericCollection),
382-
);
373+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
374+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
375+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
376+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
377+
} as GenericCollection);
383378

384379
const {result} = renderHook(() =>
385380
useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {
@@ -399,7 +394,7 @@ describe('useOnyx', () => {
399394
});
400395

401396
it('should always use the current selector reference to return new data', async () => {
402-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}));
397+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
403398

404399
let selector = (entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name}`;
405400

@@ -422,7 +417,7 @@ describe('useOnyx', () => {
422417
});
423418

424419
it('should memoize selector output and return same reference when input unchanged', async () => {
425-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name', count: 1}));
420+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name', count: 1});
426421

427422
const {result} = renderHook(() =>
428423
useOnyx(ONYXKEYS.TEST_KEY, {
@@ -446,7 +441,7 @@ describe('useOnyx', () => {
446441
});
447442

448443
it('should return new reference when selector input changes', async () => {
449-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}));
444+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
450445

451446
const {result} = renderHook(() =>
452447
useOnyx(ONYXKEYS.TEST_KEY, {
@@ -473,7 +468,7 @@ describe('useOnyx', () => {
473468
it('should memoize selector output using deep equality check', async () => {
474469
let selectorCallCount = 0;
475470

476-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'}));
471+
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
477472

478473
const {result} = renderHook(() =>
479474
useOnyx(ONYXKEYS.TEST_KEY, {
@@ -500,7 +495,7 @@ describe('useOnyx', () => {
500495
});
501496

502497
it('should memoize primitive selector results correctly', async () => {
503-
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, {count: 5, name: 'test'}));
498+
Onyx.set(ONYXKEYS.TEST_KEY, {count: 5, name: 'test'});
504499

505500
const {result} = renderHook(() =>
506501
useOnyx(ONYXKEYS.TEST_KEY, {
@@ -906,13 +901,11 @@ describe('useOnyx', () => {
906901

907902
describe('dependencies', () => {
908903
it('should return the updated selected value when a external value passed to the dependencies list changes', async () => {
909-
await act(async () =>
910-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
911-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
912-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
913-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
914-
} as GenericCollection),
915-
);
904+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
905+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
906+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
907+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
908+
} as GenericCollection);
916909

917910
let externalValue = 'ex1';
918911

@@ -957,13 +950,11 @@ describe('useOnyx', () => {
957950

958951
describe('skippable collection member ids', () => {
959952
it('should always return undefined entry when subscribing to a collection with skippable member ids', async () => {
960-
await act(async () =>
961-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
962-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
963-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
964-
[`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`]: {id: 'skippable-id_id', name: 'skippable-id_name'},
965-
} as GenericCollection),
966-
);
953+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
954+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
955+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
956+
[`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`]: {id: 'skippable-id_id', name: 'skippable-id_name'},
957+
} as GenericCollection);
967958

968959
const {result} = renderHook(() => useOnyx(ONYXKEYS.COLLECTION.TEST_KEY));
969960

@@ -1038,9 +1029,6 @@ describe('useOnyx', () => {
10381029
expect(result1.current[0]).toBe('test');
10391030

10401031
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1041-
// Onyx.set() with null value resolves immediately, but notifies subscribers asynchronously.
1042-
// waitForPromisesToResolve() ensures these async operations complete before test assertions run.
1043-
await act(async () => waitForPromisesToResolve());
10441032

10451033
expect(result1.current[0]).toBeUndefined();
10461034
expect(logAlertFn).not.toBeCalled();
@@ -1074,9 +1062,6 @@ describe('useOnyx', () => {
10741062
expect(result1.current[0]).toBe('test');
10751063

10761064
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1077-
// Onyx.set() with null value resolves immediately, but notifies subscribers asynchronously.
1078-
// waitForPromisesToResolve() ensures these async operations complete before test assertions run.
1079-
await act(async () => waitForPromisesToResolve());
10801065

10811066
expect(result1.current[0]).toBeUndefined();
10821067
expect(logAlertFn).toHaveBeenCalledTimes(2);
@@ -1108,9 +1093,6 @@ describe('useOnyx', () => {
11081093
expect(result1.current[0]).toBe('test_changed');
11091094

11101095
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1111-
// Onyx.set() with null value resolves immediately, but notifies subscribers asynchronously.
1112-
// waitForPromisesToResolve() ensures these async operations complete before test assertions run.
1113-
await act(async () => waitForPromisesToResolve());
11141096

11151097
expect(result1.current[0]).toBeUndefined();
11161098
expect(logAlertFn).toHaveBeenCalledTimes(2);
@@ -1139,9 +1121,6 @@ describe('useOnyx', () => {
11391121
expect(result1.current[0]).toBe('test_changed');
11401122

11411123
await act(async () => Onyx.set(ONYXKEYS.TEST_KEY, null));
1142-
// Onyx.set() with null value resolves immediately, but notifies subscribers asynchronously.
1143-
// waitForPromisesToResolve() ensures these async operations complete before test assertions run.
1144-
await act(async () => waitForPromisesToResolve());
11451124

11461125
expect(result1.current[0]).toBe('undefined_changed');
11471126
expect(logAlertFn).toHaveBeenCalledTimes(2);
@@ -1201,11 +1180,9 @@ describe('useOnyx', () => {
12011180
});
12021181

12031182
it('should add the connection to the blocklist when setting "canEvict" to false', async () => {
1204-
await act(async () =>
1205-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1206-
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1207-
} as GenericCollection),
1208-
);
1183+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1184+
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1185+
} as GenericCollection);
12091186

12101187
renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`, {canEvict: false}));
12111188

@@ -1216,12 +1193,10 @@ describe('useOnyx', () => {
12161193
});
12171194

12181195
it('should handle removal/adding the connection to the blocklist properly when changing the evictable key to another', async () => {
1219-
await act(async () =>
1220-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1221-
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1222-
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
1223-
} as GenericCollection),
1224-
);
1196+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1197+
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1198+
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
1199+
} as GenericCollection);
12251200

12261201
const {rerender} = renderHook((key: string) => useOnyx(key, {canEvict: false}), {initialProps: `${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1` as string});
12271202

@@ -1240,11 +1215,9 @@ describe('useOnyx', () => {
12401215
});
12411216

12421217
it('should remove the connection from the blocklist when setting "canEvict" to true', async () => {
1243-
await act(async () =>
1244-
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1245-
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1246-
} as GenericCollection),
1247-
);
1218+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, {
1219+
[`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
1220+
} as GenericCollection);
12481221

12491222
const {rerender} = renderHook((canEvict: boolean) => useOnyx(`${ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY}entry1`, {canEvict}), {initialProps: false as boolean});
12501223

0 commit comments

Comments
 (0)