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 warning caused due to usage of componentWillRecieveProps #43

Merged
merged 1 commit into from
Nov 15, 2018
Merged

Remove warning caused due to usage of componentWillRecieveProps #43

merged 1 commit into from
Nov 15, 2018

Conversation

rishichawda
Copy link

Since componentWillRecieveProps will not be supported in future versions of react (v17.0), when used
with static getDerivedStateFromProps method in a component it throws a warning that it contains
legacy lifecycle method (v16.4 and above). Using shouldComponentUpdate to check props and re-render
the component provides us with a way to do the same without componentWillRecieveProps. Also, now we
don't need to pass props between themr component's methods since we can access updated props through
this.

This also 'future-proofs' react-css-themr for React.

…ieveProps

Since componentWillRecieveProps will not be supported in future versions of react (v17.0), when used
with static getDerivedStateFromProps method in a component it throws a warning that it contains
legacy lifecycle method (v16.4 and above). Using shouldComponentUpdate to check props and re-render
the component provides us with a way to do the same without componentWillRecieveProps. Also, now we
don't need to pass props between themr component's methods since we can access updated props through
`this`.

94
@rishichawda
Copy link
Author

rishichawda commented Oct 31, 2018

This PR was originally raised here. Since the old repo is no longer maintained, we're switching to the maintained fork. Thanks @markusguenther for maintaining this.

Update: I had originally raised an issue also, on the original repo, the link to which is here.

@markusguenther
Copy link
Member

Thank you for the contribution will try to review this soonish :)
And I am happy that you find the way to the fork :)

@markusguenther
Copy link
Member

Looks good to me by reading, but need to test it later on :)

@rishichawda
Copy link
Author

@markusguenther Cool. 👍

@rishichawda
Copy link
Author

@markusguenther Did you have the time to test this?

@markusguenther
Copy link
Member

I am so sorry. Had unpredictable events at work and I promise to test this in the coming two days. Plan also to adjust the build automations a bit and GreenKeeper already mentioned updates for libraries. So you can count me in for a new release in the coming two days :)

I promise 🤘🏼

@rishichawda
Copy link
Author

I am so sorry. Had unpredictable events at work and I promise to test this in the coming two days. Plan also to adjust the build automations a bit and GreenKeeper already mentioned updates for libraries. So you can count me in for a new release in the coming two days :)

Cool. I'll test it out in prod. Will raise an issue if I find something. ( And maybe a PR too if I get time. )

@markusguenther markusguenther merged commit afc0309 into FriendsOfReactJS:develop Nov 15, 2018
@markusguenther
Copy link
Member

🎉 This PR is included in version 3.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants