Merge branch 'main' into feat/selector-collection-state

Expensify · Sep 30, 2024 · 5b94e00 · 5b94e00
2 parents fbb4a8c + 81cbefa
commit 5b94e00
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 18 deletions.
diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts
@@ -88,19 +88,25 @@ class OnyxConnectionManager {
      * according to their purpose and effect they produce in the Onyx connection.
      */
     private generateConnectionID<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKey>): string {
+        const {key, initWithStoredValues, reuseConnection, waitForCollectionCallback} = connectOptions;
         let suffix = '';
 
         // We will generate a unique ID in any of the following situations:
-        // - `connectOptions.reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused.
-        // - `connectOptions.initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused.
+        // - `reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused.
+        // - `initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused.
+        // - `key` is a collection key AND `waitForCollectionCallback` is `undefined/false`. This combination needs a new connection at every subscription
+        //   in order to send all the collection entries, so the connection can't be reused.
         // - `withOnyxInstance` is defined inside `connectOptions`. That means the subscriber is a `withOnyx` HOC and therefore doesn't support connection reuse.
-        if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || utils.hasWithOnyxInstance(connectOptions)) {
+        if (
+            reuseConnection === false ||
+            initWithStoredValues === false ||
+            (!utils.hasWithOnyxInstance(connectOptions) && OnyxUtils.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false)) ||
+            utils.hasWithOnyxInstance(connectOptions)
+        ) {
             suffix += `,uniqueID=${Str.guid()}`;
         }
 
-        return `onyxKey=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${
-            connectOptions.waitForCollectionCallback ?? false
-        }${suffix}`;
+        return `onyxKey=${key},initWithStoredValues=${initWithStoredValues ?? true},waitForCollectionCallback=${waitForCollectionCallback ?? false}${suffix}`;
     }
 
     /**

diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts
@@ -177,6 +177,11 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
     }, [key, options?.canEvict]);
 
     const getSnapshot = useCallback(() => {
+        // We return the initial result right away during the first connection if `initWithStoredValues` is set to `false`.
+        if (isFirstConnectionRef.current && options?.initWithStoredValues === false) {
+            return resultRef.current;
+        }
+
         // We get the value from cache while the first connection to Onyx is being made,
         // so we can return any cached value right away. After the connection is made, we only
         // update `newValueRef` when `Onyx.connect()` callback is fired.
@@ -234,7 +239,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
         }
 
         return resultRef.current;
-    }, [key, selectorRef, options?.allowStaleData, options?.initialValue]);
+    }, [key, selectorRef, options?.initWithStoredValues, options?.allowStaleData, options?.initialValue]);
 
     const subscribe = useCallback(
         (onStoreChange: () => void) => {

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "react-native-onyx",
-  "version": "2.0.69",
+  "version": "2.0.71",
   "author": "Expensify, Inc.",
   "homepage": "https://expensify.com",
   "description": "State management for React Native",

diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts
@@ -271,6 +271,43 @@ describe('OnyxConnectionManager', () => {
             expect(connectionsMap.size).toEqual(0);
         });
 
+        it("should create a separate connection to the same key when it's a collection one and waitForCollectionCallback is undefined/false", async () => {
+            const collection = {
+                [`${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'},
+            };
+
+            Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, collection as GenericCollection);
+
+            await act(async () => waitForPromisesToResolve());
+
+            const callback1 = jest.fn();
+            const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: undefined, callback: callback1});
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(callback1).toHaveBeenCalledTimes(3);
+            expect(callback1).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`);
+            expect(callback1).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`);
+            expect(callback1).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`);
+
+            const callback2 = jest.fn();
+            const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: false, callback: callback2});
+
+            expect(connection1.id).not.toEqual(connection2.id);
+            expect(connectionsMap.size).toEqual(2);
+            expect(connectionsMap.has(connection1.id)).toBeTruthy();
+            expect(connectionsMap.has(connection2.id)).toBeTruthy();
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(callback2).toHaveBeenCalledTimes(3);
+            expect(callback2).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`);
+            expect(callback2).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`);
+            expect(callback2).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`);
+        });
+
         it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => {
             expect(connectionsMap.size).toEqual(0);
 

diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts
@@ -467,7 +467,7 @@ describe('useOnyx', () => {
             expect(result.current[1].status).toEqual('loaded');
         });
 
-        it('should return initial value and loaded state, and after merge return updated value and loaded state', async () => {
+        it('should return `undefined` and loaded state if using `initialValue`, and after merge return updated value and loaded state', async () => {
             await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test1');
 
             const {result} = renderHook(() =>
@@ -479,16 +479,16 @@ describe('useOnyx', () => {
 
             await act(async () => waitForPromisesToResolve());
 
-            expect(result.current[0]).toEqual('initial value');
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loaded');
 
-            await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'));
+            await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test'));
 
-            expect(result.current[0]).toEqual('test2');
+            expect(result.current[0]).toEqual('test');
             expect(result.current[1].status).toEqual('loaded');
         });
 
-        it('should return selected value and loaded state, and after merge return updated selected value and loaded state', async () => {
+        it('should return `undefined` value and loaded state if using `selector`, and after merge return selected value and loaded state', async () => {
             await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test1');
 
             const {result} = renderHook(() =>
@@ -501,12 +501,12 @@ describe('useOnyx', () => {
 
             await act(async () => waitForPromisesToResolve());
 
-            expect(result.current[0]).toEqual('undefined_selected');
+            expect(result.current[0]).toBeUndefined();
             expect(result.current[1].status).toEqual('loaded');
 
-            await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'));
+            await act(async () => Onyx.merge(ONYXKEYS.TEST_KEY, 'test'));
 
-            expect(result.current[0]).toEqual('test2_selected');
+            expect(result.current[0]).toEqual('test_selected');
             expect(result.current[1].status).toEqual('loaded');
         });
     });