New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New option to ignore preference and enforce only functional components. #2511
Comments
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. |
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. |
If there is an appetite for this change in |
@ljharb unless this disallows class component it does not provide the desired feature. |
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.
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. |
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.
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:
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. |
@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. |
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. |
Could be, but could be not. Depends if she/he would ignore the convention because:
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
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. |
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. |
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
If there was such a rule, I'd use it too :P. |
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.
The text was updated successfully, but these errors were encountered: