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

Show a better warning when accidentally returning from constructor #11381

Closed
gaearon opened this issue Oct 26, 2017 · 9 comments
Closed

Show a better warning when accidentally returning from constructor #11381

gaearon opened this issue Oct 26, 2017 · 9 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Oct 26, 2017

Based on a conversation with @vjeux:

I wrote

constructor(props) {
 return {
   something: false
 }
}

instead of

constructor(props) {
 super(props);
 this.state = {
   something: false
 }
}

and the error was super confusing, it told me that render is not defined on the component, which didn't make sense since I returned it!

We can fix this to show a different message if type has .prototype.isReactComponent but constructor() gave us something without a render method.

@gaearon gaearon changed the title Show a better warning when returning from constructor Show a better warning when accidentally returning from constructor Oct 26, 2017
@M-ZubairAhmed
Copy link

hi I would like to work on it. Could you please point me to specific direction. I would really appreciate it.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 27, 2017

I think the relevant code is in ReactFiberClassComponent. I would suggest adding a test first (e.g. try to reproduce the existing warning), then try to make changes for a better warning. Does this help?

@dylanapplegate
Copy link
Contributor

Hey, @M-ZubairAhmed, I thought this was a good starting bug and went ahead with the fix as an exercise for myself. If you don't ultimately take this bug, let me know so I can create a pull request.

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 28, 2017

@M-ZubairAhmed Have you been able to look at it yet?

@M-ZubairAhmed
Copy link

@deanbrophy yes even I have started working on it but since I am a beginner is taking time for me to complete. But I think it's not wise for me to hold it up, you are welcome to continue.
@gaearon I will keep looking out the space for more issues in future

@dylanapplegate
Copy link
Contributor

@M-ZubairAhmed Thanks! The next "good first issue" is yours.

@wood1986
Copy link

wood1986 commented Nov 1, 2017

Maybe it is not related. Basically all the components that we are writing in Class style must deal with the state. Why cannot we enforce returning an initial state in Class style instead of doing this.state = {}

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 1, 2017

Because returning something from a constructor already has a (different) meaning in JavaScript.

If you're just looking for a terse syntax, use class properties and don't define a constructor at all.

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 1, 2017

Fixed in #11395.

@gaearon gaearon closed this as completed Nov 1, 2017
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

4 participants