Signals never unwatched when using hooks and suspenses

[CharlieCrisp]

• Check if updating to the latest version resolves the issue

Environment

• I am using @preact/signals-core
• I am using @preact/signals
• I am using @preact/signals-react
  • I understand usage changed in v2, and I've followed the React Integration instructions

Describe the bug
When using signals in a React project alongside suspenses, signals are never unwatched.

Rather than a wordy description, I've written some unit tests.

To Reproduce

Add the unit tests from this gist to suspense.test.tsx

https://gist.github.com/CharlieCrisp/222d10fd147596567a73ae546ff441f1

The following fail:

• "should clean up signals after unmount with multiple suspense boundaries"
• "should clean up signals after unmount with multiple suspense boundaries and use of try catch"

Expected behavior
Tests should pass, signals should be unwatched when components unmount

[JoviDeCroock]

I'd assume that's because they aren't unwatched, React keeps the virtual-dom tree in-memory afaik so the Dispose() never gets called. If we wanted to forcefully unsubscribe all signals we'd have to hook into useEffect and its cleanup function as those cleanups are forcefully called compared to the useSyncExternalStore approach and it going out of memory like we do now.

[CharlieCrisp]

@JoviDeCroock I might be wrong but I don't believe that's quite the nature of this issue. From my debugging I found that multiple EffectStores were being created for a component. It derived from this line. The useRef is not stable between renders if something suspends during a render.

What was happening is this:

• The component would render, storeRef is null, so it creates a new EffectStore.
• This EffectStore creates an effect with a subscription to the signals used in the render.
• Something else in the tree suspends and the whole thing is never mounted.
• useSyncExternalStore never calls EffectStore.subscribe and therefore EffectStore.unsubscribe is also never called.
• The suspense eventually resolves and the component renders again.
• During this render storeRef is null again, and the component creates another new EffectStore.
• The original EffectStore is left hanging. Nothing ever called it's unwatched callback.

For the project I'm working on, it's really important that we unsubscribe from resources when components are unmounted so this behaviour is significant.

I definitely don't know enough to weigh in on solutions here but if I could just make a couple of observations:

1. Any solution using useEffect isn't going to work. The suspense means the component is rendered but never actually mounted and any useEffect it registers is not going to run.
2. Really the problem comes down to the fact that the subscription is effectively registered during the render rather than when the subscribe function is called. To do this idiomatically, I think one would just track what signals are being used by the component until the first call to subscribe, rather than actually creating the effect that subscribes to them. If a subscription was actually only created during subscribe then you could guarantee that subscription will always be cleaned up correctly by useSyncExternalStore.

[JoviDeCroock]

The ref being forgotten is part of this issue yes, it's also inconsistent because react has a missing lifecycle. When a component suspends during initial mount it is destroyed while during an update it's not, this means that we are dealing with an inconsistency. Libraries like urql, react query,... will create a global cache for this where they store their promises so they can bypass the destroy vs update inconsistency, this could also be a solution for us. I don't see any other good solution as starting the store during subscribe is effectively too late to observe all accessors. In React 18+ we could leverage useId() to create this mapping, however useId has been inconsistent in my testing as well, my other fear is clearing used memory. We could clear memory when a component is unmounted by detecting that through useEffect but I reckon you see the conundrum in the suspense ... flaw (?)

Another solution could be, as you say, to only effectively call the functions during subscribe

[CharlieCrisp]

Yeah the behaviour of React definitely makes things tricky. To me it does feel like doing the subscription logic within subscribe is the only solution React has made available that isn't super hacky. Like you say, it does mean that tracking accessors has to be a separate thing to 'starting' the effect's subscription. It doesn't look like that's how the effect internals are structured at the moment but conceptually it feels reasonable.

The other solution is more of an API change but this whole problem is a lot simpler if you access each signal value through its own hook. Then you don't need an effect/accessor tracking at all. However I'm guessing you want to avoid an API change like that at all costs...