Skip to content
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

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

Closed
jamiebuilds opened this issue Jul 4, 2017 · 12 comments
Closed

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

jamiebuilds opened this issue Jul 4, 2017 · 12 comments

Comments

@jamiebuilds
Copy link

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
Copy link
Author

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

@aweary
Copy link
Contributor

aweary commented Jul 5, 2017

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
Copy link
Collaborator

acdlite commented Jul 5, 2017

The infinite loop is a known issue with error handling.

@acdlite
Copy link
Collaborator

acdlite commented Jul 5, 2017

Will fix before release

@aweary
Copy link
Contributor

aweary commented Jul 5, 2017

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
Copy link
Collaborator

acdlite commented Jul 5, 2017

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
Copy link
Contributor

swyxio commented Oct 6, 2017

gonna give it a shot... first contribution

@swyxio
Copy link
Contributor

swyxio commented Oct 6, 2017

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

@gaearon
Copy link
Collaborator

gaearon commented Oct 6, 2017

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

@swyxio
Copy link
Contributor

swyxio commented Oct 8, 2017

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
Copy link
Contributor

swyxio commented Oct 14, 2017

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

@gaearon
Copy link
Collaborator

gaearon commented Oct 31, 2017

Fixed in #11168.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants