search

kubevirt / web-ui

OpenShift Cluster Console UI
https://www.openshift.org
Apache License 2.0
26 stars 11 forks source link

Introduce LazyRenderer component #283

Closed mareklibra closed 5 years ago

mareklibra commented 5 years ago

Avoids eager re-rendering for frequent redux store changes.

Assumption: re-render requests are frequent, so skipping a few of them has no impact on rendered information from user's perspective.

If that assumption is not met, the use of this component is not recommended.

rawagner commented 5 years ago

@mareklibra isn't there better way to ask for rerender ? forceUpdate will skip shouldComponentUpdate which is not good IMO

mareklibra commented 5 years ago

@mareklibra isn't there better way to ask for rerender ? forceUpdate will skip shouldComponentUpdate which is not good IMO

Originally, I was thinking about using setState and letting shouldComponentUpdate decide. But the code seemed to me equal as it effectively by-passed any additional logic in the shouldComponentUpdate and could be reduced to this one.

The shouldComponentUpdate would return false unless throttling allows otherwise. And if we want guarantee to start the renderer, no matter if future props changes will come, this is equal. If this is not requested and we can rely on future props changes to "fire" the lifecycle, we can make the go "nicer" and get rid of the forceUpdate.

mareklibra commented 5 years ago

Right, in case it is used in wrong scenario (not our case). That will be resolved by the setState. I'll revert so everybody is happy :-).

rawagner commented 5 years ago

@cloudbehl @gnehapk FYI to improve your dashboard performance, i'd suggest to use LazyRenderer

cloudbehl commented 5 years ago

@rawagner https://github.com/kubevirt/web-ui/pull/296