`merge` and `mergeWith` treat properties inconsistently with top-level arguments
See comment here: https://github.com/toss/es-toolkit/pull/556#issuecomment-3263219254
merge(["1"], {a: 2}) gives a different result than merge({x: ["1"]}, {x: {a: 2}}).x, which is probably unexpected.
Note that the merging is consistent in the compat version.
The type checking performed on each property should possibly instead be done to at the top level of "source" and "target", in order to keep the recursive behavior consistent.
If this is indeed a bug, I'd like to try fixing it. Would that be possible?
@Seol-JY Sure! Contribution is always welcome. I guess this fix should be applied to the two functions.
https://github.com/toss/es-toolkit/pull/556#issuecomment-3263219254
In fact, lodash also has inconsistencies in certain situations:
import _ from 'lodash';
console.log(_.merge({ x: { a: 2 } }, { x: ['1'] }).x);
// Current: ['1']
// Expected: { 0: '1', a: 2 }
console.log(_.merge({ a: 2 }, ['1']));
// Current: { 0: '1', a: 2 }
// Expected: { 0: '1', a: 2 }
In (1), the object { a: 2 } is completely replaced by the array ['1']. But in (2), the object structure is preserved and only the values are merged.
I believe merge should consistently preserve the target's structure while filling in the source's values. If the target is an object, the result should remain an object regardless of whether the source is an array.
After more thought, I still believe preserving the target structure is the correct behavior. Here's why:
1. Consistency with Object.assign:
Object.assign({ a: 1 }, { b: 2 })
// { a: 1, b: 2 } - target structure preserved
Object.assign(target, source)fills target with source valuesmergeshould follow the same pattern, just recursively
2. "Recursively" should maintain the same rule:
- If top-level preserves target structure, nested levels should too
_.merge({ a: 2 }, ['1']) // { 0: '1', a: 2 } ✓ target preserved
_.merge({ x: { a: 2 } }, { x: ['1'] }).x // ['1'] ✗ target replaced
Lodash docs describe merge as similar to Object.assign, but working recursively. Given this, most users would naturally expect consistent behavior at all levels. I think both lodash and we are currently doing it wrong, and we should fix this to preserve the target structure.
Thanks for bringing this up! 🙏 I've updated the implementation based on your feedback — Object.assign should now work recursively as expected. Really appreciate the report!
https://github.com/toss/es-toolkit/pull/1553