Skip to content

Commit f34eb51

Browse files
fabioh8010fabioh8010
committed
Improve logic and tests
1 parent 03c4afe commit f34eb51

File tree

5 files changed

+106
-69
lines changed

5 files changed

+106
-69
lines changed

‎lib/Onyx.ts‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,12 @@ function disconnect(connection: Connection): void {
131131
* @param value value to store
132132
*/
133133
function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>): Promise<void> {
134+
// When we use Onyx.set to set a key we want to clear the current delta changes from Onyx.merge that were queued
135+
// before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
136+
if (OnyxUtils.hasPendingMergeForKey(key)) {
137+
delete OnyxUtils.getMergeQueue()[key];
138+
}
139+
134140
const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs();
135141
if (skippableCollectionMemberIDs.size) {
136142
try {
@@ -143,12 +149,6 @@ function set<TKey extends OnyxKey>(key: TKey, value: OnyxSetInput<TKey>): Promis
143149
}
144150
}
145151

146-
// When we use Onyx.set to set a key we want to clear the current delta changes from Onyx.merge that were queued
147-
// before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes.
148-
if (OnyxUtils.hasPendingMergeForKey(key)) {
149-
delete OnyxUtils.getMergeQueue()[key];
150-
}
151-
152152
// Onyx.set will ignore `undefined` values as inputs, therefore we can return early.
153153
if (value === undefined) {
154154
return Promise.resolve();
@@ -218,10 +218,8 @@ function multiSet(data: OnyxMultiSetInput): Promise<void> {
218218
newData = Object.keys(newData).reduce((result: OnyxMultiSetInput, key) => {
219219
try {
220220
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
221-
if (!skippableCollectionMemberIDs.has(collectionMemberID)) {
222-
// eslint-disable-next-line no-param-reassign
223-
result[key] = newData[key];
224-
}
221+
// eslint-disable-next-line no-param-reassign
222+
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? newData[key] : null;
225223
} catch {
226224
// Key is not a collection one or something went wrong during split, so we assign the data to result anyway.
227225
// eslint-disable-next-line no-param-reassign
@@ -273,7 +271,8 @@ function merge<TKey extends OnyxKey>(key: TKey, changes: OnyxMergeInput<TKey>):
273271
try {
274272
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
275273
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
276-
return Promise.resolve();
274+
// eslint-disable-next-line no-param-reassign
275+
changes = undefined;
277276
}
278277
} catch (e) {
279278
// Key is not a collection one or something went wrong during split, so we proceed with the function's logic.
@@ -407,10 +406,8 @@ function mergeCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TK
407406
resultCollection = Object.keys(resultCollection).reduce((result: OnyxInputKeyValueMapping, key) => {
408407
try {
409408
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey);
410-
if (!skippableCollectionMemberIDs.has(collectionMemberID)) {
411-
// eslint-disable-next-line no-param-reassign
412-
result[key] = resultCollection[key];
413-
}
409+
// eslint-disable-next-line no-param-reassign
410+
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
414411
} catch {
415412
// Something went wrong during split, so we assign the data to result anyway.
416413
// eslint-disable-next-line no-param-reassign
@@ -825,10 +822,8 @@ function setCollection<TKey extends CollectionKeyBase, TMap>(collectionKey: TKey
825822
resultCollection = Object.keys(resultCollection).reduce((result: OnyxInputKeyValueMapping, key) => {
826823
try {
827824
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey);
828-
if (!skippableCollectionMemberIDs.has(collectionMemberID)) {
829-
// eslint-disable-next-line no-param-reassign
830-
result[key] = resultCollection[key];
831-
}
825+
// eslint-disable-next-line no-param-reassign
826+
result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null;
832827
} catch {
833828
// Something went wrong during split, so we assign the data to result anyway.
834829
// eslint-disable-next-line no-param-reassign

‎lib/OnyxUtils.ts‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -258,17 +258,6 @@ function reduceCollectionWithSelector<TKey extends CollectionKeyBase, TMap, TRet
258258

259259
/** Get some data from the store */
260260
function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): Promise<TValue> {
261-
if (skippableCollectionMemberIDs.size) {
262-
try {
263-
const [, collectionMemberID] = splitCollectionMemberKey(key);
264-
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
265-
return Promise.resolve(undefined as TValue);
266-
}
267-
} catch (e) {
268-
// Key is not a collection one or something went wrong during split, so we proceed with the function's logic.
269-
}
270-
}
271-
272261
// When we already have the value in cache - resolve right away
273262
if (cache.hasCacheForKey(key)) {
274263
return Promise.resolve(cache.get(key) as TValue);
@@ -284,6 +273,18 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
284273
// Otherwise retrieve the value from storage and capture a promise to aid concurrent usages
285274
const promise = Storage.getItem(key)
286275
.then((val) => {
276+
if (skippableCollectionMemberIDs.size) {
277+
try {
278+
const [, collectionMemberID] = splitCollectionMemberKey(key);
279+
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
280+
// eslint-disable-next-line no-param-reassign
281+
val = undefined as OnyxValue<TKey>;
282+
}
283+
} catch (e) {
284+
// Key is not a collection one or something went wrong during split, so we proceed with the function's logic.
285+
}
286+
}
287+
287288
if (val === undefined) {
288289
cache.addNullishStorageKey(key);
289290
return undefined;
@@ -320,17 +321,6 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
320321
* These missingKeys will be later used to multiGet the data from the storage.
321322
*/
322323
keys.forEach((key) => {
323-
if (skippableCollectionMemberIDs.size) {
324-
try {
325-
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
326-
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
327-
return;
328-
}
329-
} catch (e) {
330-
// Key is not a collection one or something went wrong during split, so we proceed with the function's logic.
331-
}
332-
}
333-
334324
const cacheValue = cache.get(key) as OnyxValue<TKey>;
335325
if (cacheValue) {
336326
dataMap.set(key, cacheValue);
@@ -373,6 +363,17 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
373363
// temp object is used to merge the missing data into the cache
374364
const temp: OnyxCollection<KeyValueMapping[TKey]> = {};
375365
values.forEach(([key, value]) => {
366+
if (skippableCollectionMemberIDs.size) {
367+
try {
368+
const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key);
369+
if (skippableCollectionMemberIDs.has(collectionMemberID)) {
370+
return;
371+
}
372+
} catch (e) {
373+
// Key is not a collection one or something went wrong during split, so we proceed with the function's logic.
374+
}
375+
}
376+
376377
dataMap.set(key, value as OnyxValue<TKey>);
377378
temp[key] = value as OnyxValue<TKey>;
378379
});

‎tests/components/ViewWithCollections.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function ViewWithCollections(
2626
<View>
2727
{Object.values(collections).map((collection, i) => (
2828
// eslint-disable-next-line react/no-array-index-key
29-
<Text key={i}>{collection.ID}</Text>
29+
<Text key={i}>{collection?.ID}</Text>
3030
))}
3131
</View>
3232
);

‎tests/unit/useOnyxTest.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ describe('useOnyx', () => {
661661
});
662662

663663
describe('skippable collection member ids', () => {
664-
it('should 1', async () => {
664+
it('should always return undefined entry when subscribing to a collection with skippable member ids', async () => {
665665
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
666666
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
667667
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
@@ -679,27 +679,36 @@ describe('useOnyx', () => {
679679
});
680680
expect(result.current[1].status).toEqual('loaded');
681681

682-
await act(async () => Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'something'));
682+
await act(async () =>
683+
Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
684+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name_changed'},
685+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name_changed'},
686+
[`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`]: {id: 'skippable-id_id', name: 'skippable-id_name_changed'},
687+
} as GenericCollection),
688+
);
683689

684690
expect(result.current[0]).toEqual({
685-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
686-
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
691+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name_changed'},
692+
[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name_changed'},
687693
[`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`]: undefined,
688694
});
689695
expect(result.current[1].status).toEqual('loaded');
690696
});
691697

692-
it('should 2', async () => {
698+
it('should always return undefined when subscribing to a skippable collection member id', async () => {
693699
// @ts-expect-error bypass
694-
await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'something');
700+
await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value');
695701

696-
const {result} = renderHook(() => useOnyx(ONYXKEYS.COLLECTION.TEST_KEY));
702+
const {result} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`));
697703

698704
await act(async () => waitForPromisesToResolve());
699705

700-
expect(result.current[0]).toEqual({
701-
[`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`]: undefined,
702-
});
706+
expect(result.current[0]).toEqual(undefined);
707+
expect(result.current[1].status).toEqual('loaded');
708+
709+
await act(async () => Onyx.merge(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value_changed'));
710+
711+
expect(result.current[0]).toEqual(undefined);
703712
expect(result.current[1].status).toEqual('loaded');
704713
});
705714
});

‎tests/unit/withOnyxTest.tsx‎

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable rulesdir/onyx-props-must-have-default */
22
import React from 'react';
3-
import {render} from '@testing-library/react-native';
3+
import {act, render} from '@testing-library/react-native';
44
import Onyx, {withOnyx} from '../../lib';
55
import type {ViewWithTextOnyxProps, ViewWithTextProps} from '../components/ViewWithText';
66
import ViewWithText from '../components/ViewWithText';
@@ -820,27 +820,59 @@ describe('withOnyxTest', () => {
820820
});
821821

822822
describe('skippable collection member ids', () => {
823-
it('should 1', async () => {
824-
// TODO: Finish
825-
const TestComponentWithOnyx = withOnyx<ViewWithCollectionsProps, {text: unknown}>({
826-
text: {
823+
it('should always return undefined entry when subscribing to a collection with skippable member ids', async () => {
824+
await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
825+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry1`]: {ID: 'entry1_id'},
826+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry2`]: {ID: 'entry2_id'},
827+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`]: {ID: 'skippable-id_id'},
828+
} as GenericCollection);
829+
830+
const TestComponentWithOnyx = withOnyx<ViewWithCollectionsProps, {collections: unknown}>({
831+
collections: {
827832
key: ONYX_KEYS.COLLECTION.TEST_KEY,
828833
},
829834
})(ViewWithCollections);
830-
const onRender = jest.fn();
831-
const renderResult = render(<TestComponentWithOnyx onRender={onRender} />);
835+
const renderResult = render(<TestComponentWithOnyx />);
832836

833-
Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
834-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry1`]: {ID: 1},
835-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry2`]: {ID: 2},
836-
[`${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`]: {ID: 3},
837-
} as GenericCollection);
837+
await waitForPromisesToResolve();
838838

839-
return waitForPromisesToResolve().then(() => {
840-
expect(renderResult.getByText('1')).not.toBeNull();
841-
expect(renderResult.getByText('2')).not.toBeNull();
842-
expect(renderResult.getByText('3')).toBeNull();
843-
});
839+
expect(renderResult.queryByText('entry1_id')).not.toBeNull();
840+
expect(renderResult.queryByText('entry2_id')).not.toBeNull();
841+
expect(renderResult.queryByText('entry3_id')).toBeNull();
842+
843+
await act(async () =>
844+
Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {
845+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry1`]: {ID: 'entry1_id_changed'},
846+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}entry2`]: {ID: 'entry2_id_changed'},
847+
[`${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`]: {ID: 'skippable-id_id_changed'},
848+
} as GenericCollection),
849+
);
850+
851+
expect(renderResult.queryByText('entry1_id_changed')).not.toBeNull();
852+
expect(renderResult.queryByText('entry2_id_changed')).not.toBeNull();
853+
expect(renderResult.queryByText('skippable-id_id_changed')).toBeNull();
854+
});
855+
856+
it('should always return undefined when subscribing to a skippable collection member id', async () => {
857+
const TestComponentWithOnyx = withOnyx<ViewWithTextProps, ViewWithTextOnyxProps>({
858+
text: {
859+
key: `${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`,
860+
},
861+
})(ViewWithText);
862+
863+
// @ts-expect-error bypass
864+
await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value');
865+
866+
const renderResult = render(<TestComponentWithOnyx />);
867+
868+
await waitForPromisesToResolve();
869+
await waitForPromisesToResolve();
870+
871+
expect(renderResult.queryByText('skippable-id_value')).toBeNull();
872+
873+
await act(async () => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_KEY}skippable-id`, 'skippable-id_value_changed'));
874+
875+
expect(renderResult.queryByText('skippable-id_value_changed')).toBeNull();
844876
});
845877
});
846878
});

0 commit comments

Comments
 (0)