mergeIn bug?

[ddaza]

is this a bug?

var test = Immutable.fromJS([{'a':[1,2,3, 4, 5]}, undefined, {'c': undefined}]);
test.mergeIn([0, 'a'], [5,6,7]) // [{a: [5,6,7,4,5]} , undefined, {c: undefined}]
test.mergeIn([2, 'c'], 'hello') 
// Uncaught TypeError: Cannot read property 'merge' of undefined

I find it a little inconsistent from what i was expecting to happen.

[leebyron]

Could you add what you expected in this case? I can explain what is happening at present: "merge" is an operation you perform upon a collection, but the value found at [2,"c"] is undefined. Perhaps we could improve the behavior or at least the error message in this case, but I'm curious what you expected the outcome to be if not an error.

[mlajtos]

Maybe @ddaza is confused about merging [1,2,3,4,5] and [5,6,7] results in [5,6,7,4,5] and not [1,2,3,4,5,6,7]?

[ddaza]

Sorry i didn't specify where the confusion is.
The first merge makes sense to me the array gets merged on the same indexes of the array so that's not problem.

The last line of the test is what I find inconsistent.

When I merge 2 objects and keys I assumed that the keys will remain and values are swapped by the new merged object, since the documentation says that MergeIn is "A combination of updateIn and merge". In my opinion that logic should make undefined into hello instead of erroring out.

If the current functionality is done by design then is my mistake.

Thanks!

[leebyron]

Ah, I understand. At present, the operation you're attempting is covered by setIn - e.g. you want to take a value and position it at a certain path. mergeIn expects to operate on something which can be merged - some kind of collection - which undefined cannot. The value you provided "hello" is also not a collection and not mergeable.

[leebyron]

So these two variants should behave as you expect:

mergeIn against something collection-like:

test.mergeIn([2], { c: 'hello' })

setIn when wanting to position a value regardless of mergeability

test.setIn([2, 'c'], 'hello')

[ddaza]

thanks

[jchesterpivotal]

I disagree that this is expected behavior.

Null is not formally a value, but is typically used as a de facto value in JSON, so that the key is held open for further updates. When I have a data structure that can change shape and value over successive generations of mergeIn, I'm gambling that nobody -- at some point -- sent me anything of the form { keyToBeFilledInLater: null }.

So what are the alternatives?

First, I can use setIn. On my current project we're doing that, it was the correct behavior for our case.

But going back to the problem of a structure that adds keys. If I use setIn, I will first need to check for that key before I can use setIn.

So I throw my hands in the air and use mergeIn, since that's what it's for. But now, instead of a guard clause on keys, I need a guard clause on values.

To me, this defeats the purpose of high-level convenience functions, which is to be convenient without surprising corner cases.

If the behavior is not intended to change, perhaps a more helpful error message could be added? Something of the form

You tried to merge a new value into a null value. Try using set or ensure that the value is not-null at creation time.

[jchesterpivotal]

Update for anyone foolish enough to take me seriously, I got the above only partly halfway correct.

set* will create new keys and values, including for keys with a value of null.

a = { key: null }
aImmutable = Immutable.fromJS(a)
aImmutable.setIn(['key'], 1)
-> Object {key: 1}