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

Deprecated UNSAFE_componentWillReceiveProps #380

Closed
cibulka opened this issue Jun 8, 2021 · 14 comments
Closed

Deprecated UNSAFE_componentWillReceiveProps #380

cibulka opened this issue Jun 8, 2021 · 14 comments

Comments

@cibulka
Copy link

cibulka commented Jun 8, 2021

Hi, thanks for a very helpful library!

I've turned "React Strict mode" today and was greeted with following warning: "Warning: Using UNSAFE_componentWillReceiveProps in strict mode is not recommended and may indicate bugs in your code. See https://reactjs.org/link/unsafe-component-lifecycles for details."

This hook is used in the wrapper function of your library and judging by your comment - If someone knows a better way to replace this with sync hook that can dispatch before render let me know - you probably already know that this method will be deprecated.

Just wanted to ask what are your plans with this then? Thanks again, P!

@kirill-konshin
Copy link
Owner

Well, so far I haven't found a better alternative.

@kirill-konshin
Copy link
Owner

Here's some brief explanation:

This is normal. I am aware of this issue. The code works though.

The problem is that we need to dispatch an action during the render. componentWillReceiveProps does exactly that.

getDerivedStateFromProps does not have access to instance fields, where the store resides. Potentially we can put the store in the state instead, so that GDSFP function will have access to it. Feel free to submit a pull request.

Mention @SergiiBurgazlieiev @kamleshkatpara #387

@SergiiBurgazlieiev
Copy link

SergiiBurgazlieiev commented Jun 17, 2021 via email

@Giridhar108
Copy link

А что мне делать с этой ошибкой на проде?

@kirill-konshin
Copy link
Owner

Nothing. It is working. It is surely annoying but it does not affect anything. The fix is on the way.

@adrenaline681
Copy link

I'm getting the same error. Is there an update for this fix?

@alguerocode
Copy link

i hope getting update sooner

@ghost
Copy link

ghost commented Aug 30, 2021

@kirill-konshin What about using getSnapshotBeforeUpdate? It will not be triggered before a render(), but it will be triggered before the rendered output is commited to the DOM. Does this help?

@kirill-konshin
Copy link
Owner

@kirill-konshin What about using getSnapshotBeforeUpdate? It will not be triggered before a render(), but it will be triggered before the rendered output is commited to the DOM. Does this help?

It won't help because we need to dispatch hydration action after we received props but before render. getDerivedStateFromProps does not have access to context, but I have few ideas how this can be fixed.

fostyfost added a commit to fostyfost/next-redux-wrapper that referenced this issue Aug 30, 2021
@fostyfost
Copy link
Contributor

@kirill-konshin please take a look at my PR. Perhaps this is what we need. #413

@ghost
Copy link

ghost commented Aug 30, 2021

@kirill-konshin What about using getSnapshotBeforeUpdate? It will not be triggered before a render(), but it will be triggered before the rendered output is commited to the DOM. Does this help?

It won't help because we need to dispatch hydration action after we received props but before render. getDerivedStateFromProps does not have access to context, but I have few ideas how this can be fixed.

Oh so getSnapshotBeforeUpdate will not be able to hydrate the state before react commits to the DOM?

Also, getSnapshotBeforeUpdate does have access to context if that helps

@kirill-konshin
Copy link
Owner

Good job @fostyfost! Looks like tests are passing, can you please manually verify that things still work and there are no warnings in console. Once you confirm, I will merge and do my part of regression testing and then I'll release.

@alguerocode @adrenaline681 @Giridhar108 @SergiiBurgazlieiev @cibulka you guys are most welcome to help to verify if fix works.

@kirill-konshin
Copy link
Owner

Released 7.0.4

@kirill-konshin
Copy link
Owner

FYI I've created a new PR to get rid of old class-based component in favor of hook: #419

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

No branches or pull requests

7 participants