setState in useEffect causing a "React state update on an unmounted component" warning

[KidkArolis]

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
The warning is triggered after an intricate sequence of events. I stumbled upon it by accident, assumed it was an error in my code, but then step by step removed everything in my application until I was left with a fairly small bit of code that doesn't seem to be doing anything illegal from the API point of view, yet is triggering a warning.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem:

https://codesandbox.io/s/q87882qv64

The example is my real application code trimmed down as much as possible to demonstrate the warning. So some of the code might be a bit nonsensical/contrived, but that's because I removed lots of surrounding code leaving only the relevant bits for reproducing the issue.

In other words, there might weird looking uses of useEffect and weird sequencing of things, but that sort of falls out of how I've structured my routes, state, components in the full app, etc.

What is the expected behavior?

I would like to know if

a) is this a React bug that I stumbled upon that should be fixed?
b) is this something I'm doing "wrong" and how I could fix that in my application (i.e. is this a real memory leak being caused because of the way I structured the code)

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.8.4

[KidkArolis]

Just to add a bit about why I think this is a bug.

The recommendation from the warning is to cleanup any subscriptions inside useEffect in the effect destroy function. But in this codesandbox, you should see that useEffect destroy is only called after the setState is called. There is no way to clean that up afaik since we're not yet aware that the component is being unmounted.

[gaearon]

This code is definitely buggy btw. You’re passing [] as effect deps but close over both atom and selector. That’s not legit. I suggest you to adopt the exhaustive-deps rule and fix those bugs first.

Implementing subscriptions correctly is actually somewhat tricky. Check this for inspiration: https://gist.github.com/bvaughn/e25397f70e8c65b0ae0d7c90b731b189

Does that work better?

[KidkArolis]

I'll continue working on this codesandbox to try and minimise the code and isolate the issue better.

I am aware of some of the subscription pitfalls, this code is not what I use in production version. This is just an attempt at finding a minimal set of code that triggers this warning without using the React APIs "incorrectly" (except for the thing you pointed about about effect's deps, but I doubt that's the issue, I'll check).

It has something to do with the sync calls inside useEffect triggering calls to setStates causing components to unrender, etc. It's something along the lines of useEffect triggering a setState in parent component causing to unrender the child, but in the same tick setState getting called in the child component's useEffect's subscription, before the effect had the chance to cleanup. Not sure you're following, I can elaborate..

I guess I'm just inviting you to have a bit closer look to see that this is not bad logic in my code, but a potentially incorrect behaviour in React. I know there's 99% chance the issue is in my code, or something I'm doing that "you're not supposed to be doing", but I was hoping it's valuable to share this in case it is indeed an issue in React or a matter of missing documentation.

I know how to fix this in my application (e.g. defer my router listener callbacks), I ~know how to implement subscriptions correctly (thanks for the link, that's very useful), I'm not interested in how to fix my code, I'm just interested to help reveal a potential bug.. thus the contrived code.

[gaearon]

I doubt that's the issue, I'll check

I pointed this out because I’ve seen similar issues caused by a stale listener running after a fresh listener or something like it. So it’ll be way easier to debug after rules aren’t violated.

[KidkArolis]

Will fix that and reshare a codesandbox.

Just found some new insight (pointing to the fact that the issue is in my code). Will post back soon(ish).

[gaearon]

It's something along the lines of useEffect triggering a setState in parent component causing to unrender the child, but in the same tick setState getting called in the child component's useEffect's subscription, before the effect had the chance to cleanup. Not sure you're following, I can elaborate..

Sounds a bit like #14750 (comment).

[KidkArolis]

No, not sure it's my code.

Here's an updated, slightly smaller sandbox https://codesandbox.io/s/98lkm3731o (but with console.logs).

If this helps at all, what I can see happening is the following:

Notice how

[Onboarding] useEffect/subscription/1]
... lots of stuff happens synchronously in the middle, including running effects and destroying them
[Onboarding] useEffect/subscription/2]

The code that logs that is:

console.log(`[${name}] useEffect/subscription/1`);
setState(atom.get());
console.log(`[${name}] useEffect/subscription/2`);

Hm, not sure where I'm going with this..

[KidkArolis]

Btw, I've added all effect dependencies to the code now.

[KidkArolis]

The unexpected thing to me that these logs revealed is that my subscription callbacks are "getting incepted".. I get a subscription callback triggered while in the middle of sync handling of a subscription callback.

[KidkArolis]

Reduced the code further, removed the router bit and some effects:
https://codesandbox.io/s/zxp40j6ll4

[KidkArolis]

Oh now, it looks like an issue in my code though, since now the logs indicate subscription hasn't been cleaned up.. duh, this is so annoying.

[KidkArolis]

OK, I think your initial instict was right, Dan. It is a subscription problem.

I thought it's sufficient to clean subscriptions up by removing the subscription, but you have to use the didUnsubscribe variable like Brian did, because in this case even if the subscriber is removed from the listener array, I'm already inside a sync subscription trigger loop which is going off the old reference of the listeners and it does call my subscriber even if it was technically removed.

I can fix this upstream in tiny-atom now.

Again, thanks for your time 🙏 Apologies for raising false alarms.

[KidkArolis]

Oh noes.. In my actual app, the fix is not sufficient, there might be something else going on. I'll have to continue digging a bit.

[KidkArolis]

For what it's worth, here's a further simplified sandbox with subscription fix mentioned above: https://codesandbox.io/s/421mlo34z7 but the React warning still being triggered. I'll continue investigating..