cloning destination object makes Date value into string

[join16]

in file src/set-key-value.js, line 80,

destinationObject = JSON.parse(JSON.stringify(destinationObject));

above code occurs problem when object contains any class instances. I think handling native javascript instances should be handled when cloning object. (ex. Date instance)

How about using better way to clone object? lodash provides object cloning function and it can be a solution to solve above problem.

[rafakato]

Yes, lodash has a clone method, as does jQuery, but JSON.parse(JSON.stringify(destinationObject)); has proven to be the fastest way to deep clone a unknown object without any loops, since the function _setValue is recusive, I don't think it's a good idea to add loops to do the object cloning, nor adding dependencies.

I've made this jsperf, to show what I said. lodash clone is very slow compared to JSON stringify/parse, but we can implement a clone function like I tested.

@wankdanker what do you think?

[weshardee]

This creates some very weird behavior. Here's an example using moment.js:

const TIME_FORMAT = 'HH:mm Z';
const toMoment = (time) =>  moment(time, TIME_FORMAT);

const MAP = {
    endTime: {
        key: 'endMoment',
        default: null,
        transform: toMoment,
    },
    startTime: {
        key: 'startMoment',
        default: null,
        transform: toMoment,
    },
};

Using this map makes an object in which 'endMoment' is a string (because of the JSON stringify/parse), but 'startMoment' is an instance of a Moment object, despite both using the same transform function.

Relying on JSON to deep clone the object means we can only reliably use JSON-friendly data types in our output. And that would be fine if that was documented, but it would somewhat limit the utility of the transform handler.

Out of curiosity, why deep clone the object and reassign it to itself at all? I just ran a test removing that line from the library, and in my use case I don't see any change in behavior (except for eliminating this issue).

[rafakato]

Yeah, your point makes sense @weshardee, it is done in this way because we first made this lib for transforming JSON objects.. But it's a nice idea to go to this way, but we need more tests to check if everything will work fine. I wait your PRs :)

[gastonelhordoy]

Hey @rafakato, any update on this issue? I see @weshardee submitted a PR fixing it. Are you planning to merge it to master and release a new version any time soon?

Thanks in advance!

[rafakato]

@gastonelhordoy I've just published the version 3.0.1 with the merged PR.

[gastonelhordoy]

Cool, thanks a lot!