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

Remove passing rootNode to lifecycle methods #416

Closed
chenglou opened this issue Oct 9, 2013 · 5 comments
Closed

Remove passing rootNode to lifecycle methods #416

chenglou opened this issue Oct 9, 2013 · 5 comments

Comments

@chenglou
Copy link
Contributor

chenglou commented Oct 9, 2013

Referring to #276:

I think we should remove the rootNode being passed to componentDidMount and componentDidUpdate. For two reasons:

  1. It's unnecessarily touching the DOM. This makes the separation of React and DOM harder, something I've seen being discussed.
  2. It's inconsistent with other lifecycle methods such as componentWillUnmount. Since we're not adding rootNode to it due to reason 1, the only way to stay consistent is to remove it for the other two methods.
  3. It makes adding new arguments harder in the future.
@sophiebits
Copy link
Collaborator

+1. Also makes it more annoying to add new useful arguments if there ever are any.

@chenglou
Copy link
Contributor Author

chenglou commented Oct 9, 2013

Ops, added.

@petehunt
Copy link
Contributor

Bump, this is about to bite us with the next github sync since we added a param to componentDidUpdate(). I think we should remove this in 0.6 @zpao

@vjeux
Copy link
Contributor

vjeux commented Nov 21, 2013

We still need to document it :)

@chenglou
Copy link
Contributor Author

I'm actually doing it that's why!

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

4 participants