Skip to content

Topic/lifecycle methods #39

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

Merged
merged 2 commits into from
Aug 31, 2015

Conversation

ethul
Copy link
Contributor

@ethul ethul commented Aug 29, 2015

Some of the lifecycle methods did not pass along the arguments from React. I've added these arguments. I realize this is a breaking change and I am definitely open to discussion. But I thought they might be handy.

ethul added 2 commits August 29, 2015 13:41
 - componentWillReceiveProps(nextProps)
 - shouldComponentUpdate(nextProps, nextState)
 - componentWillUpdate(nextProps, nextState)
 - componentDidUpdate(prevProps, prevState)
@paf31
Copy link
Contributor

paf31 commented Aug 31, 2015

They don't, but you can get them with getState and getProps. This might be better though.

@ethul
Copy link
Contributor Author

ethul commented Aug 31, 2015

I think using getState and getProps would give a different result in these lifecycle methods.

For example in componentWillReceiveProps if you did getProps, I think this would yield the current props. But the props passed in as a param would be the next props.

Same goes for the others. For instance, check out the code example for shouldComponentUpdate.

@paf31
Copy link
Contributor

paf31 commented Aug 31, 2015

Ah, makes sense.

paf31 added a commit that referenced this pull request Aug 31, 2015
@paf31 paf31 merged commit e1037c9 into purescript-contrib:master Aug 31, 2015
@paf31
Copy link
Contributor

paf31 commented Aug 31, 2015

Thanks!

@ethul
Copy link
Contributor Author

ethul commented Aug 31, 2015

Welcome, thank you for the merge!

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

Successfully merging this pull request may close these issues.

2 participants