-
Notifications
You must be signed in to change notification settings - Fork 48
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
Fix componentWillUpdate
has been renamed warning
#23
base: master
Are you sure you want to change the base?
Conversation
Moves the extraction of props and eventListeners logic from `componentWillUpdate` to `componentDidUpdate`. This fixes the following warning from the console: ``` Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://fb.me/react-async-component-lifecycle-hooks for details. * Move data fetching code or side effects to componentDidUpdate. * Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. In React 17.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run `npx react-codemod rename-unsafe-lifecycles` in your project source folder. Please update the following components: Lottie ```
@felippenardi - could we have this merged in please? 🙏🏻 |
if (this.options.animationData !== nextProps.options.animationData) { | ||
this.deRegisterEvents(this.props.eventListeners); | ||
componentDidUpdate(prevProps) { | ||
if (prevProps.options.animationData !== this.props.options.animationData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is slightly incorrect. It should be this.options
here and on line 45 instead of prevProps.options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would you write the if condition here?
Pick up the changes in felippenardi#23
would be nice to have this warning removed 👍 |
Is it possible to get this merged? Would be awesome to get this error removed |
@felippenardi there is possible to get any news about the merge of this PR? |
Moves the extraction of props and eventListeners logic from
componentWillUpdate
tocomponentDidUpdate
. This fixes the following warning from the console: