Prevent invalidating subscription iterator #1438
Conversation
…ously calling unsubscribe
src/store.js
Outdated
| @@ -100,7 +100,7 @@ export class Store { | |||
| handler(payload) | |||
| }) | |||
| }) | |||
| this._subscribers.forEach(sub => sub(mutation, this.state)) | |||
| this._subscribers.slice().forEach(sub => sub(mutation, this.state)) | |||
There was a problem hiding this comment.
I'm open to cleaner/faster ways to generate a shallow copy.
There was a problem hiding this comment.
I guess it's ok to use slice for that.
|
What is the actual use case that unsubscribing in another subscribe handler by the way? |
|
It can be useful in scenarios where you want to teardown a subscription when a particular mutation occurs (e.g. logout), or mutations that are guaranteed to only be committed N times during the lifetime of the app (e.g. 1 login from OAuth flow, as another login would have to reload the page). |
src/store.js
Outdated
| @@ -100,7 +100,7 @@ export class Store { | |||
| handler(payload) | |||
| }) | |||
| }) | |||
| this._subscribers.forEach(sub => sub(mutation, this.state)) | |||
| this._subscribers.slice().forEach(sub => sub(mutation, this.state)) | |||
There was a problem hiding this comment.
I guess it's ok to use slice for that.
| @@ -100,7 +100,7 @@ export class Store { | |||
| handler(payload) | |||
| }) | |||
| }) | |||
| this._subscribers.forEach(sub => sub(mutation, this.state)) | |||
There was a problem hiding this comment.
Could you write some comment to share the context?
| @@ -100,7 +100,7 @@ export class Store { | |||
| handler(payload) | |||
| }) | |||
| }) | |||
| this._subscribers.forEach(sub => sub(mutation, this.state)) | |||
There was a problem hiding this comment.
I think we also need to do the same thing for _actionSubscribers in dispatch method.
https://github.com/vuejs/vuex/blob/dev/src/store.js#L132
There was a problem hiding this comment.
Good catch, I've shared it through a new notifySubscribers function.
|
Hi @ktsn, are there any other changes you'd like to see in this PR? |
|
Hey @cngu, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
* Prevent users from invalidating the subscription iterator by synchronously calling unsubscribe * Prevent users from invalidating the action subscription iterator by synchronously calling unsubscribe * Fix commit subscribers argument invocation
This change shallow copies
_subscribersbefore iterating over it, in-case one of the subscription listeners attempts to unsubscribe synchronously and invalidate the iterator.