diff --git a/packages/commons/src/deepMerge.ts b/packages/commons/src/deepMerge.ts index d0abbcf6e2..8961fa6ae4 100644 --- a/packages/commons/src/deepMerge.ts +++ b/packages/commons/src/deepMerge.ts @@ -24,7 +24,7 @@ const isPlainObject = (value: unknown): value is Record => { const mergeArrayItemsByIndex = ( targetArray: unknown[], sourceArray: unknown[], - seen: WeakSet + ancestors: object[] ): void => { for (let i = 0; i < sourceArray.length; i++) { const srcItem = sourceArray[i]; @@ -32,15 +32,16 @@ const mergeArrayItemsByIndex = ( const isSrcPlainObject = isPlainObject(srcItem); - // Skip already-seen objects to prevent circular references - if (isSrcPlainObject && seen.has(srcItem)) { + // Skip objects in the current ancestor chain to prevent circular references + if (isSrcPlainObject && ancestors.includes(srcItem)) { continue; } // Merge nested plain objects recursively if (isSrcPlainObject && isPlainObject(tgtItem)) { - seen.add(srcItem); - mergeRecursive(tgtItem, srcItem, seen); + ancestors.push(srcItem); + mergeRecursive(tgtItem, srcItem, ancestors); + ancestors.pop(); continue; } @@ -61,14 +62,14 @@ const handleArrayMerge = ( key: string, sourceArray: unknown[], targetValue: unknown, - seen: WeakSet + ancestors: object[] ): void => { if (!Array.isArray(targetValue)) { target[key] = [...sourceArray]; return; } - mergeArrayItemsByIndex(targetValue, sourceArray, seen); + mergeArrayItemsByIndex(targetValue, sourceArray, ancestors); }; /** @@ -81,15 +82,15 @@ const handleObjectMerge = ( key: string, sourceObject: Record, targetValue: unknown, - seen: WeakSet + ancestors: object[] ): void => { if (isPlainObject(targetValue)) { - mergeRecursive(targetValue, sourceObject, seen); + mergeRecursive(targetValue, sourceObject, ancestors); return; } const newTarget: Record = {}; - mergeRecursive(newTarget, sourceObject, seen); + mergeRecursive(newTarget, sourceObject, ancestors); target[key] = newTarget; }; @@ -101,7 +102,7 @@ const handleObjectMerge = ( const mergeRecursive = ( target: Record, source: Record, - seen: WeakSet + ancestors: object[] ): void => { for (const key of Object.keys(source)) { if (UNSAFE_KEYS.has(key)) { @@ -112,16 +113,18 @@ const mergeRecursive = ( const targetValue = target[key]; if (Array.isArray(sourceValue)) { - if (seen.has(sourceValue)) continue; - seen.add(sourceValue); - handleArrayMerge(target, key, sourceValue, targetValue, seen); + if (ancestors.includes(sourceValue)) continue; + ancestors.push(sourceValue); + handleArrayMerge(target, key, sourceValue, targetValue, ancestors); + ancestors.pop(); continue; } if (isPlainObject(sourceValue)) { - if (seen.has(sourceValue)) continue; - seen.add(sourceValue); - handleObjectMerge(target, key, sourceValue, targetValue, seen); + if (ancestors.includes(sourceValue)) continue; + ancestors.push(sourceValue); + handleObjectMerge(target, key, sourceValue, targetValue, ancestors); + ancestors.pop(); continue; } @@ -135,8 +138,9 @@ const mergeRecursive = ( * Recursively merge properties from source objects into the target object, mutating it. * * Nested plain objects are merged recursively, arrays are merged by index (e.g., `[1, 2]` + `[3]` → `[3, 2]`), - * and class instances (Date, RegExp, custom classes) are assigned by reference. Circular references and - * prototype pollution attempts (`__proto__`, `constructor`) are safely skipped. + * and class instances (Date, RegExp, custom classes) are assigned by reference. Circular references are + * detected via ancestor-chain tracking and safely skipped, while shared (non-circular) object references + * are merged correctly. Prototype pollution attempts (`__proto__`, `constructor`) are also skipped. * * @example * ```typescript @@ -155,13 +159,13 @@ const deepMerge = >( target: T, ...sources: Array | undefined | null> ): T => { - const seen = new WeakSet(); - seen.add(target); + const ancestors: object[] = [target]; for (const source of sources) { if (source != null) { - seen.add(source); - mergeRecursive(target, source, seen); + ancestors.push(source); + mergeRecursive(target, source, ancestors); + ancestors.pop(); } } diff --git a/packages/commons/tests/unit/deepMerge.test.ts b/packages/commons/tests/unit/deepMerge.test.ts index b14f9bc5e5..bcac1f4457 100644 --- a/packages/commons/tests/unit/deepMerge.test.ts +++ b/packages/commons/tests/unit/deepMerge.test.ts @@ -123,7 +123,7 @@ describe('Function: deepMerge', () => { it('replaces non-object target values with source objects', () => { // Prepare - const target = { a: 'string' } as Record; + const target: Record = { a: 'string' }; const source = { a: { nested: 1 } }; // Act @@ -146,6 +146,28 @@ describe('Function: deepMerge', () => { }); }); + describe('Multi-source merging', () => { + it('merges three sources with arrays of objects', () => { + // Prepare + const names = { characters: [{ name: 'barney' }, { name: 'fred' }] }; + const ages = { characters: [{ age: 36 }, { age: 40 }] }; + const heights = { + characters: [{ height: '5\'4"' }, { height: '5\'5"' }], + }; + + // Act + const result = deepMerge({}, names, ages, heights); + + // Assess + expect(result).toEqual({ + characters: [ + { name: 'barney', age: 36, height: '5\'4"' }, + { name: 'fred', age: 40, height: '5\'5"' }, + ], + }); + }); + }); + describe('Array merging (index-based)', () => { it('merges arrays by index', () => { // Prepare @@ -185,7 +207,7 @@ describe('Function: deepMerge', () => { it('replaces non-array target with array source', () => { // Prepare - const target = { arr: 'not an array' } as Record; + const target: Record = { arr: 'not an array' }; const source = { arr: [1, 2, 3] }; // Act @@ -219,16 +241,16 @@ describe('Function: deepMerge', () => { // Assess expect(result).toEqual({ a: 1, b: 2 }); - expect(({} as Record).polluted).toBeUndefined(); + expect({}).not.toHaveProperty('polluted'); }); it('skips constructor keys from source', () => { // Prepare const target = { a: 1 }; - const source = { constructor: { polluted: true }, b: 2 } as Record< - string, - unknown - >; + const source: Record = { + constructor: { polluted: true }, + b: 2, + }; // Act const result = deepMerge(target, source); @@ -238,6 +260,17 @@ describe('Function: deepMerge', () => { expect(result.constructor).toBe(Object); }); + it('does not indirectly pollute via toString.constructor.prototype', () => { + // Prepare & Act + const source: Record = { + toString: { constructor: { prototype: { polluted: true } } }, + }; + deepMerge({}, source); + + // Assess + expect(Function.prototype).not.toHaveProperty('polluted'); + }); + it('handles nested __proto__ keys', () => { // Prepare const target = { nested: { a: 1 } }; @@ -250,7 +283,7 @@ describe('Function: deepMerge', () => { // Assess expect(result).toEqual({ nested: { a: 1, b: 2 } }); - expect(({} as Record).polluted).toBeUndefined(); + expect({}).not.toHaveProperty('polluted'); }); }); @@ -264,7 +297,7 @@ describe('Function: deepMerge', () => { // Act const result = deepMerge(target, source); - // Assess - self-reference is skipped entirely + // Assess expect(result).toEqual({ a: 1, b: 2 }); expect(result).not.toHaveProperty('self'); }); @@ -272,22 +305,20 @@ describe('Function: deepMerge', () => { it('handles deep circular references', () => { // Prepare const target = { a: 1 }; - const source: Record = { - b: 2, - nested: { c: 3 }, - }; - (source.nested as Record).circular = source; + const nested: Record = { c: 3 }; + const source: Record = { b: 2, nested }; + nested.circular = source; // Act const result = deepMerge(target, source); - // Assess - circular reference is skipped + // Assess expect(result).toEqual({ a: 1, b: 2, nested: { c: 3 } }); }); it('handles circular arrays', () => { // Prepare - const target = { a: 1 }; + const target: Record = { a: 1 }; const circularArr: unknown[] = [1, 2]; circularArr.push(circularArr); const source = { arr: circularArr }; @@ -295,15 +326,41 @@ describe('Function: deepMerge', () => { // Act const result = deepMerge(target, source); - // Assess - circular array reference is preserved in shallow copy - // (the array itself is copied, but the circular element points to original) - expect(result.a).toBe(1); - expect(Array.isArray(result.arr)).toBe(true); - expect((result.arr as unknown[])[0]).toBe(1); - expect((result.arr as unknown[])[1]).toBe(2); + // Assess + expect(result).toHaveProperty('a', 1); + expect(result).toHaveProperty('arr[0]', 1); + expect(result).toHaveProperty('arr[1]', 2); }); - it('skips already-seen arrays to prevent duplication', () => { + it('skips array that references an ancestor array', () => { + // Prepare + const arr: unknown[] = []; + const inner: Record = { backRef: arr }; + arr.push(inner); + const target = { arr: [{ a: 1 }] }; + const source = { arr }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ arr: [{ a: 1 }] }); + }); + + it('skips circular plain objects inside arrays', () => { + // Prepare + const target = { arr: [{ a: 1 }] }; + const source: Record = { b: 2 }; + source.arr = [source]; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ arr: [{ a: 1 }], b: 2 }); + }); + + it('merges shared array references into all properties', () => { // Prepare const target = {}; const sharedArray = [1, 2, 3]; @@ -315,19 +372,17 @@ describe('Function: deepMerge', () => { // Act const result = deepMerge(target, source); - // Assess - shared array is copied on first occurrence, - // skipped on subsequent occurrences + // Assess expect(result).toEqual({ first: [1, 2, 3], - second: undefined, + second: [1, 2, 3], }); - expect(result).not.toHaveProperty('second'); }); - it('skips already-seen objects within arrays', () => { + it('merges shared objects within arrays', () => { // Prepare const sharedObj = { shared: true }; - const target = { + const target: Record = { arr: [{ a: 1 }, { b: 2 }], }; const source = { @@ -338,17 +393,81 @@ describe('Function: deepMerge', () => { // Act const result = deepMerge(target, source); - // Assess - sharedObj is merged into prop first, then skipped when - // encountered again in the array merge - expect(result.prop).toEqual({ shared: true }); - expect((result.arr as Record[])[0]).toEqual({ a: 1 }); - expect((result.arr as Record[])[1]).toEqual({ - b: 2, - c: 3, + // Assess + expect(result).toEqual({ + prop: { shared: true }, + arr: [ + { a: 1, shared: true }, + { b: 2, c: 3 }, + ], + }); + }); + + it('merges shared objects into all referencing properties', () => { + // Prepare + const target = {}; + const shared = { value: 42 }; + const source = { + first: shared, + second: { nested: shared }, + }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + first: { value: 42 }, + second: { nested: { value: 42 } }, + }); + }); + }); + + describe('Shared (non-circular) references', () => { + it('merges shared array references into both properties', () => { + // Prepare + const target = {}; + const sharedArray = [1, 2, 3]; + const source = { + first: sharedArray, + second: sharedArray, + }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + first: [1, 2, 3], + second: [1, 2, 3], }); }); - it('skips already-seen objects to prevent duplication', () => { + it('merges shared objects referenced in arrays', () => { + // Prepare + const sharedObj = { shared: true }; + const target: Record = { + arr: [{ a: 1 }, { b: 2 }], + }; + const source = { + prop: sharedObj, + arr: [sharedObj, { c: 3 }], + }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + prop: { shared: true }, + arr: [ + { a: 1, shared: true }, + { b: 2, c: 3 }, + ], + }); + }); + + it('merges shared objects into all referencing properties', () => { // Prepare const target = {}; const shared = { value: 42 }; @@ -360,11 +479,79 @@ describe('Function: deepMerge', () => { // Act const result = deepMerge(target, source); - // Assess - shared object is merged on first occurrence, - // skipped on subsequent occurrences to prevent infinite recursion + // Assess expect(result).toEqual({ first: { value: 42 }, - second: {}, + second: { nested: { value: 42 } }, + }); + }); + + it('merges shared objects across multiple sources', () => { + // Prepare + const shared = { x: 1 }; + const target = {}; + const source1 = { a: shared }; + const source2 = { b: shared }; + + // Act + const result = deepMerge(target, source1, source2); + + // Assess + expect(result).toEqual({ a: { x: 1 }, b: { x: 1 } }); + }); + + it('merges diamond-shaped shared references', () => { + // Prepare + const shared = { value: 'shared' }; + const target = {}; + const source = { + branch1: { leaf: shared }, + branch2: { leaf: shared }, + }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + branch1: { leaf: { value: 'shared' } }, + branch2: { leaf: { value: 'shared' } }, + }); + }); + + it('merges a shared object that itself contains a circular reference', () => { + // Prepare + const shared: Record = { value: 1 }; + shared.self = shared; + const target = {}; + const source = { + first: shared, + second: { nested: shared }, + }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + first: { value: 1 }, + second: { nested: { value: 1 } }, + }); + }); + + it('merges when source references an object from the target', () => { + // Prepare + const inner = { x: 1 }; + const target: Record = { a: inner }; + const source = { b: inner }; + + // Act + const result = deepMerge(target, source); + + // Assess + expect(result).toEqual({ + a: { x: 1 }, + b: { x: 1 }, }); }); }); @@ -478,7 +665,7 @@ describe('Function: deepMerge', () => { // Assess expect(result).toEqual({ a: 1, b: 2 }); - expect((result as Record)[sym]).toBeUndefined(); + expect(sym in result).toBe(false); }); it('handles numeric keys', () => { @@ -495,7 +682,7 @@ describe('Function: deepMerge', () => { it('handles special number values', () => { // Prepare - const target = {}; + const target: Record = {}; const source = { inf: Number.POSITIVE_INFINITY, negInf: Number.NEGATIVE_INFINITY, @@ -514,7 +701,7 @@ describe('Function: deepMerge', () => { it('handles function values', () => { // Prepare const fn = () => 42; - const target = {}; + const target: Record = {}; const source = { fn }; // Act @@ -522,7 +709,32 @@ describe('Function: deepMerge', () => { // Assess expect(result.fn).toBe(fn); - expect((result.fn as () => number)()).toBe(42); + }); + + it('replaces function target value with object from later source', () => { + // Prepare + const fn = () => 42; + const source1: Record = { a: fn }; + const source2 = { a: { b: 2 } }; + + // Act + const result = deepMerge({}, source1, source2); + + // Assess + expect(result).toEqual({ a: { b: 2 } }); + expect(source1.a).toBe(fn); + }); + + it('handles self-merge without infinite loop', () => { + // Prepare + const object: Record = { a: 1, b: { c: 2 } }; + + // Act + const result = deepMerge(object, object); + + // Assess + expect(result).toBe(object); + expect(result).toEqual({ a: 1, b: { c: 2 } }); }); }); });