Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return first object if nothing is changed? #73

Closed
dantrain opened this issue Aug 29, 2017 · 2 comments
Closed

Return first object if nothing is changed? #73

dantrain opened this issue Aug 29, 2017 · 2 comments

Comments

@dantrain
Copy link

I'm looking for this behaviour:

var x = { foo: { bar: 3 }, baz: 4 };
var y = { foo: { bar: 5 } };
var z = { foo: { bar: 3 } };

var result1 = merge(x, y);
var result2 = merge(x, z);

assert(result1 === x) // => false
assert(result2 === x) // => true

Currently a new object is always created so this does not hold. Not sure if this is an unreasonable request or out of scope, I've had a look at Lodash and Ramda but can't seem to find this anywhere.

@TehShrike
Copy link
Owner

That would put deepmerge in the awkward position of having to do equality checks on everything - right now, it only compares types before copying.

Equality checks are difficult, because some people would expect object identity to be used for equality checks ({} === {} should be false) and others would only want object properties or array elements of certain primitive values compared.

I would rather push the library in the direction of always cloning everything, ala #72. It would make more sense than the current default behavior (creating a new root object, but mutating child objects on the destination object).

That way users could rely on always getting a full copy, and wouldn't have to reason about partial recursive clones.

If you do need that behavior, then I think you would be better off finding a recursive-object-equality-checking library that checks equality how you expect and doing that check before calling merge.

@dantrain
Copy link
Author

Ok perhaps I can use a deepEqual utility first and then do a deepMerge. Not sure about the performance implications but I can see how this could be an awkward mix for this library. Thanks for the prompt reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants