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

Fix/4759/replace unsafe methods #4768

Conversation

KostiantynPopovych
Copy link

Closes #GH-4759

Referring to Strict Mode - remove UNSAFE_componentWillMount & UNSAFE_componentWillReceiveProps, also update some devDependencies - react, react-dom, react-test-renderer 18.2+ versions.

@codejunkienick
Copy link

@erikras any possibility this could get released in any foreseeable future?

return null
}

prepareEventHandlerIfNecessary(nextProps: Props) {

Choose a reason for hiding this comment

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

What is the reason for putting this method into the component state?

And what is getDerivedStateFromProps doing if it always returns null?

Suggested change
prepareEventHandlerIfNecessary(nextProps: Props) {
prepareEventHandlerIfNecessary = (nextProps: Props) => {

Copy link
Author

Choose a reason for hiding this comment

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

Hey @sbaechler, I'm so sorry for such a long reply.

  • the reason for putting methods into the state object is that using this approach, we will be able to call methods like this in the getDerivedStateFromProps method (it is static, so we don't have access to this);
  • in this implementation, getDerivedStateFromProps only calls the methods in the proper phase. Returning null from this method is required when we don't want to make any changes to the state object;

I have upgraded my initial solution by adding a check if we need to perform the operation in the getDerivedStateFromProps method. Previously, the componentWillReceiveProps has been called only during the updating phase, but the getDerivedStateFromProps is called during the mounting and updating phases, so we don't really want to call our method inside the getDerivedStateFromProps during the mounting phase because it will change the previous working behavior (even if the tests are passing).

The diagram shows the methods execution flow.

@KostiantynPopovych
Copy link
Author

Closing due to side-effects in the getDerivedStateFromProps method. UNSAFE_componentWillReceiveProps should be replaced with componentDidUpdate, but it also will affect a lot of logic in other class methods.

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.

None yet

3 participants