From e10ae8fea2429d0431ab2b2239cfdd4c714e9ed1 Mon Sep 17 00:00:00 2001 From: Yang Mingshan Date: Sat, 4 Jan 2025 18:38:03 +0800 Subject: [PATCH 1/2] fix: async subscription do not run when mutate state synchronously after patch --- .../pinia/__tests__/subscriptions.spec.ts | 21 ++++++-- packages/pinia/src/store.ts | 50 +++++++++---------- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/packages/pinia/__tests__/subscriptions.spec.ts b/packages/pinia/__tests__/subscriptions.spec.ts index 782871b586..96becd0cea 100644 --- a/packages/pinia/__tests__/subscriptions.spec.ts +++ b/packages/pinia/__tests__/subscriptions.spec.ts @@ -368,12 +368,23 @@ describe('Subscriptions', () => { store.user = 'a' expect(syncSpy).toHaveBeenCalledTimes(2) - // FIXME: ideally, these should be 2 but we cannot use - // a sync flush within the store's $subscribe method - // https://github.com/vuejs/pinia/issues/610 await nextTick() - expect(preSpy).toHaveBeenCalledTimes(1) - expect(postSpy).toHaveBeenCalledTimes(1) + expect(preSpy).toHaveBeenCalledTimes(2) + expect(postSpy).toHaveBeenCalledTimes(2) + }) + + it('debuggerEvents is not an array when subscription is not triggered by patch', () => { + const store = useStore() + store.$subscribe( + ({ type, events }) => { + if (type === MutationType.direct) { + expect(Array.isArray(events)).toBe(false) + } + }, + { flush: 'sync' } + ) + store.$patch({ user: 'Edu' }) + store.user = 'a' }) }) }) diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index 7ec0e17aeb..94186b3e1f 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -19,7 +19,6 @@ import { toRefs, Ref, ref, - nextTick, } from 'vue' import { StateTree, @@ -250,7 +249,7 @@ function createSetupStore< if (isListening) { debuggerEvents = event // avoid triggering this while the store is being built and the state is being set in pinia - } else if (isListening == false && !store._hotUpdating) { + } else if (isListening === false && !store._hotUpdating) { // let patch send all the events together later /* istanbul ignore else */ if (Array.isArray(debuggerEvents)) { @@ -265,8 +264,8 @@ function createSetupStore< } // internal state - let isListening: boolean // set to true at the end - let isSyncListening: boolean // set to true at the end + let isListening = false // set to true at the end + let shouldTrigger = false // The initial value does not matter, and no need to set to true at the end let subscriptions: Set> = new Set() let actionSubscriptions: Set> = new Set() let debuggerEvents: DebuggerEvent[] | DebuggerEvent @@ -281,9 +280,6 @@ function createSetupStore< const hotState = ref({} as S) - // avoid triggering too many listeners - // https://github.com/vuejs/pinia/issues/1129 - let activeListener: Symbol | undefined function $patch(stateMutation: (state: UnwrapRef) => void): void function $patch(partialState: _DeepPartial>): void function $patch( @@ -292,7 +288,7 @@ function createSetupStore< | ((state: UnwrapRef) => void) ): void { let subscriptionMutation: SubscriptionCallbackMutation - isListening = isSyncListening = false + isListening = false // reset the debugger events since patches are sync /* istanbul ignore else */ if (__DEV__) { @@ -314,13 +310,7 @@ function createSetupStore< events: debuggerEvents as DebuggerEvent[], } } - const myListenerId = (activeListener = Symbol()) - nextTick().then(() => { - if (activeListener === myListenerId) { - isListening = true - } - }) - isSyncListening = true + isListening = true // because we paused the watcher, we need to manually call the subscriptions triggerSubscriptions( subscriptions, @@ -444,11 +434,19 @@ function createSetupStore< options.detached, () => stopWatcher() ) - const stopWatcher = scope.run(() => - watch( + const stopWatcher = scope.run(() => { + const stop1 = watch( + () => pinia.state.value[$id], + () => { + shouldTrigger = isListening + }, + { deep: true, flush: 'sync' } + ) + + const stop2 = watch( () => pinia.state.value[$id] as UnwrapRef, (state) => { - if (options.flush === 'sync' ? isSyncListening : isListening) { + if (shouldTrigger) { callback( { storeId: $id, @@ -461,7 +459,14 @@ function createSetupStore< }, assign({}, $subscribeOptions, options) ) - )! + + const stop = () => { + stop1() + stop2() + } + + return stop + })! return removeSubscription }, @@ -617,12 +622,8 @@ function createSetupStore< // avoid devtools logging this as a mutation isListening = false - isSyncListening = false pinia.state.value[$id] = toRef(newStore._hmrPayload, 'hotState') - isSyncListening = true - nextTick().then(() => { - isListening = true - }) + isListening = true for (const actionName in newStore._hmrPayload.actions) { const actionFn: _Method = newStore[actionName] @@ -751,7 +752,6 @@ function createSetupStore< } isListening = true - isSyncListening = true return store } From 807cfd645dce7fe788ad087d8f138982712e15ad Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Mon, 3 Nov 2025 11:11:46 +0100 Subject: [PATCH 2/2] refactor: smaller --- packages/pinia/src/store.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index 94186b3e1f..6cd8499e65 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -249,7 +249,7 @@ function createSetupStore< if (isListening) { debuggerEvents = event // avoid triggering this while the store is being built and the state is being set in pinia - } else if (isListening === false && !store._hotUpdating) { + } else if (!isListening && !store._hotUpdating) { // let patch send all the events together later /* istanbul ignore else */ if (Array.isArray(debuggerEvents)) { @@ -264,8 +264,8 @@ function createSetupStore< } // internal state - let isListening = false // set to true at the end - let shouldTrigger = false // The initial value does not matter, and no need to set to true at the end + let isListening: boolean | undefined // set to true at the end + let shouldTrigger: boolean | undefined // The initial value does not matter, and no need to set to true at the end let subscriptions: Set> = new Set() let actionSubscriptions: Set> = new Set() let debuggerEvents: DebuggerEvent[] | DebuggerEvent @@ -460,12 +460,10 @@ function createSetupStore< assign({}, $subscribeOptions, options) ) - const stop = () => { + return () => { stop1() stop2() } - - return stop })! return removeSubscription