Eslint-plugin-react: New option to ignore preference and enforce only functional components.

[cphoover]

With the advent of hooks you no longer require class-based components to handle state. There should be an option to enforce using only function components and forbidding the use of class-based components altogether.

[ljharb]

Such a rule would also need a way to block functional components, since hooks interfere with testing and occasionally, readability.

I'm not sure this would be a valuable rule in either case.

[cphoover]

I disagree with hooks impacting readability (personally I think functional components are way easier to reason about). However, If just for completeness-sake, I agree, If you have an option for functional-components only there should also be a rule class-components only.

[cphoover]

If there is an appetite for this change in eslint-plugin-react I can try and find some time to contribute this week.

[ljharb]

I think #2414 which fixes #690 might address this, which would make this a duplicate of #690?

[cphoover]

@ljharb unless this disallows class component it does not provide the desired feature.

[ljharb]

I can't conceive of how it'd be practical to have a rule that disallows class components (which are quite often more readable than hooks-based components, based on my experience) unless it also had an autofix that seamlessly converted in both directions between the two.

If you want to try to build that, I'd be happy to review it - but I suspect that the nature of JS as a language would make it pretty impossible.

[GeorgeTaveras1231]

I can't conceive of how it'd be practical to have a rule that disallows class components (which are quite often more readable than hooks-based components, based on my experience) unless it also had an autofix that seamlessly converted in both directions between the two.
If you want to try to build that, I'd be happy to review it - but I suspect that the nature of JS as a language would make it pretty impossible.

What makes this difficult is the desire to have autofix in place. I think having an optional rule that enforces the use of functional components is both useful and reasonably attainable. Autofix would be a nice to have but I wouldn't consider it a blocker. You can take one of two heuristical approaches to detect violation of this rule.

1. Look for a class with a #render method that returns JSX. (Won't always work since its possible to delegate the return value to another function [Such as render() { return this.renderThing() }] , and recursively flattening functions to determine the final return value will be difficult).
2. Look for a ClassDeclaration with a superClass of Component (Referring to babel-eslint9 ast demonstrated in astexplorer.net) -- and resolving the component reference to determine if it's imported from 'react'. This approach also has limitations (dynamic superclass? via class MyComponent extends Mixin(React.Component) -- this is not a common pattern anyways).

Nonetheless, I'm surprised by your pushback @ljharb. I understand you personally feel like class-based components are more readable, but honestly, this is the first I hear someone say that -- most people I've talked to about this and most articles I've read suggest the opposite is the popular opinion on the matter.

[robertjk]

I can't conceive of how it'd be practical to have a rule that disallows class components

For example in my company we have a team of people who have been developing in React for no more than 2-3 years. Hardly any of us has ever used class components and understands them. I'd like to prevent having class components in our code base mostly for that reason - most of them team wouldn't be able to understand them.

which are quite often more readable than hooks-based components, based on my experience

I myself have seen and understand class components (though am not proficient in them). I personally find function components with hooks more readable. Even bigger advantage of function components is how easy it is to abstract some logic into a hook and put it in a separate file, or make it shared between multiple components.

Generally since starting to write function components with hooks, I have never felt a need to write any class component, nor have ever been limited by that.

Another thing is that ESLint philosophy is to be completely customizable. I cannot imagine anyone writing code like this:

for( let item of items ){item.sell() ;item.print() ;}

But it's possible to enforce a coding style like this using ESLint rules. I find that nice approach, as opinions on coding style vary more than we usually imagine.

[ljharb]

@robertjk if your team truly doesn't use or understand class components, when would they create one such that a rule would warn on them?

[robertjk]

@robertjk if your team truly doesn't use or understand class components, when would they create one such that a rule would warn on them?

Likely they won't, but new developers join and I'd like to ensure no one in the future creates any class components in the code base. We actually had one developer who was the opposite - he used only class components all his career, also he didn't understand function components and hooks. He was developing one project on his own and he used class components exlusively in that project. He's not working with us anymore, but another person like this can possibly join in the future.

Also I can imagine one of our current developers doing this, if he has big troubles implementing something using function component, but then finds an old article explaining how to do it using class components. That would apply especially to more junior people. It should be caught doing code review, but I'm not doing code review for every single PR and I'd rather enforce that in the linter to be safe.

[ljharb]

If that developer would ignore convention, wouldn’t they just ignore the eslint rule?

For surfacing those issues tho, sure, but at that point you could also block React.Component from being accessed or imported.

[robertjk]

If that developer would ignore convention, wouldn’t they just ignore the eslint rule?

Could be, but could be not. Depends if she/he would ignore the convention because:

1. He knows the convention but consciously breaks it
2. He simply doesn't know the convention

And for me one of the linter's role is aiding in problem no. 2. Linter configuration other than automating enforcement of project/company's coding standards, is also a way of communicating them, so that we don't have to write long style guides and/or have it memorized and explaining to every new developer verbally. The tool can ease that for us.

Also, even if he's in situation no. 1 and consciously breaks it, it's more likely to be caught in code review, if he's putting some // eslint-disable comments, than when he can submit the code without them. I for example ask every single time in code review, what is the reason for ignoring an ESLint rule, when I spot such a comment.

you could also block React.Component from being accessed or imported

That's a nice idea. I might use it to enforce that, until a more straightforward rule is written.

Still I think it would be good to have a more straightforward rule to enforce what type of components you want to have in your project. From my personal experience (and reading other people comments in this thread seems to confirm that), many people use one of the styles of defining components but not the other, and would benefit from such a rule.

[ljharb]

While I agree with the benefit the rule would offer you, I'm very hesitant to introduce a rule that blindly blocks an important feature of the language/React. We don't have one that blocks createReactClass components either, despite those being deprecated and obsolete.

[robertjk]

I'm very hesitant to introduce a rule that blindly blocks an important feature of the language/React.

Why do you say "blindly"? There are people, who might want to block it quite consciously.

ESLint core rules allow you to block almost any single part of the JavaScript langue (in the most complex form using no-restricted-syntax) rule; even the for loop if that's your wish. The philosophy of their rules seems to be - you're in control, and you can enforce any coding style you'd want. Do you think that's a bad approach? - That the more configurable the rules are the better.

We don't have one that blocks createReactClass components either, despite those being deprecated and obsolete.

If there was such a rule, I'd use it too :P.