Skip to content

Commit 695e098

Browse files
mountinymountiny
authored
Merge pull request Expensify#722 from callstack-internal/bugfix/useOnyx-collection-member-key-clear
Fix useOnyx subscription to collection member key after Onyx.clear()
2 parents 079a43d + 6b56e38 commit 695e098

6 files changed

Lines changed: 82 additions & 59 deletions

File tree

‎lib/Onyx.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import type {
99
InitOptions,
1010
KeyValueMapping,
1111
OnyxInputKeyValueMapping,
12-
OnyxCollection,
1312
MixedOperationsQueue,
1413
OnyxKey,
1514
OnyxMergeCollectionInput,
@@ -309,8 +308,13 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
309308
cache.clearNullishStorageKeys();
310309

311310
const keysToBeClearedFromStorage: OnyxKey[] = [];
312-
const keyValuesToResetAsCollection: Record<OnyxKey, OnyxCollection<KeyValueMapping[OnyxKey]>> = {};
313311
const keyValuesToResetIndividually: KeyValueMapping = {};
312+
// We need to store old and new values for collection keys to properly notify subscribers when clearing Onyx
313+
// because the notification process needs the old values in cache but at that point they will be already removed from it.
314+
const keyValuesToResetAsCollection: Record<
315+
OnyxKey,
316+
{oldValues: Record<string, KeyValueMapping[OnyxKey] | undefined>; newValues: Record<string, KeyValueMapping[OnyxKey] | undefined>}
317+
> = {};
314318

315319
const allKeys = new Set([...cachedKeys, ...initialKeys]);
316320

@@ -344,9 +348,10 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
344348

345349
if (collectionKey) {
346350
if (!keyValuesToResetAsCollection[collectionKey]) {
347-
keyValuesToResetAsCollection[collectionKey] = {};
351+
keyValuesToResetAsCollection[collectionKey] = {oldValues: {}, newValues: {}};
348352
}
349-
keyValuesToResetAsCollection[collectionKey]![key] = newValue ?? undefined;
353+
keyValuesToResetAsCollection[collectionKey].oldValues[key] = oldValue;
354+
keyValuesToResetAsCollection[collectionKey].newValues[key] = newValue ?? undefined;
350355
} else {
351356
keyValuesToResetIndividually[key] = newValue ?? undefined;
352357
}
@@ -368,7 +373,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
368373
updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value));
369374
}
370375
for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) {
371-
updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value));
376+
updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value.newValues, value.oldValues));
372377
}
373378

374379
const defaultKeyValuePairs = Object.entries(

‎lib/useOnyx.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
330330
}
331331

332332
return resultRef.current;
333-
}, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey]);
333+
}, [options?.initWithStoredValues, options?.allowStaleData, options?.canBeMissing, key, memoizedSelector, cacheKey, previousKey]);
334334

