facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.55k stars 46.42k forks source link

Improve error when you've forgotten to extend React.Component #10103

Closed jamiebuilds closed 6 years ago

jamiebuilds commented 7 years ago

When you forget to extend React.Component you get the following error:

TypeError: Cannot call a class as a function
    at _classCallCheck (null.js:7:99)
    at Hello (null.js:11:5)
    ...react internals...

[Example]

It would be nice if in dev you could do a check for Component.prototype.render and if it exists, provide a better warning along the lines of:

Warning: Using a class with a render method as a function, did you forget to extend React.Component?

I believe it would happen in ReactCompositeComponent.js.

jamiebuilds commented 7 years ago

Note that the error is much worse when you're compiling classes without _classCallCheck because this is just undefined

aweary commented 7 years ago

Thanks for the feature request @thejameskyle! I agree that a better error here would provide a better developer experience. It also looks like Fiber will get caught in an infinite loop if you try to render a class that doesn't extend React.Component, so it might even be necessary.

Here's an example demonstrating that. Be warned, it will crash or freeze your browser/tab. Click at your own risk

cc @gaearon @acdlite ^

ReactCompositeComponent might be the right place for this, but we'd need to do it outside the DEV block since it should throw an error and we want to maintain the same invariants for DEV and PROD.

acdlite commented 7 years ago

The infinite loop is a known issue with error handling.

acdlite commented 7 years ago

Will fix before release

aweary commented 7 years ago

Thanks, @acdlite! Do you know if the fix will address @thejameskyle's request? Or would a better error for non-extending classes be orthogonal to your error handing fix?

acdlite commented 7 years ago

Yeah, fixing the infinite loop bug would not address the lack of a helpful error message. Good candidate for an external PR. Should also account for React.PureComponent.

swyxio commented 6 years ago

gonna give it a shot... first contribution

swyxio commented 6 years ago

i am a bit confused. ReactCompositeComponent no longer seems to be there and only exists in tests. Was it renamed somehow?

gaearon commented 6 years ago

That logic is now in ReactFiberClassComponent. Old engine is gone.

swyxio commented 6 years ago

Ok I think I have a good handle on this. Upon deeper inspection the error happens earlier in the lifecycle than in ReactFiberClassComponent so I am placing the invariant higher up than originally suggested.

(The error actually gets thrown in mountIndeterminateComponent and so doesn't even reach mountClassInstance from ReactFiberClassComponent several lines lower)

As for what to check, i'm going with simply !(workInProgress.type.prototype && workInProgress.type.prototype.render) and that seems to work (i.e. not throw) for both pure functional components as well as extended react components while throwing for plain JS classes which I think is exactly what we want.

swyxio commented 6 years ago

for anyone else watching we made this into a warning instead as requested by Dan. also added a test

gaearon commented 6 years ago

Fixed in https://github.com/facebook/react/pull/11168.