markusenglund / react-switch

A draggable toggle-switch component for React. Check out the demo at:
https://react-switch.netlify.com/
MIT License
1.33k stars 100 forks source link

Replace deprecated lifecycle hook #41

Closed OriginalEXE closed 5 years ago

OriginalEXE commented 5 years ago

Hello,

First of all, thank you for maintaining this awesome library, it's simple but very useful.

I have wrapped my in-development app in a strict mode and got the following warning:

Warning: Unsafe lifecycle methods were found within a strict-mode tree:
    in StrictMode (at _app.js:37)
    in Container (at _app.js:36)
    in MyApp

componentWillReceiveProps: Please update the following components to use static getDerivedStateFromProps instead: ReactSwitch

Learn more about this warning here:
https://fb.me/react-strict-mode-warnings

This pull request replaces the deprecated lifecycle method. All tests pass, however, we get a linter warning which we might want to disable, as per discussion here: https://github.com/airbnb/javascript/issues/1875

Looking forward to your thoughts, I would love to get this fixed to be able to use the strict mode and in the future all the React performance goodies.

Thanks

markusenglund commented 5 years ago

Yup, looks like it's probably time to remove componentWillReceiveProps. Putting the logic in componentDidUpdate makes sense I think. It causes one extra render each time it's called, but I'm not overly concerned since we're already causing quite a few renders. getDerivedStateFromProps seems like a worse option anyway since it's so new and would require us to put the previous prop in state.

I'm approving this. I just have to make absolutely sure that it doesn't break anything before releasing.

markusenglund commented 5 years ago

Released in 5.0.0! Sorry for the delay.

OriginalEXE commented 5 years ago

No worries, thank you for your work and for this new release!