Skip to content

Commit 99dcae6

Browse files
authored
fix: prevent duplicate watcher when same callback subscribed twice (#3144)
Close #3143
1 parent 70eacb6 commit 99dcae6

3 files changed

Lines changed: 43 additions & 0 deletions

File tree

packages/pinia/__tests__/subscriptions.spec.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ import { beforeEach, describe, it, expect, vi } from 'vitest'
22
import { createPinia, defineStore, MutationType, setActivePinia } from '../src'
33
import { mount } from '@vue/test-utils'
44
import { nextTick, ref } from 'vue'
5+
import { mockWarn } from './vitest-mock-warn'
56

67
describe('Subscriptions', () => {
8+
mockWarn()
9+
710
const useOptionsStore = defineStore('main', {
811
state: () => ({
912
user: 'Eduardo',
@@ -375,5 +378,30 @@ describe('Subscriptions', () => {
375378
expect(preSpy).toHaveBeenCalledTimes(1)
376379
expect(postSpy).toHaveBeenCalledTimes(1)
377380
})
381+
382+
it('ignores a callback subscribed twice and warns', () => {
383+
const spy = vi.fn()
384+
const store = useStore()
385+
const unsub1 = store.$subscribe(spy, { flush: 'sync' })
386+
const unsub2 = store.$subscribe(spy, { flush: 'sync' })
387+
388+
expect('passed to "$subscribe()"').toHaveBeenWarnedTimes(1)
389+
390+
store.$state.user = 'once'
391+
expect(spy).toHaveBeenCalledTimes(1)
392+
393+
store.$patch({ user: 'twice' })
394+
expect(spy).toHaveBeenCalledTimes(2)
395+
396+
// the second call returned a noop, so it does not unsubscribe anything
397+
unsub2()
398+
store.$state.user = 'after-unsub2'
399+
expect(spy).toHaveBeenCalledTimes(3)
400+
401+
// unsubscribing the original removes the single subscription
402+
unsub1()
403+
store.$state.user = 'after-unsub1'
404+
expect(spy).toHaveBeenCalledTimes(3)
405+
})
378406
})
379407
})

packages/pinia/src/diagnostics.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,5 +41,11 @@ export const diagnostics = /*#__PURE__*/ defineDiagnostics({
4141
fix: 'If it should be reactive state, wrap it with ref(), reactive(), or shallowRef(). If it is an intentional non-reactive property, wrap it with markRaw() so storeToRefs() skips it explicitly.',
4242
docs: 'https://pinia.vuejs.org/core-concepts/plugins.html#Adding-new-external-properties',
4343
},
44+
PINIA_R1007: {
45+
why: (p: { id: string }) =>
46+
`The same callback was passed to "$subscribe()" of store "${p.id}" more than once. Subscriptions are deduplicated, so the duplicate is ignored.`,
47+
fix: 'Subscribe each callback only once. If you need to resubscribe, call the returned function to remove the previous subscription first, or create a new function.',
48+
docs: 'https://pinia.vuejs.org/core-concepts/state.html#Subscribing-to-the-state',
49+
},
4450
},
4551
})

packages/pinia/src/store.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,15 @@ function createSetupStore<
436436
$patch,
437437
$reset,
438438
$subscribe(callback, options = {}) {
439+
// avoid setting up multiple watchers for the same callback
440+
// https://github.com/vuejs/pinia/issues/3143
441+
if (subscriptions.has(callback)) {
442+
if (__DEV__) {
443+
diagnostics.PINIA_R1007({ id: $id })
444+
}
445+
return noop
446+
}
447+
439448
const removeSubscription = addSubscription(
440449
subscriptions,
441450
callback,

0 commit comments

Comments
 (0)