fix(subscribe): direct mutation should not trigger detached subscriptions

Fix #908

diff --git a/packages/pinia/__tests__/subscriptions.spec.ts b/packages/pinia/__tests__/subscriptions.spec.ts
index 82fb25a6d53100ee55ea567b0a321a53df843e7c..25db3a57ade44ba9cb1fe7b1bcfbbbc364157428 100644 (file)
--- a/packages/pinia/__tests__/subscriptions.spec.ts
+++ b/packages/pinia/__tests__/subscriptions.spec.ts
@@ -124,7 +124,6 @@ describe('Subscriptions', () => {
       expect(spy2).toHaveBeenCalledTimes(0)
 
       s1.name = 'Edu'
-
       expect(spy1).toHaveBeenCalledTimes(1)
       expect(spy2).toHaveBeenCalledTimes(1)
 
@@ -132,12 +131,26 @@ describe('Subscriptions', () => {
       expect(spy1).toHaveBeenCalledTimes(2)
       expect(spy2).toHaveBeenCalledTimes(2)
 
+      s1.$patch((state) => {
+        state.name = 'other'
+      })
+      expect(spy1).toHaveBeenCalledTimes(3)
+      expect(spy2).toHaveBeenCalledTimes(3)
+
       wrapper.unmount()
       await nextTick()
 
       s1.$patch({ name: 'b' })
-      expect(spy1).toHaveBeenCalledTimes(2)
-      expect(spy2).toHaveBeenCalledTimes(3)
+      expect(spy1).toHaveBeenCalledTimes(3)
+      expect(spy2).toHaveBeenCalledTimes(4)
+      s1.$patch((state) => {
+        state.name = 'c'
+      })
+      expect(spy1).toHaveBeenCalledTimes(3)
+      expect(spy2).toHaveBeenCalledTimes(5)
+      s1.name = 'd'
+      expect(spy1).toHaveBeenCalledTimes(3)
+      expect(spy2).toHaveBeenCalledTimes(6)
     })
   })
 

diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts
index c5697e065ab85f8782492112316e21c5f0336c59..d38becabc003126e0842185d3dc318328fefe248 100644 (file)
--- a/packages/pinia/src/store.ts
+++ b/packages/pinia/src/store.ts
@@ -48,7 +48,7 @@ import {
 import { setActivePinia, piniaSymbol, Pinia, activePinia } from './rootStore'
 import { IS_CLIENT } from './env'
 import { patchObject } from './hmr'
-import { addSubscription, triggerSubscriptions } from './subscriptions'
+import { addSubscription, triggerSubscriptions, noop } from './subscriptions'
 
 type _ArrayType<AT> = AT extends Array<infer T> ? T : never
 
@@ -173,8 +173,6 @@ function createOptionsStore<
   return store as any
 }
 
-const noop = () => {}
-
 function createSetupStore<
   Id extends string,
   SS,
@@ -376,10 +374,11 @@ function createSetupStore<
     $patch,
     $reset,
     $subscribe(callback, options = {}) {
-      const _removeSubscription = addSubscription(
+      const removeSubscription = addSubscription(
         subscriptions,
         callback,
-        options.detached
+        options.detached,
+        () => stopWatcher()
       )
       const stopWatcher = scope.run(() =>
         watch(
@@ -400,11 +399,6 @@ function createSetupStore<
         )
       )!
 
-      const removeSubscription = () => {
-        stopWatcher()
-        _removeSubscription()
-      }
-
       return removeSubscription
     },
     $dispose,

diff --git a/packages/pinia/src/subscriptions.ts b/packages/pinia/src/subscriptions.ts
index 86fef17063e867d9b023db0aff76b4d9e421e2be..5dfd2620fa09409be3267f1832f1c540c97d2cc6 100644 (file)
--- a/packages/pinia/src/subscriptions.ts
+++ b/packages/pinia/src/subscriptions.ts
@@ -1,10 +1,13 @@
 import { getCurrentInstance, onUnmounted } from 'vue-demi'
 import { _Method } from './types'
 
+export const noop = () => {}
+
 export function addSubscription<T extends _Method>(
   subscriptions: T[],
   callback: T,
-  detached?: boolean
+  detached?: boolean,
+  onCleanup: () => void = noop
 ) {
   subscriptions.push(callback)
 
@@ -12,6 +15,7 @@ export function addSubscription<T extends _Method>(
     const idx = subscriptions.indexOf(callback)
     if (idx > -1) {
       subscriptions.splice(idx, 1)
+      onCleanup()
     }
   }
 

diff --git a/packages/pinia/src/types.ts b/packages/pinia/src/types.ts
index 356c1f34fdb45e268c67b2536a59fdf446b0f93c..d602758205347d173b3d07c4466a1b75cecb11d5 100644 (file)
--- a/packages/pinia/src/types.ts
+++ b/packages/pinia/src/types.ts
@@ -333,11 +333,11 @@ export interface _StoreWithState<
   /**
    * Group multiple changes into one function. Useful when mutating objects like
    * Sets or arrays and applying an object patch isn't practical, e.g. appending
-   * to an array.
+   * to an array. The function passed to `$patch()` **must be synchronous**.
    *
    * @param stateMutator - function that mutates `state`, cannot be async
    */
-  $patch<F extends (state: UnwrapRef<S>) => void>(
+  $patch<F extends (state: UnwrapRef<S>) => any>(
     // this prevents the user from using `async` which isn't allowed
     stateMutator: ReturnType<F> extends Promise<any> ? never : F
   ): void

diff --git a/packages/pinia/test-dts/state.test-d.ts b/packages/pinia/test-dts/state.test-d.ts
index d3b7550e0068b1b4398bf9ca76623cb73ef48793..b69abd4fd662454db62d3ea63d70640201e69446 100644 (file)
--- a/packages/pinia/test-dts/state.test-d.ts
+++ b/packages/pinia/test-dts/state.test-d.ts
@@ -53,6 +53,11 @@ const useStore = defineStore({
 
 const store = useStore()
 
+store.$patch({ counter: 2 })
+store.$patch((state) => {
+  expectType<number>(state.counter)
+})
+
 expectType<number>(store.$state.counter)
 expectType<number>(store.$state.double)
 
@@ -62,3 +67,15 @@ expectType<number>(store.fromARef)
 
 expectType<{ msg: string }>(store.aShallowRef)
 expectType<{ msg: string }>(store.$state.aShallowRef)
+
+const onlyState = defineStore({
+  id: 'main',
+  state: () => ({
+    counter: 0,
+  }),
+})()
+
+onlyState.$patch({ counter: 2 })
+onlyState.$patch((state) => {
+  expectType<number>(state.counter)
+})