react-loadable icon indicating copy to clipboard operation
react-loadable copied to clipboard

Published 20 hours ago • verified publisher icon jamiebuilds

Migrate from legacy to new React lifecycle

Open rbardini opened this issue 7 years ago • 17 comments

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.

rbardini avatar May 29 '18 08:05 rbardini

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

tajo avatar Jun 29 '18 03:06 tajo

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

rbardini avatar Jul 08 '18 22:07 rbardini

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

mr-pinzhang avatar Jul 23 '18 10:07 mr-pinzhang

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

roccoluke avatar Jul 23 '18 11:07 roccoluke

@tajo @jamiebuilds any feedback on that?

amakhrov avatar Aug 06 '18 17:08 amakhrov

@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

titouancreach avatar Aug 17 '18 14:08 titouancreach

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 avatar Aug 18 '18 06:08 the0neWhoKnocks

Actually, looks like this may have partially been addressed by this merged PR https://github.com/jamiebuilds/react-loadable/pull/123, although componentWillMount still remains along with the added componentDidMount.

the0neWhoKnocks avatar Aug 18 '18 06:08 the0neWhoKnocks

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

the0neWhoKnocks avatar Sep 01 '18 02:09 the0neWhoKnocks

Any news about this PR?

dbenchi avatar Sep 10 '18 07:09 dbenchi

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

mshwery avatar Sep 20 '18 04:09 mshwery

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

jaybe78 avatar Oct 23 '18 17:10 jaybe78

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

jedwards1211 avatar Nov 15 '18 16:11 jedwards1211

Would love to see this make some progress.

courtyenn avatar Dec 17 '18 18:12 courtyenn

@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

jaybe78 avatar Dec 18 '18 21:12 jaybe78

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

owen2345 avatar Aug 05 '20 17:08 owen2345

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

https://github.com/jamiebuilds/react-loadable/pull/213#issuecomment-778246548

slorber avatar Feb 12 '21 15:02 slorber