Skip to content

Remove warning caused due to usage of componentWillRecieveProps - #43

Merged
markusguenther merged 1 commit into
FriendsOfReactJS:developfrom
rishichawda:update-lifecycle-method
Nov 15, 2018
Merged

Remove warning caused due to usage of componentWillRecieveProps#43
markusguenther merged 1 commit into
FriendsOfReactJS:developfrom
rishichawda:update-lifecycle-method

Conversation

@rishichawda

Copy link
Copy Markdown

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

rishichawda commented Oct 31, 2018

Copy link
Copy Markdown
Author

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
Copy Markdown
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
Copy Markdown
Member

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

@rishichawda

Copy link
Copy Markdown
Author

@markusguenther Cool. 👍

@rishichawda

Copy link
Copy Markdown
Author

@markusguenther Did you have the time to test this?

@markusguenther

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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