From: Eduardo San Martin Morote Date: Mon, 20 Dec 2021 19:08:19 +0000 (+0100) Subject: fix(subscribe): avoid $subscriptions with $patch X-Git-Tag: pinia@2.0.8~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3bfe9e51fd0f9b3f939aace53463aa10f2e9c90f;p=thirdparty%2Fvuejs%2Fpinia.git fix(subscribe): avoid $subscriptions with $patch Fix #908 --- diff --git a/packages/docs/core-concepts/state.md b/packages/docs/core-concepts/state.md index 006ed151..93c0ce8f 100644 --- a/packages/docs/core-concepts/state.md +++ b/packages/docs/core-concepts/state.md @@ -151,7 +151,7 @@ cartStore.$subscribe((mutation, state) => { }) ``` -By default, _state subscriptions_ are bound to the component where they are added (if the store is inside a component's `setup()`). Meaning, they will be automatically removed when the component is unmounted. If you want to keep them after the component is unmounted, pass `true` as the second argument to _detach_ the _state subscription_ from the current component: +By default, _state subscriptions_ are bound to the component where they are added (if the store is inside a component's `setup()`). Meaning, they will be automatically removed when the component is unmounted. If you want to keep them after the component is unmounted, pass `{ detached: true }` as the second argument to _detach_ the _state subscription_ from the current component: ```js export default { @@ -159,7 +159,7 @@ export default { const someStore = useSomeStore() // this subscription will be kept after the component is unmounted - someStore.$subscribe(callback, true) + someStore.$subscribe(callback, { detached: true }) // ... }, diff --git a/packages/pinia/__tests__/subscriptions.spec.ts b/packages/pinia/__tests__/subscriptions.spec.ts index 4fb53935..bfd36b39 100644 --- a/packages/pinia/__tests__/subscriptions.spec.ts +++ b/packages/pinia/__tests__/subscriptions.spec.ts @@ -37,6 +37,7 @@ describe('Subscriptions', () => { const patch = { user: 'Cleiton' } store.$patch(patch) + expect(spy).toHaveBeenCalledTimes(1) expect(spy).toHaveBeenCalledWith( expect.objectContaining({ payload: patch, @@ -46,6 +47,116 @@ describe('Subscriptions', () => { store.$state ) }) + const flushOptions = ['post', 'pre', 'sync'] as const + + flushOptions.forEach((flush) => { + it('calls once inside components with flush ' + flush, async () => { + const pinia = createPinia() + setActivePinia(pinia) + const spy1 = jest.fn() + + mount( + { + setup() { + const s1 = useStore() + s1.$subscribe(spy1, { flush }) + }, + template: `

`, + }, + { global: { plugins: [pinia] } } + ) + + const s1 = useStore() + + expect(spy1).toHaveBeenCalledTimes(0) + + s1.user = 'Edu' + await nextTick() + await nextTick() + expect(spy1).toHaveBeenCalledTimes(1) + + s1.$patch({ user: 'a' }) + await nextTick() + await nextTick() + expect(spy1).toHaveBeenCalledTimes(2) + + s1.$patch((state) => { + state.user = 'other' + }) + await nextTick() + await nextTick() + expect(spy1).toHaveBeenCalledTimes(3) + }) + }) + + it('works with multiple different flush', async () => { + const spyPre = jest.fn() + const spyPost = jest.fn() + const spySync = jest.fn() + + const s1 = useStore() + s1.$subscribe(spyPre, { flush: 'pre' }) + s1.$subscribe(spyPost, { flush: 'post' }) + s1.$subscribe(spySync, { flush: 'sync' }) + + expect(spyPre).toHaveBeenCalledTimes(0) + expect(spyPost).toHaveBeenCalledTimes(0) + expect(spySync).toHaveBeenCalledTimes(0) + + s1.user = 'Edu' + expect(spyPre).toHaveBeenCalledTimes(0) + expect(spyPost).toHaveBeenCalledTimes(0) + expect(spySync).toHaveBeenCalledTimes(1) + await nextTick() + expect(spyPre).toHaveBeenCalledTimes(1) + expect(spyPost).toHaveBeenCalledTimes(1) + expect(spySync).toHaveBeenCalledTimes(1) + + s1.$patch({ user: 'a' }) + // patch still triggers all subscriptions immediately + expect(spyPre).toHaveBeenCalledTimes(2) + expect(spyPost).toHaveBeenCalledTimes(2) + expect(spySync).toHaveBeenCalledTimes(2) + await nextTick() + expect(spyPre).toHaveBeenCalledTimes(2) + expect(spyPost).toHaveBeenCalledTimes(2) + expect(spySync).toHaveBeenCalledTimes(2) + + s1.$patch((state) => { + state.user = 'other' + }) + expect(spyPre).toHaveBeenCalledTimes(3) + expect(spyPost).toHaveBeenCalledTimes(3) + expect(spySync).toHaveBeenCalledTimes(3) + await nextTick() + expect(spyPre).toHaveBeenCalledTimes(3) + expect(spyPost).toHaveBeenCalledTimes(3) + expect(spySync).toHaveBeenCalledTimes(3) + }) + + it('works with multiple different flush and multiple state changes', async () => { + const spyPre = jest.fn() + const spyPost = jest.fn() + const spySync = jest.fn() + + const s1 = useStore() + s1.$subscribe(spyPre, { flush: 'pre' }) + s1.$subscribe(spyPost, { flush: 'post' }) + s1.$subscribe(spySync, { flush: 'sync' }) + + s1.user = 'Edu' + expect(spyPre).toHaveBeenCalledTimes(0) + expect(spyPost).toHaveBeenCalledTimes(0) + expect(spySync).toHaveBeenCalledTimes(1) + s1.$patch({ user: 'a' }) + expect(spyPre).toHaveBeenCalledTimes(1) + expect(spyPost).toHaveBeenCalledTimes(1) + expect(spySync).toHaveBeenCalledTimes(2) + await nextTick() + expect(spyPre).toHaveBeenCalledTimes(1) + expect(spyPost).toHaveBeenCalledTimes(1) + expect(spySync).toHaveBeenCalledTimes(2) + }) it('unsubscribes callback when unsubscribe is called', () => { const spy = jest.fn() diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index d38becab..33415b3d 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -20,6 +20,7 @@ import { ref, set, del, + nextTick, isVue2, } from 'vue-demi' import { @@ -229,6 +230,7 @@ function createSetupStore< // internal state let isListening: boolean // set to true at the end + let isSyncListening: boolean // set to true at the end let subscriptions: SubscriptionCallback[] = markRaw([]) let actionSubscriptions: StoreOnActionListener[] = markRaw([]) let debuggerEvents: DebuggerEvent[] | DebuggerEvent @@ -255,7 +257,7 @@ function createSetupStore< | ((state: UnwrapRef) => void) ): void { let subscriptionMutation: SubscriptionCallbackMutation - isListening = false + isListening = isSyncListening = false // reset the debugger events since patches are sync /* istanbul ignore else */ if (__DEV__) { @@ -277,7 +279,10 @@ function createSetupStore< events: debuggerEvents as DebuggerEvent[], } } - isListening = true + nextTick().then(() => { + isListening = true + }) + isSyncListening = true // because we paused the watcher, we need to manually call the subscriptions triggerSubscriptions( subscriptions, @@ -384,7 +389,7 @@ function createSetupStore< watch( () => pinia.state.value[$id] as UnwrapRef, (state) => { - if (isListening) { + if (options.flush === 'sync' ? isSyncListening : isListening) { callback( { storeId: $id, @@ -575,8 +580,12 @@ function createSetupStore< // avoid devtools logging this as a mutation isListening = false + isSyncListening = false pinia.state.value[$id] = toRef(newStore._hmrPayload, 'hotState') - isListening = true + isSyncListening = true + nextTick().then(() => { + isListening = true + }) for (const actionName in newStore._hmrPayload.actions) { const action: _Method = newStore[actionName] @@ -702,6 +711,7 @@ function createSetupStore< } isListening = true + isSyncListening = true return store } diff --git a/packages/pinia/src/types.ts b/packages/pinia/src/types.ts index d6027582..a37ab76a 100644 --- a/packages/pinia/src/types.ts +++ b/packages/pinia/src/types.ts @@ -349,15 +349,13 @@ export interface _StoreWithState< $reset(): void /** - * Setups a callback to be called whenever the state changes. It also returns - * a function to remove the callback. Note than when calling - * `store.$subscribe()` inside of a component, it will be automatically - * cleanup up when the component gets unmounted unless `detached` is set to - * true. + * Setups a callback to be called whenever the state changes. It also returns a function to remove the callback. Note + * than when calling `store.$subscribe()` inside of a component, it will be automatically cleanup up when the + * component gets unmounted unless `detached` is set to true. * * @param callback - callback passed to the watcher - * @param options - `watch` options + `detached` to detach the subscription - * from the context (usually a component) this is called from + * @param options - `watch` options + `detached` to detach the subscription from the context (usually a component) + * this is called from. Note that the `flush` option does not affect calls to `store.$patch()`. * @returns function that removes the watcher */ $subscribe(