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

Added shallow comparison checks to stateful components. #70

Merged
merged 1 commit into from
Mar 20, 2016

Conversation

n1k0
Copy link
Collaborator

@n1k0 n1k0 commented Mar 17, 2016

Fixes #54.

@aduth, @magopian: could you please check this branch and see if it solves (or improves) the performance issue you're encountering? Thanks!

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 17, 2016

First tests don't make me confident this is enough :/

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 17, 2016

BAH. facebook/react#5677 (comment)

@n1k0 n1k0 force-pushed the shallow-comparison branch 2 times, most recently from 77f0278 to 15bdd17 Compare March 19, 2016 11:13
@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 19, 2016

Well, even if hackish as hell, last commit improves the situation a lot when filling large forms on the playground. Please guys provide feedback /cc @aduth @magopian

@aduth
Copy link

aduth commented Mar 20, 2016

Apologies that I've not had the time to take a close look at this. Just took it for a spin, and I can confirm that it feels much more performant for reasonably fast form input. I must say, I'm a bit concerned about the impact that including Immutable.js will have on the bundle size, as it looks as though it may add upwards of 70kb minified prior to gzipping.

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 20, 2016

I share your concerns. Though from my research, I couldn't find any better ways of performing nested object shallow comparison... If you have any idea how to implement shouldRender using vanilla js in an efficient fashion, I take it. Maybe just using deep-equal, deeper or equivalent may be just as fast but much less bloated?

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 20, 2016

Side note, would be great to have some benchmark suite as a reference. I'll setup something.

@n1k0
Copy link
Collaborator Author

n1k0 commented Mar 20, 2016

Achieving nearly the same perf boost using deeper, much better. Immutable would make sense if we wanted to use its structures everywhere, but it was definitely overkill here.

@n1k0 n1k0 force-pushed the shallow-comparison branch from eb3f54d to c07fbf1 Compare March 20, 2016 12:52
n1k0 added a commit that referenced this pull request Mar 20, 2016
Added shallow comparison checks to stateful components.
@n1k0 n1k0 merged commit e61eb0a into master Mar 20, 2016
@n1k0 n1k0 deleted the shallow-comparison branch March 20, 2016 12:57
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

Successfully merging this pull request may close these issues.

2 participants