aearly/icepick

icepick.merge() of two identical objects containing an array returns a new object

zuk opened this issue · 4 comments

zuk commented

A .merge of two identical objects that contain an array is returning a new object.

For example:

var a = {foo: []}
var b = {foo: []}
var ab = icepick.merge(a, b)
ab === a  // => false

Without a nested array it works as expected:

var a = {foo: {}}
var b = {foo: {}}
var ab = icepick.merge(a, b)
ab === a  // => true

This is a bit of expected behavior. When merge encounters 2 different arrays, it just uses the array from b. Note that if the two arrays in a and b were frozen and equal, it would return the same object.

var arr = Object.freeze([]);
var a = {foo: arr}
var b = {foo: arr}
var ab = icepick.merge(a, b)
ab === a  // => true

The weirdness comes from the fact that there is no standard way to merge two arrays. It's really hard to detect insertions and deletions in the list of objects, and it's ambiguous what to do exactly in many cases. You'd have to specify some sort of key to use to uniquely identify items in the list, or some other form of deep inspection. One of the goals of Icepick is to avoid doing doing deep comparison of objects, by setting up structures that you can compare by reference. I'm intentionally keeping merge "dumb" in regards to arrays, so for now icepick just overwrites any array with the new array.

If you do need to update an array, I'd recommend against using merge -- use assocIn or updateIn with a combination of the array methods.

I'm open to adding a customizer function to merge, similar to lodash.merge, that allows you to customize the merging behavior. This would allow you to specify what to do exactly when merging arrays.

+1 to adding a customize function.

zuk commented

Added a custom resolver option for .merge. Let me know if I missed anything in that pull request.

Custom resolvers are now published in v1.2.0!