Merge pull request Expensify#566 from fabioh8010/bugfix/use-onyx-equa…

…lity-check Fix useOnyx equality check
callstack-internal · Jul 5, 2024 · b3aa495 · b3aa495
2 parents 76e7c9a + b99cc27
commit b3aa495
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 11 deletions.
diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts
@@ -159,12 +159,11 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
             newFetchStatus = 'loaded';
         }
 
-        // We do a deep equality check if we are subscribed to a collection key and `selector` is defined,
-        // since each `OnyxUtils.tryGetCachedValue()` call will generate a plain new collection object with new records as well,
-        // all of them created using the `selector` function.
+        // We do a deep equality check if `selector` is defined, since each `OnyxUtils.tryGetCachedValue()` call will
+        // generate a plain new primitive/object/array that was created using the `selector` function.
         // For the other cases we will only deal with object reference checks, so just a shallow equality check is enough.
         let areValuesEqual: boolean;
-        if (OnyxUtils.isCollectionKey(key) && selectorRef.current) {
+        if (selectorRef.current) {
             areValuesEqual = deepEqual(previousValueRef.current ?? undefined, newValueRef.current);
         } else {
             areValuesEqual = shallowEqual(previousValueRef.current ?? undefined, newValueRef.current);

diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts
@@ -3,6 +3,7 @@ import type {OnyxEntry} from '../../lib';
 import Onyx, {useOnyx} from '../../lib';
 import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
 import StorageMock from '../../lib/storage';
+import type GenericCollection from '../utils/GenericCollection';
 
 const ONYXKEYS = {
     TEST_KEY: 'test',
@@ -163,12 +164,11 @@ describe('useOnyx', () => {
         });
 
         it('should return selected data from a collection key', async () => {
-            // @ts-expect-error bypass
             Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
-            });
+            } as GenericCollection);
 
             const {result} = renderHook(() =>
                 useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {
@@ -198,30 +198,50 @@ describe('useOnyx', () => {
         it('should not change selected data if a property outside that data was changed', async () => {
             Onyx.set(ONYXKEYS.TEST_KEY, {id: 'test_id', name: 'test_name'});
 
-            const {result} = renderHook(() =>
+            // primitive
+            const {result: primitiveResult} = renderHook(() =>
+                useOnyx(ONYXKEYS.TEST_KEY, {
+                    // @ts-expect-error bypass
+                    selector: (entry: OnyxEntry<{id: string; name: string}>) => entry?.id,
+                }),
+            );
+
+            // object
+            const {result: objectResult} = renderHook(() =>
                 useOnyx(ONYXKEYS.TEST_KEY, {
                     // @ts-expect-error bypass
                     selector: (entry: OnyxEntry<{id: string; name: string}>) => ({id: entry?.id}),
                 }),
             );
 
+            // array
+            const {result: arrayResult} = renderHook(() =>
+                useOnyx(ONYXKEYS.TEST_KEY, {
+                    // @ts-expect-error bypass
+                    selector: (entry: OnyxEntry<{id: string; name: string}>) => [{id: entry?.id}],
+                }),
+            );
+
             await act(async () => waitForPromisesToResolve());
 
-            const oldResult = result.current;
+            const oldPrimitiveResult = primitiveResult.current;
+            const oldObjectResult = objectResult.current;
+            const oldArrayResult = arrayResult.current;
 
             await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, {name: 'test_name_changed'}));
 
             // must be the same reference
-            expect(oldResult).toBe(result.current);
+            expect(oldPrimitiveResult).toBe(primitiveResult.current);
+            expect(oldObjectResult).toBe(objectResult.current);
+            expect(oldArrayResult).toBe(arrayResult.current);
         });
 
         it('should not change selected collection data if a property outside that data was changed', async () => {
-            // @ts-expect-error bypass
             Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, {
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
                 [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
-            });
+            } as GenericCollection);
 
             const {result} = renderHook(() =>
                 useOnyx(ONYXKEYS.COLLECTION.TEST_KEY, {