Merge branch 'feature/structural-sharing-cache-pr-2' into feature/structural-sharing-cache-pr-3

Author: fabioh8010

lib/utils.ts

@@ -89,6 +89,10 @@ function mergeObject<TObject extends Record<string, unknown>>(
 
     const targetObject = isMergeableObject(target) ? target : undefined;
 
+    // Track whether the merge actually changed anything compared to target.
+    // If nothing changed, we return the original target reference for reference stability.
+    let hasChanged = !targetObject;
+
     // 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
@@ -103,6 +107,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
             const shouldOmitNullishProperty = options.shouldRemoveNestedNulls && (targetProperty === null || sourceProperty === null);
 
             if (targetProperty === undefined || shouldOmitNullishProperty) {
+                hasChanged = true;
                 continue;
             }
 
@@ -125,6 +130,9 @@ 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;
+            }
             destination[key] = sourceProperty;
             continue;
         }
@@ -134,6 +142,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
         // To achieve this, we first mark these nested objects with an internal flag.
         // When calling fastMerge again with "mark" removal mode, the marked objects will be removed.
         if (options.objectRemovalMode === 'mark' && targetProperty === null) {
+            hasChanged = true;
             targetProperty = {[ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true};
             metadata.replaceNullPatches.push([[...basePath, key], {...sourceProperty}]);
         }
@@ -142,6 +151,7 @@ function mergeObject<TObject extends Record<string, unknown>>(
         // has the internal flag set, we replace the entire destination object with the source one and remove
         // the flag.
         if (options.objectRemovalMode === 'replace' && sourceProperty[ONYX_INTERNALS__REPLACE_OBJECT_MARK]) {
+            hasChanged = true;
             // We do a spread here in order to have a new object reference and allow us to delete the internal flag
             // of the merged object only.
             const sourcePropertyWithoutMark = {...sourceProperty};
@@ -150,10 +160,14 @@ function mergeObject<TObject extends Record<string, unknown>>(
             continue;
         }
 
-        destination[key] = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result;
+        const merged = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result;
+        if (merged !== targetProperty) {
+            hasChanged = true;
+        }
+        destination[key] = merged;
     }
 
-    return destination as TObject;
+    return hasChanged ? (destination as TObject) : (targetObject as TObject);
 }
 
 /** Checks whether the given object is an object and not null/undefined. */
@@ -170,35 +184,36 @@ function isMergeableObject<TObject extends Record<string, unknown>>(value: unkno
     return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value);
 }
 
