Deep merge with internal arrays does not work as expected

[itaysabato]

The merge method is not available for arrays (why?) and when objects contain array descendants it causes deep merges not to work as expected.

For example:

// Works as expected without arrays:
var target = Immutable({I: {have: {anObject: {inside: "me"}}}})
var source = Immutable({I: {have: {anObject: {how: "cool"}}}})

console.log(JSON.stringify(target.merge(source, {deep: true})))
// -> {"I":{"have":{"anObject":{"inside":"me","how":"cool"}}}}

// But with arrays, original values in the target may be lost:
var targetWithArray = Immutable({I: {have: {anArray: [{inside: "me"}]}}})
var sourceWithArray = Immutable({I: {have: {anArray: [{how: "cool"}]}}})

console.log(JSON.stringify(targetWithArray.merge(sourceWithArray, {deep: true})))
// -> {"I":{"have":{"anArray":[{"how":"cool"}]}}}
// Expected: {"I":{"have":{"anArray":[{"inside":"me","how":"cool"}]}}}

This seems to be because arrays are not mergable as defined by isMergableObject which causes the merge to get "stuck" once an array is found and replace it instead of continuing the merge as seen here.

We are migrating from Immutable.js where this functionality exists and is useful to us. Is there a design decision behind this or is it a bug?

Thanks.

[gmeans]

I'm running into the same limitation. @itaysabato have you found a pattern to use as an alternative?

[itaysabato]

@gmeans my alternative is to not use this library yet... I have implemented my own setIn and mergeIn functions that use native objects and arrays.

[gmeans]

I ended up using a regular merge within the setIn:

export const saveAddress = (state = initialState, action) => {
  const { address, address2, city, province, postal, country, index } = action;

  const existing = Immutable(state.current.addresses[ index ]);

  return Immutable.setIn(state, [ 'current', 'addresses', index ], Immutable.merge(existing, {
    address,
    address2,
    city,
    province,
    postal,
    country,
  }), { deep: true })
};

Maybe my use case is slightly different since I'm directly setting an element at a specific index, but I experienced the same issue you had. After the setIn the resulting object only consisted of what was passed into setIn, wiping out all existing values that I would expect to be merged in.

[itaysabato]

@gmeans If the library had a mergeIn method (that creates a new object only if necessary) it would solve my use case and probably yours as well. You can implement it yourself though with code like:

function mergeIn(target, path, source, config) {
    var innerTarget = Immutable.getIn(target, path);
    var innerMerged = Immutable.merge(innerTarget, source, config);

    return Immutable.setIn(target, path, innerMerged);
}

There are probably some edge cases that need to be addressed - this code is based on what I'm doing without seamless-immutable.

Personally, I think this is a basic functionality (the only one I need, in fact) that should be provided by the library.

[crudh]

@itaysabato it is a design decision (at least to my knowledge). You can use a custom merger to control how to handle arrays. Some examples here: https://github.com/crudh/seamless-immutable-mergers

[blueberryfan]

@itaysabato Are you (or anyone else) aware of any good examples of deep merge handling arrays, eg an array a couple of levels down that in turn has various items in it, including arrays and objects with arrays in them? I'm thinking to put together a recursive custom merger to handle arrays, but as it hits anything other than arrays it'd be nice if it could reuse already existing merge logic. Otherwise, as I understand id, the custom merger would have to handle every other kind of item it comes across (objects, primitive types, etc) as well, which seems like a big job.

The example custom mergers seem to be quite simplistic and not cater for recursing down arrays.

I'll keep digging meanwhile.

Sample data { aa: { bb: [ 'one', { xx: { yy: [1, 2, 3] } }, [11, { zz: 123 }] ] } }

[blueberryfan]

@rtfeldman Just pinging you as well. See comment / question above :)