Fix false-positive react component classes

[devongovett]

Currently, any class with a render method is considered to be a React component. This causes issues with code-bases where not everything is React, and there are other classes besides components with render methods. These should not be documented as components by react-docgen.

This PR removes the detection of React components by render method, and requires those classes to extend from React.Component or other classes in the react module. This is obviously a breaking change, but I believe it is more correct since React component classes must extend from React.Component to be considered valid by React as well. If there are use cases that cannot be handled by this, we should accommodate them in other ways.

[danez]

Did you run into a concrete example where this does yield wrong results?
I think it was done because it is nearly impossible to catch all cases that one could do in Javascript to create a React class component. So checking for render was an easy solution and worked for most cases.

From the top of my head this change would probably fail to detect this case:

class Component extends MyCustomReactComp {

render () {}
}

where MyCustomReactComp is an imported class which extends React.Component.

So I'm not completely sure if we should change the behavior. Also I have never seen any complains about it here.

[devongovett]

Yeah we have some classes that don't use React at all that happen to have a render method and they are getting picked up as components.

I guess the MyCustomReactComp makes sort of sense but extending react components is typically not considered good practice since composition should be used instead. I could see things like react-alike libraries like preact needing this support, but we could easily add them to the whitelist.

[devongovett]

One idea could be to check if render returns JSX, similar to the detection for function components. Not sure if that would work in all cases though.

An alternative suggestion that could be considered is a property in the output for whether the class extends from React.Component so that they could be filtered out by the consumer.

[devongovett]

Any thoughts? I figured since a major is coming, now would be the time to do it, but happy to consider other options. 😄

[danez]

I think this is too restrictive, but I think we can improve what we have now.

• We could ensure, that classes which have a render method also need to extend any superclass because I think this is not valid react, because you need to extend React.Component either directly or indirectly through inheritance:

class Foo { render() {}}

• render methods need to return something otherwise it is not valid react component.
• Could we use the import-resolve functionality on the superclass to figure out if it extends React.Component through inheritance? Although if we do this, the chance is big that there are multiple components extending this superclass, so we should probably cache this resolving.

[danez]

Closing, as some functionality is now part of 5.0.0.