Eslint-plugin-react: rule proposal: PureComponent and not Component

[alexreardon]

I was thinking of writing a rule that would force extending PureComponent and not Component.

// not allowed
class App extends React.Component {...}

// allowed
class App extends React.PureComponent {...}

I would be happy to write the rules + tests

Cheers

[ljharb]

When would that rule be applied? It's not always safe to make a component into a PureComponent, and the linter rule should only be recommending it when it won't break code.

[alexreardon]

This would be a specific opt in rule for code bases that are aware of the behaviour of PureComponent.

Use case: we have a new code base and want to use PureComponent everywhere from the start. However, we currently have no way of enforcing it with eslint.

[daltones]

I like the idea. I think that's not about breaking the code or not. It's about a convention (like a good practice) in the project. It's up to you enable this rule.

When the whole project respects immutability for all data in props or state, we can say that it's a good thing to use PureComponent, so we can use this rule.

[alexreardon]

Okay great - I will try to get a pull request open soon

[ljharb]

It's definitely about breaking code - eslint rules are certainly subjective, but they should not recommend changes on existing codebases that would make it easy to accidentally break code.

If the rule can detect when a component should not be a PureComponent, I'm +1 on it. Otherwise, I'm -1.

[alexreardon]

Calculating whether or not a component is relying on mutation of existing objects seems quite difficult to catch. I appreciate the apprehension to add it as a rule as turning it on without an understanding of PureComponent might cause bugs.

Perhaps we could search for a subset of obvious issues? Although trying to come up with a list of these is providing difficult for me.

[ljharb]

To reiterate - just like prefer-stateless-function, I think prefer-pure-component would be a great rule - provided that it's very very cautious about when it warns. Since detecting mutation is hard, perhaps it would be better to enumerate some cases where the rule could know with certainty that changing to PureComponent is safe?

[alexreardon]

I am looking at it from the perspective that I want to black list using React.Component for a whole code base rather than just preferring React.PureComponent. Is that too strong an opinion?

[ljharb]

I'm not the only deciding vote, but imo, yes, that's far too strong an opinion.

[alexreardon]

I will hold off on a pull request until there is more of an agreement about whether to move forward with this one - and what moving forward would look like

[alexreardon]

Unless it is a leaf node it is really hard to tell because sub nodes might be relying on the mutation change

[ljharb]

I think you could maybe use no-restricted-properties in eslint core to forbid React.Component, without an extra rule?

[alexreardon]

That might work! I will give it a go

[alexreardon]

I can confirm that the rule is able to be enforced using no-restricted-properties

 "no-restricted-properties": ["error", {
        "object": "React",
        "property": "Component",
        "message": "Please use PureComponent instead."
    }],

[alexreardon]

I would be happy to close this issue give that the outcome is already achievable