Eslint-plugin-react: Add a rule to enforce extending React.PureComponent instead of React.Component?

[Gaafar]

Would it make sense to add a rule to enforce components to be pure i.e: extend React.PureComponent

This would be very useful in redux projects where you want to enforce all components to rely on shallow prop comparison.

[ljharb]

I'm unclear why you'd ever want to force all components to do that; PureComponent is a blunt instrument that blocks the entire subtree from updating, including in response to context changes. There's rarely a codebase where every component in the app can be pure and also correct.

I'd be concerned about how often a rule like this would have to be overridden.

[Gaafar]

We're working in a relatively large team and I want everyone to use PureComponent by default to avoid a lot of unneeded rendering. If a component is not pure in that sense (which we're trying to avoid), I want to make it explicit by adding a comment to disable this rule. Does this seem like a valid use case?

[ljharb]

Airbnb has learned the hard way that overusing PureComponent actually hurts performance. Using PureComponent is a bad idea unless the component typically renders more than once with the same referentially equal props, because the shouldComponentUpdate logic incurs a cost.

I’d urge you to reconsider indiscriminately applying a very fine-trained optimization technique as the default.

[hedgepigdaniel]

Do you have a link to support the finding that PureComponents hurt performance?

The way I tend to use them is to enforce that all props are immutable values (e.g. immutable.js). This way if any node changes in the props, then only each branch of the component tree where a changed value has passed though rerenders, instead of the entire tree as with regular Component classes. I'm sure there is a balance between constant/algorithmic factors, but if you imagine a large tree of components then this is an algorithmic improvement.

I haven't tested this, but it seems to make sense?

[ljharb]

@hedgepigdaniel not a link, just airbnb’s anecdotal experience.

[foray1010]

I doubt that PureComponent hurt performance as it is just shallow compare of props and state (https://github.com/facebook/react/blob/4fe6eec15bc69737910e5c6c749ebc6187d67be0/packages/react-reconciler/src/ReactFiberClassComponent.js#L259-L263). There are much more costly codes in js.

As a code reviewer/programmer, I prefer to reduce time on deciding where to use PureComponent to optimize things, it should be fine to use it as default

[ljharb]

@foray1010 in fact it does, because the comparison itself is slow. Renders can happen thousands of times in a minute.

It's totally fine to selectively use, or not use, a PureComponent - which is why a naive rule such as this wouldn't serve anyone.

I'm going to close this unless other collaborators have a different opinion.

[alexmnv]

This would be very useful in redux projects

Actually, it's the other way around. Because the HOC produced by connect() with default options already impelements shouldComponentUpdate logic with shallow state props comparison. See docs. So making the wrapped component extend PureComponent will have no effect other than double props comparison.

[shakhbulatgaz]

Can't we inplement a lighter version of the suggested rule?
I mean, at least advice to use PureComponent, if no props.children is found.