Skip to content

Commit

Permalink
fix: Prevent invalidating subscription iterator (#1438)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
cngu authored and ktsn committed Dec 25, 2019
1 parent 8fd61c9 commit e012653
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ export class Store {
handler(payload)
})
})
this._subscribers.forEach(sub => sub(mutation, this.state))

this._subscribers
.slice() // shallow copy to prevent iterator invalidation if subscriber synchronously calls unsubscribe
.forEach(sub => sub(mutation, this.state))

if (
process.env.NODE_ENV !== 'production' &&
Expand Down Expand Up @@ -132,6 +135,7 @@ export class Store {

try {
this._actionSubscribers
.slice() // shallow copy to prevent iterator invalidation if subscriber synchronously calls unsubscribe
.filter(sub => sub.before)
.forEach(sub => sub.before(action, this.state))
} catch (e) {
Expand Down
42 changes: 42 additions & 0 deletions test/unit/store.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,48 @@ describe('Store', () => {
expect(secondSubscribeSpy.calls.count()).toBe(2)
})

it('subscribe: should handle subscriptions with synchronous unsubscriptions', () => {
const subscribeSpy = jasmine.createSpy()
const testPayload = 2
const store = new Vuex.Store({
state: {},
mutations: {
[TEST]: () => {}
}
})

const unsubscribe = store.subscribe(() => unsubscribe())
store.subscribe(subscribeSpy)
store.commit(TEST, testPayload)

expect(subscribeSpy).toHaveBeenCalledWith(
{ type: TEST, payload: testPayload },
store.state
)
expect(subscribeSpy.calls.count()).toBe(1)
})

it('subscribeAction: should handle subscriptions with synchronous unsubscriptions', () => {
const subscribeSpy = jasmine.createSpy()
const testPayload = 2
const store = new Vuex.Store({
state: {},
actions: {
[TEST]: () => {}
}
})

const unsubscribe = store.subscribeAction(() => unsubscribe())
store.subscribeAction(subscribeSpy)
store.dispatch(TEST, testPayload)

expect(subscribeSpy).toHaveBeenCalledWith(
{ type: TEST, payload: testPayload },
store.state
)
expect(subscribeSpy.calls.count()).toBe(1)
})

// store.watch should only be asserted in non-SSR environment
if (!isSSR) {
it('strict mode: warn mutations outside of handlers', () => {
Expand Down

0 comments on commit e012653

Please sign in to comment.