Svelte: using get or subscribing and unsubscribing on queryStore response breaks the store

[hahn-kev]

Describe the bug

When using the Svelte query store (possibly other's I'm not using them yet), if you call get on the returned store immediately after calling queryStore then the store will be broken and no longer propagate results or cache updates. This issue also exhibits itself if you call subscribe and later on unsubscribe before any other subscriptions are made, then the value in the store will be frozen at the time you unsubscribe and no cache updates or results will be propagated.

I encountered this issue when I attempt to await the query store with a promise, Something like this:

await new Promise<OperationResultState<Data, Variables>>((resolve) => {
  let invalidate = undefined;
  invalidate = resultStore.subscribe(value => {
    if (value.fetching) return;
    if (invalidate) invalidate();
    resolve(value);
  });
});

I just want to await until the store has finished fetching and then clean up my subscription. But I still want to pass the resultStore to the caller so they can get cache updates later on.

My reproduction on stackblitz is much simpler, I just calling the queryStore function, call get on the store to break it. Then subscribe and assign the value to a variable for use in Svelte. This should work just fine and if you comment out the get then it does work exactly as expected. However with the get call the store breaks and never receives the result from the server.

Taking a look at the source it looks like the problem is here, we unsubscribe when the store is unsubscribed from by the last observer, however if another observer comes after that we don't subscribe again and the store stays broken. I believe the correct fix would be to execute the pipe call on line 146 inside the writable callback on line 142, though care would need to be taken so that executeRequestOperation is not called more than once.

As a workaround for now I'm going to use my first subscription to propagate values to another store that I setup.

Reproduction

https://stackblitz.com/edit/github-pznkt1?file=src%2FPokemonList.svelte

Urql version

urql: 4.10 urql/svelte: 4.0.3 svelte: 4.0.5

Validations

• [X] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• [X] Read the docs.
• [X] Follow our Code of Conduct

[kitten]

Nice catch! 👏

Yes, after the proposed refactor that we got for @urql/svelte@^4, I didn't notice that the subscription was only instantiated once, even after a refactor.

The reason why this probably didn't occur to me — and why you can also work around this rather easily — is that the Client already manages the stream subscriptions quite strictly and well. For instance, other bindings may start simultaneous subscriptions. These will not internally start multiple operations, but instead, query/subscription operations are considered atomic and identical when their keys match. This means that you can start a subscription multiple times and they'll be deduplicated and the updates will be distributed to all of them, to simplify what's going on.

So, you could also create multiple operationStores and it'll still work.

This basically should also make the fix quite simple. Since svelte/store already has semaphore logic inside writable and doesn't start the source subscription until there's a reader, and doesn't stop it until there's no more readers, we can simply subscribe inside the store's StartStopNotifier itself.

On an unrelated note, you can actually see that @urql/vue does something similar to what you're trying to do: https://github.com/urql-graphql/urql/blob/45d686ec947ca0055c73e443cdcabb2d8d3102dc/packages/vue-urql/src/useQuery.ts#L393-L409

I could actually imagine that we could integrate this natively into @urql/svelte as well, but I'd have to see more sample code to decide on this. I think we had a prior implementation for this that may have disappeared in v4 as well, but I'm not sure anymore.

The double subscription is also quite common and must be done in React to determine initial state, ergo, using get should obviously be a supported pattern in Svelte as well