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

merge should return the same object when no meaningful changes were applied #13

Closed
raorao opened this issue Nov 6, 2014 · 2 comments
Closed

Comments

@raorao
Copy link
Contributor

raorao commented Nov 6, 2014

I think it would be helpful if merge returned the same object when no changes were made. Here's what I mean:

obj  = Immutable({ key: 'value' })
obj1 = obj.merge({ key: 'new value '}) 
obj2 = obj.merge({ key: 'value' }) //no meaningful change

// current behavior
obj === obj1 //false
obj === obj2 //false

// ideal behavior
obj === obj1 //false
obj === obj2 //true

Immutable.js provides this feature with their 'set' function, which I've found helpful in the past.

This allows users to run quick checks to see if anything has changed after a merge, which would, for instance, be useful in the context of react/flux. While the check can be done after the fact (using a deepEquals function or something like that), combining that logic with merge has obvious performance benefits.

Maybe both versions of merge should be included in the public API, e.g. 'merge' and 'safeMerge' -- but that may also be overkill. Interested to hear thoughts on this.

@rtfeldman
Copy link
Owner

Oh nice! I think this should be a very inexpensive check to do...I'll take a crack at it real quick.

@rtfeldman
Copy link
Owner

Changed in c7887bb

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