Deep compare arrays on merge and return old array if unchanged

lib/OnyxCache.ts

@@ -24,6 +24,8 @@ const FROZEN_EMPTY_COLLECTION: Readonly<NonUndefined<OnyxCollection<KeyValueMapp
 const CACHE_MERGE_OPTIONS: FastMergeOptions = {
     shouldRemoveNestedNulls: true,
     objectRemovalMode: 'replace',
+    // Keeps cached references of deep-equal arrays, so unchanged merges are no-ops.
+    shouldKeepEqualArrays: true,
 };
 
 // Task constants

lib/utils.ts

@@ -1,3 +1,4 @@
+import {deepEqual} from 'fast-equals';
 import type {OnyxInput, OnyxKey} from './types';
 
 type EmptyObject = Record<string, never>;
@@ -22,6 +23,12 @@ type FastMergeOptions = {
      * If set to "replace", we will completely replace the marked objects with the new value instead of merging them.
      */
     objectRemovalMode?: 'mark' | 'replace' | 'none';
+
+    /**
+     * If true, a source array deep-equal to the target array keeps the target reference, so unchanged merges return the target.
+     * Arrays inside an object are only compared when nothing else in it changed. Costs a deep comparison per array.
+     */
+    shouldKeepEqualArrays?: boolean;
 };
 
 type FastMergeMetadata = {
@@ -56,12 +63,15 @@ function fastMerge<TValue>(target: TValue, source: TValue, options?: FastMergeOp
     // otherwise "mergeObject" will throw an error,
     // because it expects an object as "source"
     if (!isMergeableObject(source)) {
-        return {result: source, replaceNullPatches: metadata.replaceNullPatches};
+        // Arrays are replaced, not merged, but a deep-equal source array keeps the target reference.
+        const result = options?.shouldKeepEqualArrays && areDeepEqualArrays(target, source) ? target : source;
+        return {result, replaceNullPatches: metadata.replaceNullPatches};
     }
 
     const optionsWithDefaults: FastMergeOptions = {
         shouldRemoveNestedNulls: options?.shouldRemoveNestedNulls ?? false,
         objectRemovalMode: options?.objectRemovalMode ?? 'none',
+        shouldKeepEqualArrays: options?.shouldKeepEqualArrays ?? false,
     };
 
     const mergedValue = mergeObject(target, source as Record<string, unknown>, optionsWithDefaults, metadata, basePath) as TValue;
@@ -93,6 +103,10 @@ function mergeObject<TObject extends Record<string, unknown>>(
     // If nothing changed, we return the original target reference for reference stability.
     let hasChanged = !targetObject;
 
+    // Keys whose array was replaced. Compared at the end, and only if nothing else changed,
+    // since any other change gives the object a new reference anyway.
+    let replacedArrayKeys: string[] | undefined;
+
     // First we want to copy over all keys from the target into the destination object,
     // in case "target" is a mergable object.
     // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object
@@ -130,8 +144,14 @@ function mergeObject<TObject extends Record<string, unknown>>(
 
         // If the source value is not a mergable object, we need to set the key directly.
         if (!isMergeableObject(sourceProperty)) {
-            if (destination[key] !== sourceProperty) {
-                hasChanged = true;
+            const destinationProperty = destination[key];
+            if (destinationProperty !== sourceProperty) {
+                if (options.shouldKeepEqualArrays && !hasChanged && Array.isArray(destinationProperty) && Array.isArray(sourceProperty)) {
+                    replacedArrayKeys ??= [];
+                    replacedArrayKeys.push(key);
+                } else {
+                    hasChanged = true;
+                }
             }
             destination[key] = sourceProperty;
             continue;
@@ -167,7 +187,20 @@ function mergeObject<TObject extends Record<string, unknown>>(
         destination[key] = merged;
     }
 
-    return hasChanged ? (destination as TObject) : (targetObject as TObject);
+    if (hasChanged) {
+        return destination as TObject;
+    }
+
+    // Only arrays were replaced. If they are all deep-equal, the merge is a no-op.
+    if (replacedArrayKeys) {
+        for (const key of replacedArrayKeys) {
+            if (!deepEqual(targetObject?.[key], destination[key])) {
+                return destination as TObject;
+            }
+        }
+    }
+
+    return targetObject as TObject;
 }
 
 /** Checks whether the given object is an object and not null/undefined. */
@@ -196,6 +229,11 @@ function isMergeableObject<TObject extends Record<string, unknown>>(value: unkno
     return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value);
 }
 
+/** Checks whether both values are arrays with deep-equal contents. */
+function areDeepEqualArrays(a: unknown, b: unknown): boolean {
+    return Array.isArray(a) && Array.isArray(b) && deepEqual(a, b);
+}
+
 /**
  * Reports whether a value needs cleaning (nested null/undefined, or the replace-object mark) before it's
  * safe to store by reference. Read-only, non-allocating.

tests/perf-test/Onyx.perf-test.ts

@@ -2,6 +2,7 @@ import {measureAsyncFunction} from 'reassure';
 import type {OnyxKey, OnyxUpdate} from '../../lib';
 import Onyx from '../../lib';
 import createRandomReportAction, {getRandomReportActions} from '../utils/collections/reportActions';
+import {createObjectArrayCollection} from '../utils/collections/objectArrays';
 import type GenericCollection from '../utils/GenericCollection';
 import OnyxUtils from '../../lib/OnyxUtils';
 import createDeferredTask from '../../lib/createDeferredTask';
@@ -82,6 +83,35 @@ describe('Onyx', () => {
                 afterEach: clearOnyxAfterEachMeasure,
             });
         });
+
+        test('one call with 1.5k deep-equal arrays and a collection subscriber that recomputes changed members', async () => {
+            const arraysCollectionKey = ONYXKEYS.COLLECTION.TEST_KEY_2;
+            const arrays = createObjectArrayCollection(arraysCollectionKey, 1500);
+            const sameArrays = createObjectArrayCollection(arraysCollectionKey, 1500);
+
+            // Stand-in for a subscriber that recomputes every member whose reference changed.
+            let previousCollection: Record<string, unknown> | undefined;
+            const connection = Onyx.connect({
+                key: arraysCollectionKey,
+                callback: (collection) => {
+                    for (const key of Object.keys(collection ?? {})) {
+                        if (collection[key] !== previousCollection?.[key]) {
+                            JSON.stringify(collection[key]);
+                        }
+                    }
+                    previousCollection = collection;
+                },
+            });
+
+            await measureAsyncFunction(() => Onyx.mergeCollection(arraysCollectionKey, sameArrays), {
+                beforeEach: async () => {
+                    await Onyx.mergeCollection(arraysCollectionKey, arrays);
+                },
+                afterEach: clearOnyxAfterEachMeasure,
+            });
+
+            connection.unsubscribe();
+        });
     });
 
     describe('setCollection', () => {

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

@@ -1,10 +1,11 @@
-import {screen} from '@testing-library/react-native';
+import {act, screen} from '@testing-library/react-native';
 import React from 'react';
 import {Text, View} from 'react-native';
 import {measureRenders} from 'reassure';
 import type {FetchStatus, OnyxEntry, OnyxKey, OnyxValue, ResultMetadata, UseOnyxOptions} from '../../lib';
 import Onyx, {useOnyx} from '../../lib';
 import type {UseOnyxSelector} from '../../lib/useOnyx';
+import {createObjectArrayCollection} from '../utils/collections/objectArrays';
 
 const ONYXKEYS = {
     TEST_KEY: 'test',
@@ -394,6 +395,30 @@ describe('useOnyx', () => {
             );
         });
 
+        /**
+         * Expected renders: 1.
+         *
+         * Merging deep-equal members (arrays included) keeps the cached references, so a subscriber without a selector doesn't re-render.
+         */
+        test('merging deep-equal members with arrays does not re-render', async () => {
+            const key = ONYXKEYS.COLLECTION.TEST_KEY;
+            await measureRenders(<UseOnyxWrapper onyxKey={key} />, {
+                beforeEach: async () => {
+                    await Onyx.mergeCollection(key, createObjectArrayCollection(key, 3));
+                },
+                scenario: async () => {
+                    await screen.findByText(dataMatcher(key, createObjectArrayCollection(key, 3)));
+
+                    await act(async () => {
+                        await Onyx.mergeCollection(key, createObjectArrayCollection(key, 3));
+                    });
+
+                    await screen.findByText(dataMatcher(key, createObjectArrayCollection(key, 3)));
+                },
+                afterEach: clearOnyxAfterEachMeasure,
+            });
+        });
+
         /**
          * Expected renders: 1.
          *

tests/perf-test/utils.perf-test.ts

@@ -2,6 +2,7 @@ import {measureFunction} from 'reassure';
 import Onyx from '../../lib';
 import utils from '../../lib/utils';
 import createRandomReportAction, {getRandomReportActions} from '../utils/collections/reportActions';
+import {createObjectArray, createObjectArrayCollection} from '../utils/collections/objectArrays';
 
 const ONYXKEYS = {
     COLLECTION: {
@@ -31,6 +32,53 @@ describe('utils', () => {
         });
     });
 
+    describe('fastMerge collection of arrays', () => {
+        const COLLECTION_SIZE = 1500;
+
+        const createCollection = (createMember?: typeof createObjectArray) => createObjectArrayCollection(collectionKey, COLLECTION_SIZE, createMember);
+
+        // Merges every member separately, like OnyxCache.merge() does.
+        const mergeCollection = (target: Record<string, unknown[]>, source: Record<string, unknown[]>) => {
+            for (const key of Object.keys(source)) {
+                utils.fastMerge(target[key], source[key], {shouldRemoveNestedNulls: true, objectRemovalMode: 'replace', shouldKeepEqualArrays: true});
+            }
+        };
+
+        const target = createCollection();
+
+        test('all members deep-equal', async () => {
+            const source = createCollection();
+            await measureFunction(() => mergeCollection(target, source));
+        });
+
+        test('one member different', async () => {
+            const source = createCollection((index) => createObjectArray(index, undefined, index === 0 ? 0 : undefined));
+            await measureFunction(() => mergeCollection(target, source));
+        });
+
+        test('all members different in the first element', async () => {
+            const source = createCollection((index) => createObjectArray(index, undefined, 0));
+            await measureFunction(() => mergeCollection(target, source));
+        });
+
+        test('all members different in the last element', async () => {
+            const source = createCollection((index) => createObjectArray(index, undefined, index % 3));
+            await measureFunction(() => mergeCollection(target, source));
+        });
+
+        test('all members long and different only in the first element', async () => {
+            const longTarget = createCollection((index) => createObjectArray(index, 50));
+            const source = createCollection((index) => createObjectArray(index, 50, 0));
+            await measureFunction(() => mergeCollection(longTarget, source));
+        });
+
+        test('all members long and different only in the last element', async () => {
+            const longTarget = createCollection((index) => createObjectArray(index, 50));
+            const source = createCollection((index) => createObjectArray(index, 50, 49));
+            await measureFunction(() => mergeCollection(longTarget, source));
+        });
+    });
+
     describe('formatActionName', () => {
         test('one call', async () => {
             await measureFunction(() => utils.formatActionName(Onyx.METHOD.SET, ONYXKEYS.COLLECTION.TEST_KEY));

tests/unit/onyxCacheTest.tsx

@@ -970,6 +970,37 @@ describe('Onyx', () => {
                 expect(before).toBe(after);
             });
 
+            it('should keep the cached array reference and not mark collection dirty when merged array is deep-equal', async () => {
+                await initOnyx();
+                const memberKey = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`;
+                await Onyx.set(memberKey, [{name: 'item', data: {value: 1}}]);
+
+                const cachedValue = cache.get(memberKey);
+                const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION);
+
+                // Merge a new, deep-equal array
+                cache.merge({[memberKey]: [{name: 'item', data: {value: 1}}]});
+
+                expect(cache.get(memberKey)).toBe(cachedValue);
+                // A rebuild would reuse the snapshot too, so check the dirty set directly.
+                expect((cache as unknown as {dirtyCollections: Set<string>}).dirtyCollections.has(ONYX_KEYS.COLLECTION.MOCK_COLLECTION)).toBe(false);
+                expect(cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION)).toBe(before);
+            });
+
+            it('should replace the cached array when merged array differs', async () => {
+                await initOnyx();
+                const memberKey = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`;
+                await Onyx.set(memberKey, [{name: 'item', data: {value: 1}}]);
+
+                const before = cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION);
+                const newValue = [{name: 'item', data: {value: 2}}];
+
+                cache.merge({[memberKey]: newValue});
+
+                expect(cache.get(memberKey)).toBe(newValue);
+                expect(cache.getCollectionData(ONYX_KEYS.COLLECTION.MOCK_COLLECTION)).not.toBe(before);
+            });
+
             it('should mark collection dirty when a member value changes', async () => {
                 await initOnyx();
                 await Onyx.set(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1});

tests/unit/onyxTest.ts

@@ -1542,6 +1542,43 @@ describe('Onyx', () => {
             });
     });
 
+    it('should not call collection subscribers when every merged collection member is deep-equal and contains arrays', async () => {
+        const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`;
+        const dog = `${ONYX_KEYS.COLLECTION.ANIMALS}dog`;
+        const createCollection = (): GenericCollection => ({
+            [cat]: [{name: 'Fluffy', tags: ['small']}],
+            [dog]: {name: 'Rex', tags: [{id: 1}, {id: 2}]},
+        });
+
+        await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ANIMALS, createCollection());
+
+        const collectionCallback = jest.fn();
+        const catCallback = jest.fn();
+        const connections = [Onyx.connect({key: ONYX_KEYS.COLLECTION.ANIMALS, callback: collectionCallback}), Onyx.connect({key: cat, callback: catCallback})];
+        await waitForPromisesToResolve();
+        expect(collectionCallback).toHaveBeenCalledTimes(1);
+        expect(catCallback).toHaveBeenCalledTimes(1);
+
+        const cachedCat = cache.get(cat);
+        const cachedDog = cache.get(dog);
+
+        // When the same collection is merged again with new, deep-equal values
+        await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.ANIMALS, createCollection());
+
+        // And then merged through Onyx.update, which batches members of the same collection
+        await Onyx.update(Object.entries(createCollection()).map(([key, value]) => ({onyxMethod: Onyx.METHOD.MERGE, key, value})) as Array<OnyxUpdate<OnyxKey>>);
+
+        // Then the cached values keep their references and no subscriber is called again
+        expect(cache.get(cat)).toBe(cachedCat);
+        expect(cache.get(dog)).toBe(cachedDog);
+        expect(collectionCallback).toHaveBeenCalledTimes(1);
+        expect(catCallback).toHaveBeenCalledTimes(1);
+
+        for (const id of connections) {
+            id.unsubscribe();
+        }
+    });
+
     it('should update Snapshot when its data changed', async () => {
         const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`;
         const snapshot1 = `${ONYX_KEYS.COLLECTION.SNAPSHOT}1`;