millermedeiros / amd-utils

modular JavaScript utilities written in the AMD format
http://millermedeiros.github.com/amd-utils
142 stars 11 forks source link

Merge two objects recursively #49

Closed satazor closed 12 years ago

satazor commented 12 years ago

At the moment we have mixIn that merges two objects but not recursively. We could have something similar to mootools Object.merge

JamesMGreene commented 12 years ago

+1

millermedeiros commented 12 years ago

this is indeed useful and not hard to implement. +1

millermedeiros commented 12 years ago

one thing I realized while coding it is that since objects are passed by reference they will end up being updated/edited during the merge, so the proper way of implementing it to avoid undesired side-effects is to clone the object before merging...

what do you guys think? should it also clone arrays?

Current behavior:

var obj1 = {a: 1, b : { c: 1}};
var obj2 = {a: 2, b : { c : { d : 2 } }};
var obj3 = { b : { e : 3 } };

merge(obj1, obj2, obj3); // { a: 2, b : { c : { d : 2 }, e : 3 } }

// since objects are passed by reference this weird behavior happens
// obj2 is also affected during merge
obj1.b === obj2.b; // true
obj2.b.e === 3;    // true

PS: I didn't merged this into master yet since current behavior is funky

JamesMGreene commented 12 years ago

Yes, I would say both objects and arrays should be cloned. Some people would then say, "what about object/class instances?"; I, however, have never found myself wanting to merge any structure that contains such things... only plain data objects.

JamesMGreene commented 12 years ago

Also, you may find some of the discussion interesting in this JSFixed thread: https://github.com/JSFixed/JSFixed/issues/16

satazor commented 12 years ago

bump!

millermedeiros commented 12 years ago

@satazor thanks for the "bump!", I've been busy lately and forgot about it. Merged into master.

JamesMGreene commented 12 years ago

@millermedeiros Is the committed behavior still funky?

millermedeiros commented 12 years ago

@JamesMGreene now it clones all the Object/Array/Date/RegExp, so it shouldn't cause any side-effects. see specs for more info

JamesMGreene commented 12 years ago

Greats, thanks!

satazor commented 12 years ago

@millermedeiros no problem! I was long waiting for this function :D