Simpler classful component check (fixes #458)

[developit]

Instead of checking for the isReactComponent property (which is internal to React), it seems advantageous to check for render(). This improves Preact support (when using recompose with non-preact-compat components) but also importantly works with components that don't inherit from Component.

[codecov-io]

Codecov Report

Merging #463 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #463   +/-   ##
=======================================
  Coverage   88.86%   88.86%           
=======================================
  Files          53       53           
  Lines         386      386           
=======================================
  Hits          343      343           
  Misses         43       43

Impacted Files	Coverage Δ
src/packages/recompose/isClassComponent.js	100% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update adbefc0...1c9cf7b. Read the comment docs.

[billneff79]

To maintain compatibility with what he is already doing in isClassComponent, it would have to return true for React.Component but false for React.PureComponent and React.AsyncComponent, since React.Component is the only one that has the isReactComponent prop. All of them would have render, but only React.Component has setState and forceUpdate. If he doesn't care about changing it so it returns true for PureComponent then checking for render would work as well.

[billneff79]

^ Just saw your comment about being compatible with things that are classes but don't extend React.Component. Again goes back to what kind of contract he is going for with isClassCompoenent - is it just trying to return something that is not a functional component (in which case checking for render is perfect), or is it trying to check only that it is extending React.Component

[istarkov]

Thank you

[gaearon]

In case other library authors see this PR and adopt the same approach, I’m encouraging you not to.

As noted in #538 (comment), this change was actually a major change (and broke people’s code in production). It can break code because render might be on the instance rather than on the prototype in some cases. It can also break code in React Native. Those reasons are exactly why we didn’t go with this check.

Here is the exact algorithm React is using:

• Check for fn.prototype && fn.prototype.isReactComponent
• If it is truthy
  • This is a class component and must be new'd
• If it is falsy
  • Call the function without new
  • If the return value is an object and has a render method on it
    • Treat it as a class instance
  • Otherwise, treat it as a functional component

React plans to continue using this heuristic. In this sense, if for some reason your library needs to know whether a component is "newable" or not, fn.prototype && fn.prototype.isReactComponent is a safe way to check it.

I understand we might not be able to persuade Preact or others to adopt the same heuristic, but I hope this is helpful.