diff --git a/lib/utils.ts b/lib/utils.ts index d057bf418..1fca1c8a4 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -89,6 +89,10 @@ function mergeObject>( 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>( const shouldOmitNullishProperty = options.shouldRemoveNestedNulls && (targetProperty === null || sourceProperty === null); if (targetProperty === undefined || shouldOmitNullishProperty) { + hasChanged = true; continue; } @@ -125,6 +130,9 @@ function mergeObject>( // 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>( // 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>( // 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>( 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,16 +184,13 @@ function isMergeableObject>(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 | 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 = {}; // eslint-disable-next-line no-restricted-syntax, guard-for-in @@ -187,18 +198,22 @@ function removeNestedNullValues | null>(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. */ diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts deleted file mode 100644 index edcbdfab1..000000000 --- a/tests/unit/fastMergeTest.ts +++ /dev/null @@ -1,213 +0,0 @@ -import utils from '../../lib/utils'; -import type {GenericDeepRecord} from '../types'; - -type DeepObject = GenericDeepRecord | unknown[]; - -const testObject: DeepObject = { - a: 'a', - b: { - c: 'c', - d: { - e: 'e', - f: 'f', - }, - g: 'g', - }, -}; - -const testObjectWithNullishValues: DeepObject = { - a: undefined, - b: { - c: { - h: 'h', - }, - d: { - e: null, - }, - }, -}; - -const testObjectWithNullValuesRemoved: DeepObject = { - b: { - c: { - h: 'h', - }, - d: {}, - }, -}; - -const testMergeChanges: DeepObject[] = [ - { - b: { - d: { - h: 'h', - }, - }, - }, - { - b: { - d: null, - h: 'h', - }, - }, -]; - -describe('fastMerge', () => { - describe('primitives', () => { - it('should replace strings', () => { - const result = utils.fastMerge('old', 'new'); - expect(result.result).toEqual('new'); - }); - - it('should replace numbers', () => { - const result = utils.fastMerge(1000, 1001); - expect(result.result).toEqual(1001); - }); - - it('should replace booleans', () => { - const result = utils.fastMerge(true, false); - expect(result.result).toEqual(false); - }); - }); - - describe('arrays', () => { - it('should replace arrays', () => { - const result = utils.fastMerge(['a', 1, true], ['b', false]); - expect(result.result).toEqual(['b', false]); - }); - }); - - describe('objects', () => { - it('should merge an object with another object and remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); - - expect(result.result).toEqual({ - a: 'a', - b: { - c: { - h: 'h', - }, - d: { - f: 'f', - }, - g: 'g', - }, - }); - }); - - it('should merge an object with another object and not remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues); - - expect(result.result).toEqual({ - a: 'a', - b: { - c: { - h: 'h', - }, - d: { - e: null, - f: 'f', - }, - g: 'g', - }, - }); - }); - - it('should merge an object with an empty object and remove deeply nested null values', () => { - const result = utils.fastMerge({}, testObjectWithNullishValues, { - shouldRemoveNestedNulls: true, - }); - - expect(result.result).toEqual(testObjectWithNullValuesRemoved); - }); - - it('should remove null values by merging two identical objects with fastMerge', () => { - const result = utils.removeNestedNullValues(testObjectWithNullishValues); - - expect(result).toEqual(testObjectWithNullValuesRemoved); - }); - - it('should replace Date objects', () => { - const oldDate = new Date('2024-01-01'); - const newDate = new Date('2025-01-01'); - const result = utils.fastMerge(oldDate, newDate); - expect(result.result).toEqual(newDate); - }); - - it('should replace RegExp objects', () => { - const oldRegex = /old/gi; - const newRegex = /new/i; - const result = utils.fastMerge(oldRegex, newRegex); - expect(result.result).toEqual(newRegex); - }); - - it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { - const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'mark', - }); - - expect(result.result).toEqual({ - b: { - d: { - h: 'h', - [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, - }, - h: 'h', - }, - }); - expect(result.replaceNullPatches).toEqual([[['b', 'd'], {h: 'h'}]]); - }); - - it('should completely replace the target object with its source when the source has the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag and "objectRemovalMode" is set to "replace"', () => { - const result = utils.fastMerge( - testObject, - { - b: { - d: { - h: 'h', - [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, - }, - h: 'h', - }, - }, - { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'replace', - }, - ); - - expect(result.result).toEqual({ - a: 'a', - b: { - c: 'c', - d: { - h: 'h', - }, - h: 'h', - g: 'g', - }, - }); - }); - - test.each([ - ['a string', 'value'], - ['a number', 1000], - ['a boolean', true], - ['an array', []], - ])('should replace an object with %s', (_label, expected) => { - const result = utils.fastMerge(testObject, expected); - expect(result.result).toEqual(expected); - }); - - test.each([ - ['a string', 'value'], - ['a number', 1000], - ['a boolean', true], - ['an array', []], - ])('should replace %s with an object', (_label, data) => { - const result = utils.fastMerge(data, testObject); - expect(result.result).toEqual(testObject); - }); - }); -}); diff --git a/tests/unit/utilsTest.ts b/tests/unit/utilsTest.ts new file mode 100644 index 000000000..c76cae58b --- /dev/null +++ b/tests/unit/utilsTest.ts @@ -0,0 +1,392 @@ +import utils from '../../lib/utils'; +import type {GenericDeepRecord} from '../types'; + +type DeepObject = GenericDeepRecord | unknown[]; + +const testObject: DeepObject = { + a: 'a', + b: { + c: 'c', + d: { + e: 'e', + f: 'f', + }, + g: 'g', + }, +}; + +const testObjectWithNullishValues: DeepObject = { + a: undefined, + b: { + c: { + h: 'h', + }, + d: { + e: null, + }, + }, +}; + +const testObjectWithNullValuesRemoved: DeepObject = { + b: { + c: { + h: 'h', + }, + d: {}, + }, +}; + +const testMergeChanges: DeepObject[] = [ + { + b: { + d: { + h: 'h', + }, + }, + }, + { + b: { + d: null, + h: 'h', + }, + }, +]; + +describe('utils', () => { + describe('fastMerge', () => { + describe('primitives', () => { + it('should replace strings', () => { + const result = utils.fastMerge('old', 'new'); + expect(result.result).toEqual('new'); + }); + + it('should replace numbers', () => { + const result = utils.fastMerge(1000, 1001); + expect(result.result).toEqual(1001); + }); + + it('should replace booleans', () => { + const result = utils.fastMerge(true, false); + expect(result.result).toEqual(false); + }); + }); + + describe('arrays', () => { + it('should replace arrays', () => { + const result = utils.fastMerge(['a', 1, true], ['b', false]); + expect(result.result).toEqual(['b', false]); + }); + }); + + describe('objects', () => { + it('should merge an object with another object and remove nested null values', () => { + const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); + + expect(result.result).toEqual({ + a: 'a', + b: { + c: { + h: 'h', + }, + d: { + f: 'f', + }, + g: 'g', + }, + }); + }); + + it('should merge an object with another object and not remove nested null values', () => { + const result = utils.fastMerge(testObject, testObjectWithNullishValues); + + expect(result.result).toEqual({ + a: 'a', + b: { + c: { + h: 'h', + }, + d: { + e: null, + f: 'f', + }, + g: 'g', + }, + }); + }); + + it('should merge an object with an empty object and remove deeply nested null values', () => { + const result = utils.fastMerge({}, testObjectWithNullishValues, { + shouldRemoveNestedNulls: true, + }); + + expect(result.result).toEqual(testObjectWithNullValuesRemoved); + }); + + it('should replace Date objects', () => { + const oldDate = new Date('2024-01-01'); + const newDate = new Date('2025-01-01'); + const result = utils.fastMerge(oldDate, newDate); + expect(result.result).toEqual(newDate); + }); + + it('should replace RegExp objects', () => { + const oldRegex = /old/gi; + const newRegex = /new/i; + const result = utils.fastMerge(oldRegex, newRegex); + expect(result.result).toEqual(newRegex); + }); + + it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { + const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'mark', + }); + + expect(result.result).toEqual({ + b: { + d: { + h: 'h', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + }, + }); + expect(result.replaceNullPatches).toEqual([[['b', 'd'], {h: 'h'}]]); + }); + + it('should completely replace the target object with its source when the source has the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag and "objectRemovalMode" is set to "replace"', () => { + const result = utils.fastMerge( + testObject, + { + b: { + d: { + h: 'h', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + }, + }, + { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }, + ); + + expect(result.result).toEqual({ + a: 'a', + b: { + c: 'c', + d: { + h: 'h', + }, + h: 'h', + g: 'g', + }, + }); + }); + + test.each([ + ['a string', 'value'], + ['a number', 1000], + ['a boolean', true], + ['an array', []], + ])('should replace an object with %s', (_label, expected) => { + const result = utils.fastMerge(testObject, expected); + expect(result.result).toEqual(expected); + }); + + test.each([ + ['a string', 'value'], + ['a number', 1000], + ['a boolean', true], + ['an array', []], + ['null', null], + ['undefined', undefined], + ])('should replace %s with an object', (_label, data) => { + const result = utils.fastMerge(data, testObject); + expect(result.result).toEqual(testObject); + }); + }); + + describe('reference stability', () => { + 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(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(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); + }); + + it('should skip undefined source values and preserve target reference', () => { + const target = {a: 1, b: 2}; + const source = {a: undefined}; + const result = utils.fastMerge(target, source); + expect(result.result).toBe(target); + }); + + it('should preserve references through deeply nested merges (3+ levels)', () => { + const deepNested = {x: 1}; + const target = {a: {b: {c: deepNested}}, d: 'hello'}; + const source = {d: 'hello'}; + const result = utils.fastMerge(target, source); + expect(result.result).toBe(target); + expect(result.result.a.b.c).toBe(deepNested); + }); + + it('should return a new reference at each changed level in a deep merge', () => { + const target = {a: {b: {c: 1, d: 2}}, e: 'unchanged'}; + const source = {a: {b: {c: 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.b).not.toBe(target.a.b); + expect(result.result.a.b).toEqual({c: 99, d: 2}); + }); + }); + }); + + describe('removeNestedNullValues', () => { + it('should remove null values by merging two identical objects with fastMerge', () => { + const result = utils.removeNestedNullValues(testObjectWithNullishValues); + + expect(result).toEqual(testObjectWithNullValuesRemoved); + }); + + it('should pass through primitives unchanged', () => { + expect(utils.removeNestedNullValues('hello')).toBe('hello'); + expect(utils.removeNestedNullValues(42)).toBe(42); + expect(utils.removeNestedNullValues(true)).toBe(true); + expect(utils.removeNestedNullValues(null)).toBe(null); + expect(utils.removeNestedNullValues(undefined)).toBe(undefined); + }); + + it('should return the same array reference', () => { + const arr = [1, 2, 3]; + const result = utils.removeNestedNullValues(arr); + expect(result).toBe(arr); + }); + + 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 return a new empty object when all properties are null/undefined', () => { + const value = {a: null, b: undefined, c: null}; + const result = utils.removeNestedNullValues(value); + expect(result).not.toBe(value); + expect(result).toEqual({}); + }); + + describe('reference stability', () => { + 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).b).toBe(nested); + }); + + 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).a).toBe(sibling); + }); + + 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).items).toBe(arr); + }); + + it('should return the same reference for an empty object', () => { + const value = {}; + const result = utils.removeNestedNullValues(value); + expect(result).toBe(value); + }); + }); + }); +});