turnilo icon indicating copy to clipboard operation
turnilo copied to clipboard

Replace React.Component with React.PureComponent

Open pwolaq opened this issue 6 years ago • 3 comments

Throughout the project I have seen PureComponent being used only once. Replacing Component with PureComponent should work seamlessly in most of the cases, giving a nice performance boost as a result (especially after getting rid of bind in #203).

Moreover, there are components that do not have state or use lifecycle methods, meaning that they can be replaced with functional components.

pwolaq avatar Oct 24 '18 22:10 pwolaq

+1 for functional components. That would make for cleaner code. Fixing low hanging fruits would be great, but a lot of components need some rethinking. Probably we could simplify more components than it seems.

-1 for PureComponent - unless profiled I would advise against it. Speed benefits are not so clear (even ref checks can be costly) and could lead to some weird bugs (also due to current state management quality in components :()

adrianmroz avatar Oct 24 '18 22:10 adrianmroz

I agree with @adrianmroz, premature optimization is an antipattern. And as long as we do not have any performance automated tests the optimization is pointless.

What we should do with second part of the issue (pure functional components)? We could create more specific issues for particular Turnilo components, or eliminate technical debt as a side effect of other features.

mkuthan avatar Dec 12 '18 08:12 mkuthan

Converting stateless component to functional component is a mechanical task and is done alongside any other issues. I think there is no need to tracking it in separate issue.

adrianmroz avatar Dec 12 '18 08:12 adrianmroz