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

updating componentWillMount W2-lesson-Plan.md #238

Closed
prof-xed opened this issue Sep 26, 2018 · 12 comments
Closed

updating componentWillMount W2-lesson-Plan.md #238

prof-xed opened this issue Sep 26, 2018 · 12 comments
Assignees

Comments

@prof-xed
Copy link

componentWillMount is no longer in use since 16.3v, as one of the students mentioned and by following that in the React official docs too.
what we may apply in the lesson-plan instead is: facebook/react#7671 (comment)

or we may get back to the constructor method with using the super key word instead?

@isaachinman
Copy link
Member

We should remove the comment about componentWillMount being used in SSR. This is an outdated comment now - people used to use it for SSR, but there are much better options currently.

I don't understand what this has to do with using constructor or super though, can you explain that?

@prof-xed
Copy link
Author

prof-xed commented Oct 11, 2018

yes for sure in order to initialize a state we should use componentWillMount and today it's not there any more, so I saw two different solutions for this:

  1. we will go back to the constructor
    if we used the constructor as the old days (before I knew what react is even 😅) we will use the .bind method for binding the methods inside of our class component, or in best case we will have to have a very big constructor method to handle it.
  2. other option is to set a plain method inside the class by it self and invoke it in the state property it self.

Documentations Update according to the second point I think we should use the constructor instead of the componentWillMount.

Thanks for noticing me about the SSR Part, I try to will take care of that 🙏

@isaachinman
Copy link
Member

You should never be initialising state in componentWillMount, ever.

You should either use a constructor or a class property to initialise state (I think when you say "plain method" you mean class property).

@prof-xed
Copy link
Author

Oh Nice to know, every day is a school day 💐, because I was doing it 🙃, in the project even.
But why? just in case somebody asked me in the future.

How ever it has been deprecated now, but what about componentDidMount does it have the same issue?

@prof-xed
Copy link
Author

One more thing, is it a good Idea to use the wiki tab for a non-static things such as the easy guides?
https://github.com/Jawhar-B/React/wiki/MobX-&-creat-react-app

Because I think it's a better use of leaving there out, while it's there for this propose.

so in this way we will have all the static parts to work on in collaborate maybe, and the docs we may just make it there for an easy edit follow and detect by maintainers.

@isaachinman
Copy link
Member

Can you paste an example of what exactly you were doing inside componentWillMount to initialise state?

Generally speaking, it's bad practice because it's going to result in an unnecessary render cycle, whereas using a constructor or class property will simply allow the component to mount with the correct state on the first cycle.

I've never used GitHub wikis, but it seems like a good place for tutorials and tips.

@isaachinman
Copy link
Member

I should be clear though - the state initialisation thing is mostly a concern because it shows incomplete understanding of what React is actually doing, and how you should develop a component-based application. One extra render cycle is basically a non-issue in all practical cases.

@prof-xed
Copy link
Author

an example of the project hyfer I was in:
https://github.com/Jawhar-B/hyfer/blob/master/src/App.js#L170

@prof-xed
Copy link
Author

I will correct the spelling and try to maintain the language grammars there because I didn't study English as people doing usually, and then I will update the wikis here, I will make a note to @gijscor to make it for maintainers only access.

@isaachinman
Copy link
Member

Thanks for that example. In those kind of cases, where you are checking cookies and doing other things, it would make sense to use the constructor, not componentWillMount.

@isaachinman
Copy link
Member

Good luck with the wiki, as always just let me know if you need any help.

@prof-xed
Copy link
Author

I appreciate your help, thanks a lot 😃

prof-xed pushed a commit to prof-xed/React that referenced this issue Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants