Merge branch 'main' into bugfix/reassure-flakiness

Author: fabioh8010

lib/Onyx.ts

@@ -44,6 +44,7 @@ function init({
     debugSetState = false,
     enablePerformanceMetrics = false,
     skippableCollectionMemberIDs = [],
+    fullyMergedSnapshotKeys = [],
 }: InitOptions): void {
     if (enablePerformanceMetrics) {
         GlobalSettings.setPerformanceMetricsEnabled(true);
@@ -70,7 +71,7 @@ function init({
         cache.setRecentKeysLimit(maxCachedKeysCount);
     }
 
-    OnyxUtils.initStoreValues(keys, initialKeyStates, evictableKeys);
+    OnyxUtils.initStoreValues(keys, initialKeyStates, evictableKeys, fullyMergedSnapshotKeys);
 
     // Initialize all of our keys with data provided then give green light to any pending connections........
     Promise.all([cache.addEvictableKeysToRecentlyAccessedList(OnyxUtils.isCollectionKey, OnyxUtils.getAllKeys), OnyxUtils.initializeWithDefaultKeyStates()]).then(

lib/OnyxUtils.ts

@@ -2,6 +2,7 @@
 import {deepEqual} from 'fast-equals';
 import lodashClone from 'lodash/clone';
 import type {ValueOf} from 'type-fest';
+import lodashPick from 'lodash/pick';
 import DevTools from './DevTools';
 import * as Logger from './Logger';
 import type Onyx from './Onyx';
@@ -71,6 +72,8 @@ let lastConnectionCallbackData = new Map<number, OnyxValue<OnyxKey>>();
 
 let snapshotKey: OnyxKey | null = null;
 
+let fullyMergedSnapshotKeys: Set<string> | undefined;
+
 // Keeps track of the last subscriptionID that was used so we can keep incrementing it
 let lastSubscriptionID = 0;
 
@@ -132,8 +135,9 @@ function setSkippableCollectionMemberIDs(ids: Set<string>): void {
  * @param keys - `ONYXKEYS` constants object from Onyx.init()
  * @param initialKeyStates - initial data to set when `init()` and `clear()` are called
  * @param evictableKeys - This is an array of keys (individual or collection patterns) that when provided to Onyx are flagged as "safe" for removal.
+ * @param fullyMergedSnapshotKeys - Array of snapshot collection keys where full merge is supported and data structure can be changed after merge.
  */
-function initStoreValues(keys: DeepRecord<string, OnyxKey>, initialKeyStates: Partial<KeyValueMapping>, evictableKeys: OnyxKey[]): void {
+function initStoreValues(keys: DeepRecord<string, OnyxKey>, initialKeyStates: Partial<KeyValueMapping>, evictableKeys: OnyxKey[], fullyMergedSnapshotKeysParam?: string[]): void {
     // We need the value of the collection keys later for checking if a
     // key is a collection. We store it in a map for faster lookup.
     const collectionValues = Object.values(keys.COLLECTION ?? {}) as string[];
@@ -152,6 +156,7 @@ function initStoreValues(keys: DeepRecord<string, OnyxKey>, initialKeyStates: Pa
 
     if (typeof keys.COLLECTION === 'object' && typeof keys.COLLECTION.SNAPSHOT === 'string') {
         snapshotKey = keys.COLLECTION.SNAPSHOT;
+        fullyMergedSnapshotKeys = new Set(fullyMergedSnapshotKeysParam ?? []);
     }
 }
 
@@ -446,8 +451,8 @@ function isCollectionKey(key: OnyxKey): key is CollectionKeyBase {
     return onyxCollectionKeySet.has(key);
 }
 
-function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collectionKey: TCollectionKey, key: string, collectionKeyLength: number): key is `${TCollectionKey}${string}` {
-    return key.startsWith(collectionKey) && key.length > collectionKeyLength;
+function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collectionKey: TCollectionKey, key: string): key is `${TCollectionKey}${string}` {
+    return key.startsWith(collectionKey) && key.length > collectionKey.length;
 }
 
 /**
@@ -461,7 +466,7 @@ function splitCollectionMemberKey<TKey extends CollectionKey, CollectionKeyType
     key: TKey,
     collectionKey?: string,
 ): [CollectionKeyType, string] {
-    if (collectionKey && !isCollectionMemberKey(collectionKey, key, collectionKey.length)) {
+    if (collectionKey && !isCollectionMemberKey(collectionKey, key)) {
         throw new Error(`Invalid '${collectionKey}' collection key provided, it isn't compatible with '${key}' key.`);
     }
 
@@ -556,14 +561,12 @@ function getCachedCollection<TKey extends CollectionKeyBase>(collectionKey: TKey
     const allKeys = collectionMemberKeys || cache.getAllKeys();
     const collection: OnyxCollection<KeyValueMapping[TKey]> = {};
 
-    const collectionKeyLength = collectionKey.length;
-
     // forEach exists on both Set and Array
     allKeys.forEach((key) => {
         // If we don't have collectionMemberKeys array then we have to check whether a key is a collection member key.
         // Because in that case the keys will be coming from `cache.getAllKeys()` and we need to filter out the keys that
         // are not part of the collection.
-        if (!collectionMemberKeys && !isCollectionMemberKey(collectionKey, key, collectionKeyLength)) {
+        if (!collectionMemberKeys && !isCollectionMemberKey(collectionKey, key)) {
             return;
         }
 
@@ -599,7 +602,6 @@ function keysChanged<TKey extends CollectionKeyBase>(
     // individual collection key member for the collection that is being updated. It is important to note that the collection parameter cane be a PARTIAL collection
     // and does not represent all of the combined keys and values for a collection key. It is just the "new" data that was merged in via mergeCollection().
     const stateMappingKeys = Object.keys(callbackToStateMapping);
-    const collectionKeyLength = collectionKey.length;
 
     for (const stateMappingKey of stateMappingKeys) {
         const subscriber = callbackToStateMapping[stateMappingKey];
@@ -620,7 +622,7 @@ function keysChanged<TKey extends CollectionKeyBase>(
         /**
          * e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...});
          */
-        const isSubscribedToCollectionMemberKey = isCollectionMemberKey(collectionKey, subscriber.key, collectionKeyLength);
+        const isSubscribedToCollectionMemberKey = isCollectionMemberKey(collectionKey, subscriber.key);
 
         // Regular Onyx.connect() subscriber found.
         if (typeof subscriber.callback === 'function') {
@@ -1293,12 +1295,21 @@ function subscribeToKey<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKe
             // can send data back to the subscriber. Note that multiple keys can match as a subscriber could either be
             // subscribed to a "collection key" or a single key.
             const matchingKeys: string[] = [];
-            keys.forEach((key) => {
-                if (!isKeyMatch(mapping.key, key)) {
-                    return;
+
+            // Performance optimization: For single key subscriptions, avoid O(n) iteration
+            if (!isCollectionKey(mapping.key)) {
+                if (keys.has(mapping.key)) {
+                    matchingKeys.push(mapping.key);
                 }
-                matchingKeys.push(key);
-            });
+            } else {
+                // Collection case - need to iterate through all keys to find matches (O(n))
+                keys.forEach((key) => {
+                    if (!isKeyMatch(mapping.key, key)) {
+                        return;
+                    }
+                    matchingKeys.push(key);
+                });
+            }
             // If the key being connected to does not exist we initialize the value with null. For subscribers that connected
             // directly via connect() they will simply get a null value sent to them without any information about which key matched
             // since there are none matched. In withOnyx() we wait for all connected keys to return a value before rendering the child
@@ -1380,7 +1391,6 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array<
     const promises: Array<() => Promise<void>> = [];
 
     const snapshotCollection = OnyxUtils.getCachedCollection(snapshotCollectionKey);
-    const snapshotCollectionKeyLength = snapshotCollectionKey.length;
 
     Object.entries(snapshotCollection).forEach(([snapshotEntryKey, snapshotEntryValue]) => {
         // Snapshots may not be present in cache. We don't know how to update them so we skip.
@@ -1392,7 +1402,7 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array<
 
         data.forEach(({key, value}) => {
             // snapshots are normal keys so we want to skip update if they are written to Onyx
-            if (OnyxUtils.isCollectionMemberKey(snapshotCollectionKey, key, snapshotCollectionKeyLength)) {
+            if (OnyxUtils.isCollectionMemberKey(snapshotCollectionKey, key)) {
                 return;
             }
 
@@ -1416,8 +1426,17 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array<
             }
 
             const oldValue = updatedData[key] || {};
+            let collectionKey: string | undefined;
+            try {
+                collectionKey = getCollectionKey(key);
+            } catch (e) {
+                // If getCollectionKey() throws an error it means the key is not a collection key.
+                collectionKey = undefined;
+            }
+            const shouldFullyMerge = fullyMergedSnapshotKeys?.has(collectionKey || key);
+            const newValue = shouldFullyMerge ? value : lodashPick(value, Object.keys(snapshotData[key]));
 
-            updatedData = {...updatedData, [key]: Object.assign(oldValue, value)};
+            updatedData = {...updatedData, [key]: Object.assign(oldValue, newValue)};
         });
 
         // Skip the update if there's no data to be merged

lib/types.ts

@@ -480,6 +480,14 @@ type InitOptions = {
      * Additionally, any subscribers from these keys to won't receive any data from Onyx.
      */
     skippableCollectionMemberIDs?: string[];
+
+    /**
+     * Array of snapshot collection keys where full merge is supported and data structure can be changed after merge.
+     * For e.g. if oldSnapshotData is {report_1: {name 'Fitsum'}} and BE update is {report_1: {name:'Fitsum2', nickName:'Fitse'}}
+     * if it is fullyMergedSnapshotkey the `nickName` prop that didn't exist in the previous data will be merged
+     * otherwise only existing prop will be picked from the BE update and merged (in this case only name).
+     */
+    fullyMergedSnapshotKeys?: string[];
 };
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any

lib/useOnyx.ts

@@ -143,11 +143,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(
             const previousCollectionKey = OnyxUtils.splitCollectionMemberKey(previousKey)[0];
             const collectionKey = OnyxUtils.splitCollectionMemberKey(key)[0];
 
-            if (
-                OnyxUtils.isCollectionMemberKey(previousCollectionKey, previousKey, previousCollectionKey.length) &&
-                OnyxUtils.isCollectionMemberKey(collectionKey, key, collectionKey.length) &&
-                previousCollectionKey === collectionKey
-            ) {
+            if (OnyxUtils.isCollectionMemberKey(previousCollectionKey, previousKey) && OnyxUtils.isCollectionMemberKey(collectionKey, key) && previousCollectionKey === collectionKey) {
                 return;
             }
         } catch (e) {

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "react-native-onyx",
-  "version": "2.0.111",
+  "version": "2.0.113",
   "author": "Expensify, Inc.",
   "homepage": "https://expensify.com",
   "description": "State management for React Native",

package.json

@@ -154,14 +154,12 @@ describe('OnyxUtils', () => {
     });
 
     describe('isCollectionMemberKey', () => {
-        const collectionKeyLength = collectionKey.length;
-
         test('one call with correct key', async () => {
-            await measureFunction(() => OnyxUtils.isCollectionMemberKey(collectionKey, `${collectionKey}entry1`, collectionKeyLength));
+            await measureFunction(() => OnyxUtils.isCollectionMemberKey(collectionKey, `${collectionKey}entry1`));
         });
 
         test('one call with wrong key', async () => {
-            await measureFunction(() => OnyxUtils.isCollectionMemberKey(collectionKey, `${ONYXKEYS.COLLECTION.TEST_KEY_2}entry1`, collectionKeyLength));
+            await measureFunction(() => OnyxUtils.isCollectionMemberKey(collectionKey, `${ONYXKEYS.COLLECTION.TEST_KEY_2}entry1`));
         });
     });
 

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

@@ -31,6 +31,7 @@ Onyx.init({
         [ONYX_KEYS.KEY_WITH_UNDERSCORE]: 'default',
     },
     skippableCollectionMemberIDs: ['skippable-id'],
+    fullyMergedSnapshotKeys: [ONYX_KEYS.COLLECTION.ANIMALS, ONYX_KEYS.OTHER_TEST],
 });
 
 describe('Onyx', () => {
@@ -1445,13 +1446,19 @@ describe('Onyx', () => {
 
     it('should update Snapshot when its data changed', async () => {
         const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`;
+        const people = `${ONYX_KEYS.COLLECTION.PEOPLE}1`;
         const snapshot1 = `${ONYX_KEYS.COLLECTION.SNAPSHOT}1`;
 
         const initialValue = {name: 'Fluffy'};
-        const finalValue = {name: 'Kitty', nickName: 'Fitse'};
+        const initialValueOtherTest = {1: {name: 'Kitty'}};
+        const finalValuePeople = {name: 'Kitty'};
+        const finalValueOtherTest = {1: {name: 'First person'}, 2: {name: 'Second person'}};
+        const finalValueCat = {name: 'Kitty', nickName: 'Fitse'};
+        const onyxUpdate = {name: 'Kitty', nickName: 'Fitse'};
 
         await Onyx.set(cat, initialValue);
-        await Onyx.set(snapshot1, {data: {[cat]: initialValue}});
+        await Onyx.set(people, initialValue);
+        await Onyx.set(snapshot1, {data: {[ONYX_KEYS.OTHER_TEST]: initialValueOtherTest, [cat]: initialValue, [people]: initialValue}});
 
         const callback = jest.fn();
 
@@ -1462,11 +1469,15 @@ describe('Onyx', () => {
 
         await waitForPromisesToResolve();
 
-        await Onyx.update([{key: cat, value: finalValue, onyxMethod: Onyx.METHOD.MERGE}]);
+        await Onyx.update([
+            {key: cat, value: onyxUpdate, onyxMethod: Onyx.METHOD.MERGE},
+            {key: people, value: onyxUpdate, onyxMethod: Onyx.METHOD.MERGE},
+            {key: ONYX_KEYS.OTHER_TEST, value: finalValueOtherTest, onyxMethod: Onyx.METHOD.MERGE},
+        ]);
 
         expect(callback).toBeCalledTimes(2);
-        expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue}}, snapshot1);
-        expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: finalValue}}, snapshot1);
+        expect(callback).toHaveBeenNthCalledWith(1, {data: {[cat]: initialValue, [ONYX_KEYS.OTHER_TEST]: initialValueOtherTest, [people]: initialValue}}, snapshot1);
+        expect(callback).toHaveBeenNthCalledWith(2, {data: {[cat]: finalValueCat, [ONYX_KEYS.OTHER_TEST]: finalValueOtherTest, [people]: finalValuePeople}}, snapshot1);
     });
 
     describe('update', () => {