onBecomeUnobserved not triggered immediately on useObserver component unmount

[Wroud]

Intended outcome:
onBecomeUnobserved triggered when the component wrapped with observer is unmounted

Actual outcome:
onBecomeUnobserved never triggered or triggered randomly

How to reproduce the issue:

https://codesandbox.io/s/practical-ioana-6sthfs?file=/src/App.tsx

Versions

seems like it's reproduced on any version

[Wroud]

It only reproduces with React.StrictMode

[Wroud]

same issue with react suspense: https://codesandbox.io/s/goofy-wright-kmpjxd?file=/src/App.tsx

[Wroud]

Ideas:

1. on render, use reaction to collect used observables and interrupt render if some changes
2. destroy the first reaction after executing the render function but save a list of used observables
3. use useEffect to re-create reaction after the component is mounted with the saved list of used observables

This approach tries to reach two goals:

1. save react behavior with render interruption on state change when rendering component
2. clears reaction and its dependencies to support React.StrictMode and React Suspense (you don't need to track 'lost' reactions in this case also)

Why to use useEffect to recreate reaction:

1. server rendering support (useEffect won't be called in SSR)
2. executed after component mounted (Suspense)

[urugator]

It takes a while, but it eventually unobserves (at least for me):

[Wroud]

Does it work for both examples?

[Wroud]

Yes, it's released, but the delay is long; for example, if I want to track used resources to load them asynchronously, it will cause loading data even when a component is already unmounted.

[urugator]

Yes, the suspense example actually unobserved immediately upon clicking the btn on the first try. It depends on GC, so it's a bit random, which I agree isn't ideal, but I wouldn't expect changes here anytime soon. Ideally use useEffect if precise timing is needed. Would be good to mention this in docs though (PR welcome).

use useEffect to re-create reaction after the component is mounted with the saved list of used observables

You also have to diff saved and current values as your state could be changed before mount. We were considering these before. I am a bit afraid that the machinery involved could often end up being slower or comparable to just scheduling another render on mount...

[Wroud]

I see. Why are you using setTimeout with a custom timeout to clear lost reactions?
Is it beneficial to use requestAnimationFrame and cancelAnimationFrame instead?

Theoretically, when the component hasn't been rendered on the following browser's repaint, it's optimized by React and it will be rendered in the following frames, or the render has been interrupted by StrictMode or Suspense.

[Wroud]

I created a pull request that includes the use of requestAnimationFrame and a new additional test case.
I also tested it on my project where I found this issue initially

[joshuakb2]

I just encountered this bug too. I made a reproduction, not sure if it helps but I'll link it just in case.
https://codesandbox.io/s/mobx-leak-repro-8wph2n?file=/src/App.tsx