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

Add shouldComponentUpdate to major components. #1123

Merged
merged 3 commits into from
Feb 25, 2020
Merged

Add shouldComponentUpdate to major components. #1123

merged 3 commits into from
Feb 25, 2020

Conversation

STRML
Copy link
Collaborator

@STRML STRML commented Jan 13, 2020

feat(render): Add shouldComponentUpdate to ReactGridLayout & GridItem.

This improves performance quite a bit in most cases, is implemented safely
to avoid breakage on equal arrays and objects.

To do so, while avoiding the maintenance burden of keeping a keylist
in sync with propTypes, we read propTypes using babel-plugin-preval
then construct a function to do the fastest possible comparison.

This is a lot faster than isEqual while avoiding unnecessary
comparison of e.g. children.

Supersedes #1032 - pinging @zaykaalexander.

@STRML STRML requested review from daynin and n1ghtmare January 13, 2020 20:26
@github-actions github-actions bot added deps use this label for changes in the project's dependencies infrastructure labels Jan 13, 2020
@STRML STRML mentioned this pull request Jan 13, 2020
zaykaalexander and others added 2 commits January 22, 2020 11:56
#760

1) Add shouldComponentUpdate for ReactGridLayout
2) Add shouldComponentUpdate for GridItem
3) Fix when items are dragging not from own position, but from zero top left
This improves performance quite a bit in most cases, is implemented safely
to avoid breakage on equal arrays and objects.

To do so, while avoiding the maintenance burden of keeping a keylist
in sync with propTypes, we read propTypes using babel-plugin-preval
then construct a function to do the fastest possible comparison.

This is *a lot* faster than isEqual while avoiding unnecessary
comparison of e.g. `children`.
@STRML
Copy link
Collaborator Author

STRML commented Jan 22, 2020

Resolved conflict with #1127 / #1128

@n1ghtmare
Copy link
Collaborator

This looks ready to be merged. @STRML are there issues with it? I've tested it and everything seems ok. Need some help with it?

Copy link
Collaborator

@n1ghtmare n1ghtmare left a comment

Choose a reason for hiding this comment

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

Really cool! This should be much faster. I've tested it locally and I don't see any issues. Is there a specific area this change could affect?

@STRML
Copy link
Collaborator Author

STRML commented Feb 17, 2020 via email

@igorsha
Copy link

igorsha commented Feb 26, 2020

Hi! thanks for great component. But after adding shouldComponentUpdate shallow comparison, when i'am using ResponsiveReactGridLayout it is is won't rendering at all! (obliviously why) i'am using dynamic count of elements with array inside body, like this ->

{_.map(forms, (Form) => ( <div key={Form.model.code} data-grid={{ x: 1, y: 0, w: 16, h: 12 }}> <Form /> </div> ))}

thanks)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps use this label for changes in the project's dependencies enhancement infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants