Unsubscription doesn't happen in certain occasions

[vicsnow]

Moving on from #881 It's a small slice from my current project, where I have a subscription query as a mutable variable, and urql does not unsubscribe in couple of instances, although react-apollo does. urql version & exchanges: "urql": "^1.9.8" Steps to reproduce Here is an issue repo https://github.com/vicsnow/urqlsubscriptionissue

Expected behavior To unsubscribe from every unused subscription. Actual behavior Does not unsubscribe in specific occasions (watch README.md in my repo)

[JoviDeCroock]

Hey,

I've been looking in to this issue and it looks like the correct behavior, let's look at the why

Assume we have a similar scenario to your reproduction, on the left side we have a hypothetical component and on the right the variables used within the subscription of the component.

A --> 1
B --> 2
C --> 3

When we change component A to use variable 2 instead of 1 we'll see the 1 subscription unmount correctly because no component is using said subscription.

If we return to the original scenario and instead of changing A to 2 we'll shift all of them through 1 position so A uses 3 B uses 1 and C uses 2. This means that instead of unsubscribing and resubscribing (http-traffic) we can just pass this subscription on to the next component. This results in less network traffic and seems like the correct behavior.

I've seen one use case where you'd want this unsub/resub mechanism to get initial data for your component but it feels more correct to first dispatch a query and react to the responses of your subscription instead.

Feel free to discuss this more/ask more questions

[vicsnow]

@JoviDeCroock, I understand, that similar subs are not unmounting if you've got a duplication, but it seems that this mechanism isn't working correctly somehow. It seems, like if you have

A -> 1 
B -> 1 
C -> 3

And you make a switch so it becomes

A -> 3
B -> 1 
C -> 1

The expected behaviour, as far as I understand, is not to unsub from 1 and 3, but just pass them to a different component instead, so the total number of subs remains the same. But look what actually happens here, I made a very short video of my repo's bug reproduction https://youtu.be/3yjHtEP3xgY

[JoviDeCroock]

This seems like a timing related thing, which would live in React scheduling, the client used and websockets in general. This doesn't seem like a problem that causes bugs, is there something you are relying on for logging, metrics,... In your backend that causes issues with this?

[vicsnow]

@JoviDeCroock , it was my thought at first, but I checked this very backend using react-apollo, and it worked just fine

[kitten]

Hiya, Sorry, I believe we never got to fully write out a response here. The video you've sent is somewhat useful (but too fast even slowed down to really see what you're pointing out), however we took a look at your example.

The first problem is that you're observing changes on the server-side and we don't control how the subscription client handles subscriptions and unsubscriptions. This can be altered, you can add debouncing, etc to this and it'll change the entire outcome. So what we care about is how it behaves in urql.

As far as we can tell this is behaving correctly, as per the second problem, unmount timing. When you unmount components and remount them then that means that the subscription becomes inactive. Your component (for instance in your ABC, 113 to 311 example) completely switches its subscriptions, which is an unrealistic scenario.

But what happens when you switch them? Well, React dictates that on updates, unsubscriptions/teardowns from effect will complete before new effects start or mount after an update. This means that in all three cases your subscriptions have been ended before the new one has started. This is expected behaviour, although timing will be sensitive in React Concurrent Mode.

What you're looking for is: subscribe before unsubscribing, which isn't a mechanism that is particularly natural to React or urql, the reason for that being that teardowns and cleaning up after operations is cheap.

Even if we take the subscription client into account, it keeps a stable connection, so for it unsubscribing and resubscribing is cheap too. If that's not desired then again a debounce should be added.

Ultimately we don't see this as an issue because this is behaviour we expect. What we assume is also that this difference isn't only cheap but it also shouldn't make a difference in subscriptions as subscriptions shouldn't do anything special when they're started, since that's behaviour that a query would perform.

Hope that makes sense :wave: Sorry again for the late response!