React team has removed the warning on setting `setState` on an unmounted component about advanced-react-apis

[kentcdodds]

Thanks @payapula! I'll be removing this from the workshop in the future :)

[Aprillion]

IMHO the are actual (but insignificantly small) memory leaks were/are real, but the bigger problem was that 99.9% of workarounds to get rid of the warning (including the mounted ref in this workshop) could not do anything about any memory leaks, it only silenced the warning itself and made the code more complex...

the pattern can be still useful to have in our tool belts, but no idea what is the best scenario to explain it if dispatch / setState after unmount won't have to be guarded any more ¯\_(ツ)_/¯

if (isMounted) {
  doSomethingThatMakesSenseOnlyIfMounted()
}

[Aprillion]

...maybe starting some imperative animation for "payment confirmed" after 200 response from a POST request, that would only make sense if still mounted

and perhaps a counter-example to remove some imperative DOM event listeners - those should always be in the cleanup of a useEffect without any guarding, event listeners have to be removed unconditionally, especially if React is unmounting / already unmounted 😅

[aimanzaheb]

In your workshop material you mentioned "Also notice that while what we're doing here is still useful and you'll learn valuable skills"
function useSafeDispatch(dispatch) {
const mountedRef = React.useRef(false)

React.useEffect(() => {
mountedRef.current = true
return () => {
mountedRef.current = false
}
}, [])

return React.useCallback(
(...args) => (mountedRef.current ? dispatch(...args) : void 0),
[dispatch],
)
}

But As mentioned on the link reactwg/react-18#82
"The workaround is worse than the original problem
The workaround above is very common. But not only is it unnecessary, it also makes things a bit worse:

In the future, we'd like to offer a feature that lets you preserve DOM and state even when the component is not visible, but disconnect its effects. With this feature, the code above doesn't work well. You'll "miss" the setPending(false) call because isMountedRef.current is false while the component is in a hidden tab. So once you return to the hidden tab and make it visible again, it will look as if your mutation hasn't "come through". By trying to evade the warning, we've made the behavior worse.

In addition, we've seen that this warning pushes people towards moving mutations (like POST) into effects just because effects offer an easy way to "ignore" something using the cleanup function and a flag. However, this also usually makes the code worse. The user action is associated with a particular event — not with the component being displayed — and so moving this code into effect introduces extra fragility. Such as the risk of the effect re-firing when some related data changes. So the warning against a problematic pattern ends up pushing people towards more problematic patterns though the original code was actually fine."

Please tell what should we do?

[kentcdodds]

Don't worry about it unless it becomes a problem in your app (it probably won't).