335335
const subscribe = useCallback(
336336
(onStoreChange: () => void) => {

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import OnyxUtils, {clearOnyxUtilsInternals} from '../../lib/OnyxUtils';
99
import type GenericCollection from '../utils/GenericCollection';
1010
import type {OnyxUpdate} from '../../lib/Onyx';
1111
import createDeferredTask from '../../lib/createDeferredTask';
12-
import type {OnyxInputKeyValueMapping, OnyxKey, RetriableOnyxOperation} from '../../lib/types';
12+
import type {OnyxEntry, OnyxInputKeyValueMapping, OnyxKey, RetriableOnyxOperation} from '../../lib/types';
1313

1414
const ONYXKEYS = {
1515
TEST_KEY: 'test',
@@ -31,11 +31,11 @@ const evictableKeys = [ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY];
3131

3232
const initialKeyStates = {};
3333

34-
// @ts-expect-error bypass
35-
const generateTestSelector = (): Selector<string, unknown, unknown> => (value: Record<string, unknown>) => ({
36-
reportActionID: value.reportActionID,
37-
originalMessage: value.originalMessage,
38-
});
34+
const generateTestSelector = () =>
35+
((value: OnyxEntry<Record<string, unknown>>) => ({
36+
reportActionID: value?.reportActionID,
37+
originalMessage: value?.originalMessage,
38+
})) as Selector<OnyxKey, {reportActionID?: string; originalMessage?: string}>;
3939

4040
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
4141
const mockedReportActionsMap = getRandomReportActions(collectionKey);

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {measureRenders} from 'reassure';
55
import type {FetchStatus, OnyxEntry, OnyxKey, OnyxValue, ResultMetadata, UseOnyxOptions} from '../../lib';
66
import Onyx, {useOnyx} from '../../lib';
77
import StorageMock from '../../lib/storage';
8+
import type {UseOnyxSelector} from '../../lib/useOnyx';
89

910
const ONYXKEYS = {
1011
TEST_KEY: 'test',
@@ -142,8 +143,7 @@ describe('useOnyx', () => {
142143
<UseOnyxWrapper
143144
onyxKey={key}
144145
onyxOptions={{
145-
// @ts-expect-error bypass
146-
selector: (entry: OnyxEntry<{id: string; name: string}>) => `${entry?.name}_changed`,
146+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => `${entry?.name}_changed`) as UseOnyxSelector<OnyxKey, string>,
147147
}}
148148
/>,
149149
{
@@ -173,8 +173,7 @@ describe('useOnyx', () => {
173173
<UseOnyxWrapper
174174
onyxKey={key}
175175
onyxOptions={{
176-
// @ts-expect-error bypass
177-
selector: (entry: OnyxEntry<{id: string; name: string}>) => `${entry?.name}_changed`,
176+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => `${entry?.name}_changed`) as UseOnyxSelector<OnyxKey, string>,
178177
}}
179178
/>,
180179
{

‎tests/unit/storage/providers/IDBKeyvalProviderTest.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('IDBKeyValProvider', () => {
6767
expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value');
6868
});
6969

70-
it.skip('should remove the key when passing null', async () => {
70+
it('should remove the key when passing null', async () => {
7171
await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value');
7272
expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value');
7373

‎tests/unit/useOnyxTest.ts‎

Lines changed: 61 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import {act, renderHook} from '@testing-library/react-native';
2-
import type {OnyxCollection, OnyxEntry} from '../../lib';
2+
import type {OnyxCollection, OnyxEntry, OnyxKey} from '../../lib';
33
import Onyx, {useOnyx} from '../../lib';
44
import OnyxCache from '../../lib/OnyxCache';
55
import StorageMock from '../../lib/storage';
66
import type GenericCollection from '../utils/GenericCollection';
77
import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
88
import * as Logger from '../../lib/Logger';
99
import onyxSnapshotCache from '../../lib/OnyxSnapshotCache';
10+
import type {UseOnyxSelector} from '../../lib/useOnyx';
1011

1112
const ONYXKEYS = {
1213
TEST_KEY: 'test',
@@ -236,7 +237,7 @@ describe('useOnyx', () => {
236237
expect(result2.current[1].status).toEqual('loaded');
237238
});
238239

239-
it('should return updated state when connecting to the same key after an Onyx.clear() call', async () => {
240+
it('should return updated state when connecting to the same regular key after an Onyx.clear() call', async () => {
240241
await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test');
241242

242243
const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY));
@@ -270,6 +271,41 @@ describe('useOnyx', () => {
270271
expect(result3.current[0]).toEqual('test2');
271272
expect(result3.current[1].status).toEqual('loaded');
272273
});
274+
275+
it('should return updated state when connecting to the same colection member key after an Onyx.clear() call', async () => {
276+
await StorageMock.setItem<string>(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test');
277+
278+
const {result: result1} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`));
279+
280+
await act(async () => waitForPromisesToResolve());
281+
282+
expect(result1.current[0]).toEqual('test');
283+
expect(result1.current[1].status).toEqual('loaded');
284+
285+
await act(async () => Onyx.clear());
286+
287+
const {result: result2} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`));
288+
const {result: result3} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`));
289+
290+
await act(async () => waitForPromisesToResolve());
291+
292+
expect(result1.current[0]).toBeUndefined();
293+
expect(result1.current[1].status).toEqual('loaded');
294+
expect(result2.current[0]).toBeUndefined();
295+
expect(result2.current[1].status).toEqual('loaded');
296+
expect(result3.current[0]).toBeUndefined();
297+
expect(result3.current[1].status).toEqual('loaded');
298+
299+
Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test2');
300+
await act(async () => waitForPromisesToResolve());
301+
302+
expect(result1.current[0]).toEqual('test2');
303+
expect(result1.current[1].status).toEqual('loaded');
304+
expect(result2.current[0]).toEqual('test2');
305+
expect(result2.current[1].status).toEqual('loaded');
306+
expect(result3.current[0]).toEqual('test2');
307+
expect(result3.current[1].status).toEqual('loaded');
308+
});
273309
});
274310

275311
describe('selector', () => {
@@ -278,8 +314,7 @@ describe('useOnyx', () => {
278314

279315
const {result} = renderHook(() =>
280316
useOnyx(ONYXKEYS.TEST_KEY, {
281-
// @ts-expect-error bypass
282-
selector: (entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name}`,
317+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name}`) as UseOnyxSelector<OnyxKey, string>,
283318
}),
284319
);
285320

@@ -301,12 +336,11 @@ describe('useOnyx', () => {
301336

302337
const {result} = renderHook(() =>
303338
useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {
304-
// @ts-expect-error bypass
305-
selector: (entries: OnyxCollection<{id: string; name: string}>) =>
339+
selector: ((entries: OnyxCollection<{id: string; name: string}>) =>
306340
Object.entries(entries ?? {}).reduce<NonNullable<OnyxCollection<string>>>((acc, [key, value]) => {
307341
acc[key] = value?.id;
308342
return acc;
309-
}, {}),
343+
}, {})) as UseOnyxSelector<OnyxKey, NonNullable<OnyxCollection<string>>>,
310344
}),
311345
);
312346

@@ -334,24 +368,21 @@ describe('useOnyx', () => {
334368
// primitive
335369
const {result: primitiveResult} = renderHook(() =>
336370
useOnyx(ONYXKEYS.TEST_KEY, {
337-
// @ts-expect-error bypass
338-
selector: (entry: OnyxEntry<{id: string; name: string}>) => entry?.id,
371+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => entry?.id) as UseOnyxSelector<OnyxKey, string>,
339372
}),
340373
);
341374

342375
// object
343376
const {result: objectResult} = renderHook(() =>
344377
useOnyx(ONYXKEYS.TEST_KEY, {
345-
// @ts-expect-error bypass
346-
selector: (entry: OnyxEntry<{id: string; name: string}>) => ({id: entry?.id}),
378+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => ({id: entry?.id})) as UseOnyxSelector<OnyxKey, {id?: string}>,
347379
}),
348380
);
349381

350382
// array
351383
const {result: arrayResult} = renderHook(() =>
352384
useOnyx(ONYXKEYS.TEST_KEY, {
353-
// @ts-expect-error bypass
354-
selector: (entry: OnyxEntry<{id: string; name: string}>) => [{id: entry?.id}],
385+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => [{id: entry?.id}]) as UseOnyxSelector<OnyxKey, Array<{id?: string}>>,
355386
}),
356387
);
357388

@@ -378,8 +409,7 @@ describe('useOnyx', () => {
378409

379410
const {result} = renderHook(() =>
380411
useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {
381-
// @ts-expect-error bypass
382-
selector: (entry: OnyxEntry<{id: string; name: string}>) => ({id: entry?.id}),
412+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => ({id: entry?.id})) as UseOnyxSelector<OnyxKey, {id?: string}>,
383413
}),
384414
);
385415

@@ -396,19 +426,18 @@ describe('useOnyx', () => {
396426
it('should always use the current selector reference to return new data', async () => {
397427
Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
398428

399-
let selector = (entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name}`;
429+
let selector = ((entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name}`) as UseOnyxSelector<OnyxKey, string>;
400430

401431
const {result, rerender} = renderHook(() =>
402432
useOnyx(ONYXKEYS.TEST_KEY, {
403-
// @ts-expect-error bypass
404433
selector,
405434
}),
406435
);
407436

408437
expect(result.current[0]).toEqual('id - test_id, name - test_name');
409438
expect(result.current[1].status).toEqual('loaded');
410439

411-
selector = (entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name} - selector changed`;
440+
selector = ((entry: OnyxEntry<{id: string; name: string}>) => `id - ${entry?.id}, name - ${entry?.name} - selector changed`) as UseOnyxSelector<OnyxKey, string>;
412441
rerender(undefined);
413442

414443
expect(result.current[0]).toEqual('id - test_id, name - test_name - selector changed');
@@ -420,11 +449,10 @@ describe('useOnyx', () => {
420449

421450
const {result} = renderHook(() =>
422451
useOnyx(ONYXKEYS.TEST_KEY, {
423-
// @ts-expect-error bypass
424-
selector: (entry: OnyxEntry<{id: string; name: string; count: number}>) => ({
452+
selector: ((entry: OnyxEntry<{id: string; name: string; count: number}>) => ({
425453
id: entry?.id,
426454
name: entry?.name,
427-
}),
455+
})) as UseOnyxSelector<OnyxKey, {id?: string; name?: string}>,
428456
}),
429457
);
430458

@@ -444,11 +472,10 @@ describe('useOnyx', () => {
444472

445473
const {result} = renderHook(() =>
446474
useOnyx(ONYXKEYS.TEST_KEY, {
447-
// @ts-expect-error bypass
448-
selector: (entry: OnyxEntry<{id: string; name: string}>) => ({
475+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => ({
449476
id: entry?.id,
450477
name: entry?.name,
451-
}),
478+
})) as UseOnyxSelector<OnyxKey, {id?: string; name?: string}>,
452479
}),
453480
);
454481

@@ -471,11 +498,10 @@ describe('useOnyx', () => {
471498

472499
const {result} = renderHook(() =>
473500
useOnyx(ONYXKEYS.TEST_KEY, {
474-
// @ts-expect-error bypass
475-
selector: (entry: OnyxEntry<{id: string; name: string}>) => {
501+
selector: ((entry: OnyxEntry<{id: string; name: string}>) => {
476502
selectorCallCount++;
477503
return {id: entry?.id, name: entry?.name};
478-
},
504+
}) as UseOnyxSelector<OnyxKey, {id?: string; name?: string}>,
479505
}),
480506
);
481507

@@ -498,8 +524,7 @@ describe('useOnyx', () => {
498524

499525
const {result} = renderHook(() =>
500526
useOnyx(ONYXKEYS.TEST_KEY, {
501-
// @ts-expect-error bypass
502-
selector: (entry: OnyxEntry<{count: number; name: string}>) => entry?.count || 0,
527+
selector: ((entry: OnyxEntry<{count: number; name: string}>) => entry?.count || 0) as UseOnyxSelector<OnyxKey, number>,
503528
}),
504529
);
505530

@@ -721,8 +746,7 @@ describe('useOnyx', () => {
721746

722747
const {result} = renderHook(() =>
723748
useOnyx(ONYXKEYS.TEST_KEY, {
724-
// @ts-expect-error bypass
725-
selector: (entry: OnyxEntry<string>) => `${entry}_changed`,
749+
selector: ((entry: OnyxEntry<string>) => `${entry}_changed`) as UseOnyxSelector<OnyxKey, string>,
726750
}),
727751
);
728752

@@ -777,8 +801,7 @@ describe('useOnyx', () => {
777801
const {result} = renderHook(() =>
778802
useOnyx(ONYXKEYS.TEST_KEY, {
779803
initWithStoredValues: false,
780-
// @ts-expect-error bypass
781-
selector: (value: OnyxEntry<string>) => `${value}_selected`,
804+
selector: ((value: OnyxEntry<string>) => `${value}_selected`) as UseOnyxSelector<OnyxKey, string>,
782805
}),
783806
);
784807

@@ -912,12 +935,11 @@ describe('useOnyx', () => {
912935
useOnyx(
913936
ONYXKEYS.COLLECTION.TEST_KEY,
914937
{
915-
// @ts-expect-error bypass
916-
selector: (entries: OnyxCollection<{id: string; name: string}>) =>
938+
selector: ((entries: OnyxCollection<{id: string; name: string}>) =>
917939
Object.entries(entries ?? {}).reduce<NonNullable<OnyxCollection<string>>>((acc, [key, value]) => {
918940
acc[key] = `${value?.id}_${externalValue}`;
919941
return acc;
920-
}, {}),
942+
}, {})) as UseOnyxSelector<OnyxKey, NonNullable<OnyxCollection<string>>>,
921943
},
922944
[externalValue],
923945
),
@@ -981,8 +1003,7 @@ describe('useOnyx', () => {
9811003
});
9821004

9831005
it('should always return undefined when subscribing to a skippable collection member id', async () => {
984-
// @ts-expect-error bypass
985-
await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value');
1006+
await StorageMock.setItem<string>(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value');
9861007

9871008
const {result} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`));
9881009

@@ -1070,8 +1091,7 @@ describe('useOnyx', () => {
10701091
it('should log an alert if Onyx doesn\'t return selected data in loaded state and "canBeMissing" property is false', async () => {
10711092
const {result: result1} = renderHook(() =>
10721093
useOnyx(ONYXKEYS.TEST_KEY, {
1073-
// @ts-expect-error bypass
1074-
selector: (entry: OnyxEntry<string>) => (entry ? `${entry}_changed` : undefined),
1094+
selector: ((entry: OnyxEntry<string>) => (entry ? `${entry}_changed` : undefined)) as UseOnyxSelector<OnyxKey, string | undefined>,
10751095
canBeMissing: false,
10761096
}),
10771097
);
@@ -1101,9 +1121,8 @@ describe('useOnyx', () => {
11011121
it('should log an alert if Onyx doesn\'t return data but there is a selector that always return something and "canBeMissing" property is false', async () => {
11021122
const {result: result1} = renderHook(() =>
11031123
useOnyx(ONYXKEYS.TEST_KEY, {
1104-
// @ts-expect-error bypass
11051124
// This selector will always return a value, even if the Onyx data is missing.
1106-
selector: (entry: OnyxEntry<string>) => `${entry}_changed`,
1125+
selector: ((entry: OnyxEntry<string>) => `${entry}_changed`) as UseOnyxSelector<OnyxKey, string>,
11071126
canBeMissing: false,
11081127
}),
11091128
);

0 commit comments

Comments
 (0)