[New] Add no-pure-component-children rule

[rickhanlonii]

Overview

This PR adds a new rule no-pure-component-children which checks that a component extending React.PureComponent does not declare children as a proptype.

Motivation

Consider standard use of children:

<Parent>
  <Child /> // note this is sugar for React.createElement(...)
</Parent>

On every render, <Child /> !== <Child /> so Parent props will fail equality checks even though they appear to be equal. This means that if Parent is a component that extends PureComponent, it will be less performant than if it extended just Component since it will do the extra work to check props which will ultimately always be unequal.

See facebook/react#8669 for more discussion

[ljharb]

This doesn't make any sense to me. The thing that would cause rerenders is passing new children to the PureComponent - what the PureComponent itself renders is utterly irrelevant.

[ljharb]

Specifically, the linked example talks about how ComponentWithChildren is rendered - its actual implementation makes zero difference whatsoever.

This, imo, is not something that is lintable in any reliable way.

[rickhanlonii]

Thanks for taking a look!

I agree with you, what the PureComponent renders is irrelevant and passing new children to PureComponent is what would would cause re-renders. The key here is that children are always new unless they're primitives* or cached, so having a children prop on a PureComponent is a smell and could be at least warned on

And yeah, it would be better to check that the calling code doesn't pass new children but I agree that that's probably not lintable. Checking out this search shows there's a ton of PureComponents out there that are basically worthless because of children props

*I can update this to check the types, we shouldn't warn if the children proptype is a number or string

[ljharb]

Precisely because children could be memoized or primitives, is why i think this doesn't make sense as a lint rule.

[rickhanlonii]

I'll update to allow primitives, which leaves just the possibility of memoized children react elements as your argument against it?

Note also that I didn't add this to the recommended list since I did not expect it to universally apply

[ljharb]

The problem is that I think this rule will create confusion, by targeting the wrong thing.

This is a problem that should be solved at runtime by looking for excessive renders - potentially by monkeypatching PureComponent to track when they happen.

[rickhanlonii]

Yeah I agree it could be solved that way (though regressions could be an issue)

I think this also makes sense, so I'd like hear what the rest of the community thinks 👍

[Jessidhia]

A PureComponent that receives children would indeed be forced to rerender if the parent rerenders because it will receive new props -- however, if the update is happening because of a state change, it'll still reuse the previous render's props.

It's a big footgun which kinda defeats the shallow equality check on the props, but there are still valid uses... 🤔