-/** Deep removes the nested null values from the given value. */
+/** Deep removes the nested null values from the given value. Returns the original reference if no nulls were found. */
 function removeNestedNullValues<TValue extends OnyxInput<OnyxKey> | null>(value: TValue): TValue {
-    if (value === null || value === undefined || typeof value !== 'object') {
+    if (value === null || value === undefined || typeof value !== 'object' || Array.isArray(value)) {
         return value;
     }
 
-    if (Array.isArray(value)) {
-        return [...value] as TValue;
-    }
-
+    let hasChanged = false;
     const result: Record<string, unknown> = {};
 
     // eslint-disable-next-line no-restricted-syntax, guard-for-in
     for (const key in value) {
         const propertyValue = value[key];
 
         if (propertyValue === null || propertyValue === undefined) {
+            hasChanged = true;
             continue;
         }
 
         if (typeof propertyValue === 'object' && !Array.isArray(propertyValue)) {
-            const valueWithoutNestedNulls = removeNestedNullValues(propertyValue);
-            result[key] = valueWithoutNestedNulls;
+            const cleaned = removeNestedNullValues(propertyValue);
+            if (cleaned !== propertyValue) {
+                hasChanged = true;
+            }
+            result[key] = cleaned;
         } else {
             result[key] = propertyValue;
         }
     }
 
-    return result as TValue;
+    return hasChanged ? (result as TValue) : value;
 }
 
 /** Formats the action name by uppercasing and adding the key if provided. */

tests/unit/fastMergeTest.ts tests/unit/utilsTest.tstests/unit/fastMergeTest.ts renamed to tests/unit/utilsTest.ts

@@ -209,5 +209,129 @@ describe('fastMerge', () => {
             const result = utils.fastMerge<unknown>(data, testObject);
             expect(result.result).toEqual(testObject);
         });
+
+        it('should return the same reference when source values match target', () => {
+            const target = {a: 1, b: 'hello', c: true};
+            const source = {a: 1, b: 'hello'};
+            const result = utils.fastMerge(target, source);
+            expect(result.result).toBe(target);
+        });
+
+        it('should return a new reference when source adds a key', () => {
+            const target = {a: 1};
+            const source = {a: 1, b: 2};
+            const result = utils.fastMerge(target, source);
+            expect(result.result).not.toBe(target);
+            expect(result.result).toEqual({a: 1, b: 2});
+        });
+
+        it('should return a new reference when source changes a value', () => {
+            const target = {a: 1, b: 2};
+            const source = {b: 3};
+            const result = utils.fastMerge(target, source);
+            expect(result.result).not.toBe(target);
+            expect(result.result).toEqual({a: 1, b: 3});
+        });
+
+        it('should preserve nested object references when unchanged', () => {
+            const nested = {x: 1, y: 2};
+            const target = {a: 'hello', b: nested};
+            const source = {a: 'hello'};
+            const result = utils.fastMerge<GenericDeepRecord>(target, source);
+            expect(result.result).toBe(target);
+            expect(result.result.b).toBe(nested);
+        });
+
+        it('should preserve unchanged nested references when sibling changes', () => {
+            const nested = {x: 1, y: 2};
+            const target = {a: nested, b: 'old'};
+            const source = {b: 'new'};
+            const result = utils.fastMerge<GenericDeepRecord>(target, source);
+            expect(result.result).not.toBe(target);
+            expect(result.result.a).toBe(nested);
+        });
+
+        it('should return a new reference when nested object changes', () => {
+            const target = {a: {x: 1, y: 2}, b: 'hello'};
+            const source = {a: {x: 99}};
+            const result = utils.fastMerge(target, source);
+            expect(result.result).not.toBe(target);
+            expect(result.result.a).not.toBe(target.a);
+            expect(result.result.a).toEqual({x: 99, y: 2});
+        });
+
+        it('should return a new reference when shouldRemoveNestedNulls removes a key', () => {
+            const target = {a: 1, b: null};
+            const source = {a: 1};
+            const result = utils.fastMerge(target, source, {shouldRemoveNestedNulls: true});
+            expect(result.result).not.toBe(target);
+            expect(result.result).toEqual({a: 1});
+        });
+
+        it('should return the same reference when merging with empty source keys', () => {
+            const target = {a: 1, b: 2};
+            const source = {};
+            const result = utils.fastMerge(target, source);
+            expect(result.result).toBe(target);
+        });
+    });
+
+    describe('removeNestedNullValues', () => {
+        it('should return the same reference when no nulls exist', () => {
+            const value = {a: 1, b: 'hello', c: true};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).toBe(value);
+        });
+
+        it('should return the same reference for nested objects without nulls', () => {
+            const nested = {x: 1, y: 2};
+            const value = {a: 'hello', b: nested};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).toBe(value);
+            expect((result as Record<string, unknown>).b).toBe(nested);
+        });
+
+        it('should return a new reference when a null property is removed', () => {
+            const value = {a: 1, b: null};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).not.toBe(value);
+            expect(result).toEqual({a: 1});
+        });
+
+        it('should return a new reference when an undefined property is removed', () => {
+            const value = {a: 1, b: undefined};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).not.toBe(value);
+            expect(result).toEqual({a: 1});
+        });
+
+        it('should return a new reference when a deeply nested null is removed', () => {
+            const value = {a: {b: {c: null, d: 1}}};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).not.toBe(value);
+            expect(result).toEqual({a: {b: {d: 1}}});
+        });
+
+        it('should preserve sibling references when a nested null is removed', () => {
+            const sibling = {x: 1};
+            const value = {a: sibling, b: {c: null}};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).not.toBe(value);
+            expect((result as Record<string, unknown>).a).toBe(sibling);
+        });
+
+        it('should return the same array reference', () => {
+            const arr = [1, 2, 3];
+            const result = utils.removeNestedNullValues(arr);
+            expect(result).toBe(arr);
+        });
+
+        it('should return the same reference for objects containing arrays', () => {
+            const arr = ['a', 'b'];
+            const value = {items: arr, count: 2};
+            const result = utils.removeNestedNullValues(value);
+            expect(result).toBe(value);
+            expect((result as Record<string, unknown>).items).toBe(arr);
+        });
     });
 });