rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.37k stars 195 forks source link

Deep merge fails in non-trivial cases #55

Closed ianp closed 9 years ago

ianp commented 9 years ago

Unless I'm doing something daft (always a strong possibility!). Given

var obj = Immutable({
  streams: {
    a: { name: "Conversations", channel: ["Facebook","Twitter"], loading: false},
    b: { name: "@Mentions", channel: ["Twitter"], loading: false},
    c: { name: "Facebook", channel: ["Facebook"], loading: false}
  },
  streamOrder: ["c", "a", "b"]
})

then running j.merge({streams: {a: { loading: true }}}, true) produces

{
  streams: {
    a: { loading:true }
  },
  streamOrder: {0: "c", 1: "a", 2: "b"}
}

so instead of doing a deep merge it has thrown away all of the data under the streams key! Have I misunderstood how the deep merge is meant to work?

It's also converted the data under streamOder from an Array to an Object, but I can live with that for the time being :-)

ianp commented 9 years ago

FWIW a simpler example based on the one from your docs seems to do the right thing for me:

Immutable({all: "your base", are: {belong: "to them"}}).merge({are: {belong: "to us"}}).merge({all: "the world"})

produces:

{all: "the world", are: { belong: "to us"} }
crudh commented 9 years ago

@ianp: If you want to do a deep merge then the second argument shouldn't be true, it should be {deep: true} like:

obj.merge({streams: {a: { loading: true }}}, {deep: true});

It produces the desired results:

{
  streams: {
    a: {name: 'Conversations', channel: [Object], loading: true},
    b: {name: '@Mentions', channel: [Object], loading: false},
    c: {name: 'Facebook', channel: [Object], loading: false}
  },
  streamOrder: ['c', 'a', 'b']
}
ianp commented 9 years ago

Right you are, told you it was probably me doing something daft! Thanks!