Skip to content

Observer still called when unsubscribed during store update #4794

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

Open
pushkine opened this issue May 7, 2020 · 4 comments
Open

Observer still called when unsubscribed during store update #4794

pushkine opened this issue May 7, 2020 · 4 comments
Labels
awaiting submitter needs a reproduction, or clarification stale-bot

Comments

@pushkine
Copy link
Contributor

pushkine commented May 7, 2020

https://svelte.dev/repl/703dd3e1045543388fd5583c489bac55?version=3.22.2

const store = writable(0);
let is2cancelled = false;
const cancel_sub1 = store.subscribe((v) => {
	if(v === 1){
		cancel_sub2()
		is2cancelled = true
	}
});
const cancel_sub2 = store.subscribe((v) => {
	if(v === 1 && is2cancelled){
		console.log('I have been cancelled already');
	}
});
store.set(1); // logs 'I have been cancelled already'
cancel_sub1();

will fix with #4742 unless someone thinks this behavior is expected ?

@dimfeld
Copy link
Contributor

dimfeld commented May 7, 2020

One concern with changing this is that it potentially relies on the observers being called in the same order in which they were subscribed. For example, if in the future stores are changed so that the callbacks are called in reverse order of subscription, the behavior of the example above would change so that both callbacks are called once again.

With that in mind, I think I would feel most comfortable with this change if Svelte made one of these behaviors official:

  1. Store subscriptions will always be called in the same order that they subscribed.

or

  1. There is explicitly no guarantee about the order in which subscribers are called. Kind of the same thing that Go says about iterating over maps.

What do you think?

@pushkine
Copy link
Contributor Author

pushkine commented May 7, 2020

Observables are just arrays of callbacks so as per ECMA the order is guaranteed, I don't see how that could change in the future

@dimfeld
Copy link
Contributor

dimfeld commented May 7, 2020

The most obvious possibility in my mind is that at some point in the future there is a need for better performance of unsubscribing, and so the array of subscribers is replaced with some other data structure. It would probably be a Map though, and so iteration would still be in insertion order. I suppose some other change could necessitate iterating in reverse order, but I cant think of any real reason to do that.

But mostly I'm just thinking about the formal promises made by the Svelte API vs. the logical, but undocumented behavior that it exhibits, such as iterating over subscribers in the order they were added. If the answer is that I'm just worrying too much, that's fine and I'm happy to drop the issue :)

@Conduitry Conduitry added the awaiting submitter needs a reproduction, or clarification label Jun 8, 2020
@stale
Copy link

stale bot commented Dec 24, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification stale-bot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants