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

Removed use of tempArray #114

Closed
wants to merge 1 commit into from
Closed

Conversation

lukashlavacka
Copy link

@lukashlavacka lukashlavacka commented Sep 30, 2017

After updating my project to [email protected] I experienced strange issue where position of handles would be wrong on initial load. After debugging I determined the issue is caused by re-use of tempArray to generate some arrays. As far as I know react uses simple object equality === when detecting changes of props and state. Because the array was re-used in some cases react would fail to detect changes in style object.

Fixes #115

Re-use of tempArray to to generate arrays was causing issues with react and change detection that uses simple object equality `===`. Because the array was re-used in some cases react would fail to detect changes in style object
@hellosmithy
Copy link

This fixes a bug when upgrading to React 16. Any chance of getting this merged?
The re-use of tempArray seems like an unnecessary optimisation anyway.

Copy link

@misino misino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that preserving tempArray is not necessary and is probably a premature optimization.
The PR solves issue perfectly.

@tobiastornros
Copy link

Please merge this!

@misino
Copy link

misino commented Feb 12, 2018

@mpowaga Hi, would it be possible to merge this fix? Because it is critical for supporting React 16.

@mpowaga
Copy link
Contributor

mpowaga commented Feb 28, 2018

The same issue has been solved in #121 but thanks for sending this PR @lukashlavacka!

@mpowaga mpowaga closed this Feb 28, 2018
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.

5 participants