Skip to content

Warn if using ES6 class but not directly extending React.Component #1048

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

Closed
interactivellama opened this issue Jan 28, 2017 · 8 comments
Closed

Comments

@interactivellama
Copy link

interactivellama commented Jan 28, 2017

Now that React 16.x is taking out createClass, is there currently a way to warn if not directly extending React.(Pure)?Component. That was always my issue with ES6 classes (Button inherits Tooltip, Tooltip inherits onHoverComponent, etc.) and have been holding off on them. I'm seeing isExplicitComponent in the utils module which I believe would do this, if surfaced.

Maybe it would be an additional feature to prefer-es6-class rule:

'react/prefer-es6-class': [1, 'explicit'],

Anyone else think this is a good idea in order to promote use of higher order components for composition over classical inheritance? This would be in line with @gaearon 's How to Use Classes and Sleep at Night

@ljharb
Copy link
Member

ljharb commented Jan 28, 2017

How would you propose detecting a component that extends something else? The presence of a "render" method isn't sufficient - Backbone views have a render method, for example - and some subclasses only override lifecycle methods, or only override non lifecycle methods.

The current way this is handled is that if a component extends another component, it's ignored - precisely because there's no way to detect them.

A new generic rule that prohibited all forms of extends that aren't extending React.{Pure,}Component might be useful, though.

@interactivellama
Copy link
Author

I'm not very familiar with what AST provides, but I assumed that there's be an object name lookup and then another lookup for it's parent that would be React. It'd flakey, of course, if React was not the name of the library in the file, etc., but that would be an edge case. I'll need to AST parse some code to figure out a true suggestion.

@ljharb
Copy link
Member

ljharb commented Jan 31, 2017

Your assumption is incorrect - AST is only based on what code is typed into the file. It's utterly impossible to find "the parent" because that might be in another file, or that might be the result of random evaluation.

There's no way to know that class Foo extends Bar is meant to be a React component unless a) it's used in jsx or createElement in the file; b) it's annotated with a special jsdoc comment that this plugin supports to indicate it's a react element, or c) unless Bar is literally defined as React.Component in the same file.

Your desired rule doesn't target any of these cases. This is the type of thing that just needs to be solved with code review, not with a linter.

@jpickwell
Copy link

I like the idea of an annotation comment. The plugin could check for Component or PureComponent, and if neither of those is present (extended), then check for the annotation comment.

@ljharb
Copy link
Member

ljharb commented Feb 28, 2017

Note that this plugin already supports an optional annotation comment, but it must not require it, or else the linter rules would miss out on a lot of places it needs to be targeting.

@jpickwell
Copy link

Is there documentation for this optional annotation comment?

@ljharb
Copy link
Member

ljharb commented Feb 28, 2017

I believe it's undocumented atm, and uses jsdoc syntax.

@ljharb
Copy link
Member

ljharb commented Feb 3, 2022

In react, inheriting from something that's not React.Component or React.PureComponent is a long-ago-decided antipattern.

If you do it anyways, you can stick /** @extends React.Component */ before the class, and this plugin will pretend it's a react component.

@ljharb ljharb closed this as completed Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants