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

Migrate from legacy to new React lifecycle #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Migrate from legacy to new React lifecycle #127

wants to merge 1 commit into from

Conversation

rbardini
Copy link

React will soon deprecate componentWillMount, componentWillReceiveProps and componentWillUpdate methods due to their potential misuse, especially once async rendering is launched. In fact, enabling strict mode already warns about these unsafe lifecycle methods usage.

This PR moves module loading logic from componentWillMount to componentDidMount, which is the recommended upgrade path in this case.

@tajo
Copy link
Collaborator

tajo commented Jun 29, 2018

Did you test this with regards of SSR (cWM is called but cDM not)?

@rbardini
Copy link
Author

rbardini commented Jul 8, 2018

I assumed tests were already covering this. Are there any use cases missing in this regard?

@mr-pinzhang
Copy link

isn't this should be migrate to constructor as new lifecycle suggested?

@roccoluke
Copy link

Any timeframe as to when to expect this to me merged?

@amakhrov
Copy link

amakhrov commented Aug 6, 2018

@tajo @jamiebuilds any feedback on that?

@tajo tajo closed this Aug 9, 2018
@tajo tajo reopened this Aug 9, 2018
@titouancreach
Copy link

@bjminhuang I don't think so. If you do that, you'll potentially add a side effect (this.setState) in the constructor that is not recommended. The React way for doing this is componentDidMount

@the0neWhoKnocks
Copy link

Would really like to see this get merged. Currently trying to integrate this with a project on React 16.4 and the use of componentWillMount is causing the component not to render properly for SSR. I went in to the react-loadable module and swapped componentWillMount for componentDidMount and no more warnings or SSR issues.

@the0neWhoKnocks
Copy link

Actually, looks like this may have partially been addressed by this merged PR #123, although componentWillMount still remains along with the added componentDidMount.

@the0neWhoKnocks
Copy link

Any traction on this? The PR appears ready to merge.

@dbenchi
Copy link

dbenchi commented Sep 10, 2018

Any news about this PR?

@mshwery
Copy link

mshwery commented Sep 20, 2018

LGTM! Anything I can do to help move this along?

@jaybe78 jaybe78 mentioned this pull request Oct 22, 2018
@jaybe78
Copy link

jaybe78 commented Oct 23, 2018

@abenchi @mshwery
This includes your PR + migration to babel .7 and webpack 5
https://www.npmjs.com/package/jaybe-react-loadable
Cheers

@jedwards1211
Copy link

jedwards1211 commented Nov 15, 2018

@jamiebuilds can we please get this merged? Do you need a new maintainer?

@courtyenn
Copy link

courtyenn commented Dec 17, 2018

Would love to see this make some progress.

@jaybe78
Copy link

jaybe78 commented Dec 18, 2018

@abenchi yes you can lazy load with latest version of react, though suspense is not supported in SSR mode... So for people like me who server side render their app, there's no option beside react universal or react loadable

Repository owner deleted a comment from SimenB Feb 28, 2020
Repository owner deleted a comment from dbenchi Feb 28, 2020
@owen2345
Copy link

owen2345 commented Aug 5, 2020

Hey @jamiebuilds @tajo
any news to fix the concerning issue of this PR?

@slorber
Copy link

slorber commented Feb 12, 2021

For anyone interested, here's my solution to get rid of the warning (React 16 + 17) and still get this working (client+SSR).

#213 (comment)

Repository owner deleted a comment from ygs-code Sep 6, 2022
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.