Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(svelte): Move Client subscription to writable start #3331

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

kitten
Copy link
Member

@kitten kitten commented Jul 25, 2023

Resolves #3329

Summary

This moves the source subscription to the result writable in queryStore and subscriptionStore. We must subscribe when the StartStopNotifier is called, so to start the subscription whenever there isn't an active one to the Client.

If we don't start the subscription on demand, then unsubscribing (with all observers) from the stores will let them “die”. They won’t have an active subscription and won’t be able to start a new one.

The writable in Svelte already checks when to call StartStopNotifier via semaphore logic, so no extra code is needed.

The only risk with this fix is whether people relied on queryStore starting a subscription implicitly. This really shouldn't have been done before as it's pointless to instantiate a store without reading from it, so I don't consider it a breaking change — unless there's a specific reproduction that is proven to break from this change.

Set of changes

  • Move Client subscription into writable instantiation

@kitten kitten merged commit a0741aa into main Jul 25, 2023
6 checks passed
@kitten kitten deleted the fix/svelte-unsubscribe-once branch July 25, 2023 10:14
@github-actions github-actions bot mentioned this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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