Arrays are not copied properly - keys are lost

[arturog]

Consider:

a = { items: [ { name: 'Superman', strength: 1000 }, { name: 'Jim', strength: 2 } ]
a.items.top = 0
a
// =>
// { items: 
//    [ { name: 'Superman', strength: 1000 },
//      { name: 'Jim', strength: 2 },
//      top: 0 ] }
c
=>
 { items: [
    { name: "Superman", strength: 1000 },
    { name: "Jim", strength: 2 }
  ] 
}
cmd = { items: { 1: { strength: { $set: 3 } } } }
b = update(a, cmd)
b
// =>
// { items: 
//    [ { name: 'Superman', strength: 1000 },
//      { name: 'Jim', strength: 3 } ] }

Notice the "top" key was lost. The culprit is (I believe) line 22 where it should be:

function copy(object) {
  if (object instanceof Array) {
-     return object.slice();
+     return assign(object.constructor(), object)
  } else if (object && typeof object === 'object') {
    var prototype = object.constructor && object.constructor.prototype

Keys should not be lost.

[kolodny]

Thanks for the feedback. This has been fixed in 2.3.0

[arturog]

Awwsome!

[arturog]

$push and $unshift lose keys as well. I had to change them. How do you reopen issues in github?

  $push: function(value, nextObject, spec) {
    invariantPushAndUnshift(nextObject, spec, '$push');
-   return value.length ? nextObject.concat(value) : nextObject;
+   if (value.length) {
+     nextObject = copy(nextObject)
+     nextObject.push.apply(nextObject, value)
+   }
+   return nextObject;
  },
  $unshift: function(value, nextObject, spec) {
    invariantPushAndUnshift(nextObject, spec, '$unshift');
-   return value.length ? value.concat(nextObject) : nextObject;
+   if (value.length) {
+     nextObject = copy(nextObject)
+     nextObject.unshift.apply(nextObject, value)
+   }
+   return nextObject;
  },

[kolodny]

$push and $unshift should lose keys. Consider the following:

var a = ['w','x','y']
a.foo = 'bar'
var b = a.concat(['z'])
b.foo === undefined

The reason why this isn't true in $set is because $set is a replacement for doing a[prop] = value which the object "returned" would retain any properties that existed in the array.

I'd suggest overriding the $push and $unshift directives to support non standard keys to be retained in the next object

[arturog]

What you just did was a concat, not a push... Push doesn't lose keys at all.

…

On 17 Jul 2017 02:06, "Moshe Kolodny" ***@***.***> wrote: $push and $unshift should lose keys. Consider the following: var a = ['w','x','y']a.foo = 'bar'var b = a.concat(['z'])b.foo === undefined The reason why this isn't true in $set is because $set is a replacement for doing a[prop] = value which the object "returned" would retain any properties that existed in the array. I'd suggest overriding the $push and $unshift directives to support non standard keys to be retained in the next object — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUjq6ctQBiPMU8GdKru6K1HOJSLjrh6ks5sOrOXgaJpZM4OUHVC> .

[kolodny]

Yes $push is basically a concat, meaning a new object is expected, as opposed to a set which is just reusing the old object

[arturog]

I think $push and $unshift should match .push and .unshift. No?

…

On 18 July 2017 at 15:27, Moshe Kolodny ***@***.***> wrote: Yes $push is basically a concat, meaning a new object is expected, as opposed to a set which is just reusing the old object — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAUjq8mabvos0B4TC-ibsjCN9RX_zI0lks5sPMDFgaJpZM4OUHVC> .

[ghost]

If $push is actually a concat, why is it called push?
Same goes for $splice, instead shouldn't it be named $slice? If applicable.

[jednano]

I agree that $concat would be more indicative of how this feature works. Sounds like a good candidate for a breaking change, in fact.