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

Keep reference equality if possible in update() #6353

Closed
wants to merge 7 commits into from
Closed

Keep reference equality if possible in update() #6353

wants to merge 7 commits into from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 26, 2016

I merged #4968 locally and then made a few changes on top that I thought would be welcome:

I figured I’d run this by Travis as well so I’m submitting it as a PR.

Credits for making this change happen go to @davidmason, I’m just tweaking minor things.
I intend this to land in 15 RC3.

davidmason and others added 6 commits September 26, 2015 23:25
For $set and $apply it is cheap and easy to check whether the new value will
be identical (===) to the old value and create a new object only when this is not
the case. This will reduce excessive renders when using the pureRenderMixin.

The same should be possible for for merge, but is more complicated so is not
attempted here.
@facebook-github-bot
Copy link

@gaearon updated the pull request.

@gaearon
Copy link
Collaborator Author

gaearon commented Apr 1, 2016

Ultimately we made a decision to not ship any more changes to update(). Our current plan is to declare it legacy and let the community pick it up and maintain and evolve its code.

I’m closing this PR for that reason. If someone wants to fork update() and contribute this fix, they are welcome! In fact it would probably be best to contribute to this existing update() fork: https://github.com/kolodny/immutability-helper. We’ll be happy to link to it if that’s what the community ends up using.

Thank you for understanding!

@facebook-github-bot
Copy link

@gaearon updated the pull request.

@gaearon gaearon deleted the update branch April 1, 2016 17:31
@gaearon gaearon removed this from the 15.x milestone Apr 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants