Skip to content

Conversation

Joshuabaker2
Copy link
Contributor

This should go a long way to resolving #6. For large datasets this won't be very performant, as it needs to do an O(n) check for color, but it is better than the current setup where it has to redraw every time. Suggested improvements for improving algorithmic performance (not included in this PR) would include sending the specific indices of the data points that are different.

@gor181
Copy link
Collaborator

gor181 commented Jul 20, 2016

Hey @Joshuabaker2,

Thank you for taking time to improve this lib. I'm definitely accepting the PR.
I'm also not a fan of direct value comparison, however I think should do unless people start passing a really massive data sets.

However even in that case the comparison bails out on the first value mismatch.

Thanks once again!

@gor181 gor181 merged commit 29cf2d7 into reactchartjs:master Jul 20, 2016
@Joshuabaker2
Copy link
Contributor Author

Great, thanks! Just a note - comparing objects equality always return false, even if their contents are the same - so the shouldComponentUpdate that you had before will always fire. Here's an example:

> foo = {a: 1}
{ a: 1 }
> bar = {a: 1}
{ a: 1 }
> foo === bar
false

Cheers!

@gor181
Copy link
Collaborator

gor181 commented Jul 20, 2016

Hey,

True as it uses referential equality.

Thanks!

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