Not exact `currentPath` at array pushing

[icebob]

Hi, your lib is great. I'm trying to use it in my project, but I have a problem. According to changes I want to reproduce the object and I use the currentPath and newValue fields. When I push an item to an array, the currentPath not too exact.

Example

let data = {};

let reproduced = {};

var p = ObservableSlim.create(data, true, function(changes) {
    console.log(JSON.stringify(changes));
    changes.forEach(change => {
      _.set(reproduced, change.currentPath, change.newValue);
    });
});

p.user = {
    roles: ["admin"]
};

p.user.roles.push("member");

setTimeout(function() {
    console.log("REPRODUCED OBJECT:", JSON.stringify(reproduced));
}, 2000);

Output

[{"type":"add","target":{"user":{"roles":["admin","member"]}},"property":"user","newValue":{"roles":["admin","member"]},"currentPath":"user","proxy":{"user":{"roles":["admin","member"]}}}]
[{"type":"add","target":["admin","member"],"property":"1","newValue":"member","currentPath":"user.roles","proxy":["admin","member"]}]
REPRODUCED OBJECT: {"user":{"roles":"member"}}

So when I use the roles.push the currentPath doesn't contain the index, just the property. But when I update object, the currentPath contains the property field too. I think the correct currentPath should be user.roles.1. What do you think?

Reproduce jsFiddle: https://jsfiddle.net/icebob/rtvcn685/

Thanks, Icebob

[ElliotNB]

Thank you for the feedback! Yeah I've been a little torn about the same scenario that you describe. The push technically modifies user.roles but we're also setting a value for user.roles.1 at the same time. If I recall correctly I tried to make the changes callback receive a set of changes that most closely resembles what the native Proxy object reports.

I'm heading out the door to go backpacking until Sunday so unfortunately I can't respond for the next couple days, but I'll follow-up as soon as I return.

[ElliotNB]

@icebob I'm back home now and I took a closer look at this -- your suggestion makes perfect sense. I'll make that modification today and push out the change shortly.

If you'd prefer to make the change yourself and submit the PR, please feel free to do so!

[icebob]

Hi @ElliotNB, great. I'm on vacation right now, so maybe I will able to help next week.

[ElliotNB]

@icebob I went ahead and implemented a fix for this. If you get the chance, please let me know if you notice any other issues with currentPath for arrays.

Thanks again for the feedback!

[icebob]

Thanks @ElliotNB, I'm checking...

[icebob]

Hi, I tested, and it's working properly! :+1: Maybe someone else would be useful, this is my small code which rebuilds the object by changes (uses lodash's set & unset functions):

changes.forEach(c => {
    if (c.type == "add" || c.type == "update") {
        _.set(obj.__getTarget, c.currentPath, _.cloneDeep(c.newValue));
    } else if (c.type == "delete") {
        _.unset(obj.__getTarget, c.currentPath);
    }
});