Skip to content

fix(index.js): Replaced the deprecated componentWillPeceiveProps. #487

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 1 commit into from
Aug 29, 2019

Conversation

jackall3n
Copy link
Contributor

React has deprecated the use of componentWillReceivedProps, the static method
getDerivedStateFromProps should be used.

@aronhelser
Copy link
Collaborator

Thanks for the update! It's great to keep up with React.

Unfortunately, you have some linting problems - you can run make lint to see them, or look at the Travis output above.

@jackall3n jackall3n force-pushed the master branch 2 times, most recently from 3dec51f to c07bc7b Compare April 10, 2019 15:55
@SNikon
Copy link

SNikon commented Jun 3, 2019

@jackall3n My apologies for intruding on your PR but I believe you are replacing state with its own copy of state at every render call.
https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops
I may be incorrect here but after you're correctly checked if ariaProps changed, you should either return null if nothing changed or return the new ariaProps only.

Thanks for helping maintain the library, I'm hoping to use this in a new project

@jackall3n
Copy link
Contributor Author

@SNikon thanks, let me check it out :)

@jackall3n jackall3n force-pushed the master branch 2 times, most recently from 83d2d9c to 7e98332 Compare June 24, 2019 13:59
@jackall3n jackall3n closed this Jun 24, 2019
@jackall3n jackall3n reopened this Jun 24, 2019
@jackall3n
Copy link
Contributor Author

As per the comment by @SNikon, I've updated the lifestyle method to return null when there is no state change.


static getDerivedStateFromProps (nextProps, prevState) {
const { ariaProps } = prevState
const newAriaProps = parseAria(nextProps)
const isChanged = Object.keys(newAriaProps).some(props => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not within the scope of this PR, but it seems like this check doesn't cover the case where newAriaProps is a subset of the previous ariaProps?

E.g.,

// prevProps
a = { a: 1, b: 2, c: 3 }

// new props
b = { a: 1, b: 2 }

// returns false, but `a` and `b` actually have different props
const isChanged = Object.keys(b).some(prop => b[prop] !== a[prop])

So maybe adding a length check would do?

E.g.,

const isChanged = Object.keys(newAriaProps).some(props => {
  return newAriaProps[props] !== ariaProps[props]
}) && Object.keys(newAriaProps).length === Object.keys(ariaProps)

@gfx
Copy link

gfx commented Aug 15, 2019

Now React 16.9.0 is released, which produces warnings about componentWillReceivedProps.

@5minpause
Copy link

What's holding this from merging and releasing? Where do you need help?

@aronhelser aronhelser merged commit cfd5997 into ReactTooltip:master Aug 29, 2019
@wwayne
Copy link
Collaborator

wwayne commented Aug 29, 2019

🎉 This PR is included in version 3.10.1 🎉

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants