From 3cf24668a4a7b21f496697c2a44cf11a376a34c8 Mon Sep 17 00:00:00 2001 From: doubledare704 Date: Sun, 14 Sep 2025 20:04:55 +0300 Subject: [PATCH 1/3] fix: trigger reactivity for shallowRef when using object syntax - Add triggerRef import from Vue - Implement isShallowRef utility function to detect shallowRef using __v_isShallow - Modify method to detect shallowRef targets and trigger reactivity after patching - Add comprehensive tests for shallowRef reactivity scenarios - Ensure compatibility with existing patch behavior close #2861 --- packages/pinia/__tests__/store.patch.spec.ts | 188 ++++++++++++++++++- packages/pinia/src/store.ts | 40 ++++ 2 files changed, 226 insertions(+), 2 deletions(-) diff --git a/packages/pinia/__tests__/store.patch.spec.ts b/packages/pinia/__tests__/store.patch.spec.ts index 5a94c15ac2..f32155718c 100644 --- a/packages/pinia/__tests__/store.patch.spec.ts +++ b/packages/pinia/__tests__/store.patch.spec.ts @@ -1,5 +1,5 @@ -import { describe, it, expect } from 'vitest' -import { reactive, ref } from 'vue' +import { describe, it, expect, vi } from 'vitest' +import { reactive, ref, shallowRef, computed, nextTick, watchEffect } from 'vue' import { createPinia, defineStore, Pinia, setActivePinia } from '../src' describe('store.$patch', () => { @@ -215,4 +215,188 @@ describe('store.$patch', () => { expect(store.item).toEqual({ a: 1, b: 1 }) }) }) + + describe('shallowRef reactivity', () => { + const useShallowRefStore = () => { + setActivePinia(createPinia()) + return defineStore('shallowRef', () => { + const counter = shallowRef({ count: 0 }) + const counter2 = shallowRef({ count: 0 }) + const counter3 = shallowRef({ count: 0 }) + const nestedCounter = shallowRef({ + nested: { count: 0 }, + simple: 1, + }) + + return { counter, counter2, counter3, nestedCounter } + })() + } + + it('triggers reactivity when patching shallowRef with object syntax', async () => { + const store = useShallowRefStore() + const watcherSpy = vi.fn() + + // Create a computed that depends on the shallowRef + const doubleCount = computed(() => store.counter.count * 2) + + // Watch the computed to verify reactivity + const stopWatcher = watchEffect(() => { + watcherSpy(doubleCount.value) + }) + + expect(watcherSpy).toHaveBeenCalledWith(0) + watcherSpy.mockClear() + + // Patch using object syntax - this should trigger reactivity + store.$patch({ counter: { count: 1 } }) + + await nextTick() + + expect(store.counter.count).toBe(1) + expect(doubleCount.value).toBe(2) + expect(watcherSpy).toHaveBeenCalledWith(2) + + stopWatcher() + }) + + it('triggers reactivity when patching nested properties in shallowRef', async () => { + const store = useShallowRefStore() + const watcherSpy = vi.fn() + + const nestedCount = computed(() => store.nestedCounter.nested.count) + + const stopWatcher = watchEffect(() => { + watcherSpy(nestedCount.value) + }) + + expect(watcherSpy).toHaveBeenCalledWith(0) + watcherSpy.mockClear() + + // Patch nested properties + store.$patch({ + nestedCounter: { + nested: { count: 5 }, + simple: 2, + }, + }) + + await nextTick() + + expect(store.nestedCounter.nested.count).toBe(5) + expect(store.nestedCounter.simple).toBe(2) + expect(nestedCount.value).toBe(5) + expect(watcherSpy).toHaveBeenCalledWith(5) + + stopWatcher() + }) + + it('works with function syntax (baseline test)', async () => { + const store = useShallowRefStore() + const watcherSpy = vi.fn() + + const doubleCount = computed(() => store.counter2.count * 2) + + const stopWatcher = watchEffect(() => { + watcherSpy(doubleCount.value) + }) + + expect(watcherSpy).toHaveBeenCalledWith(0) + watcherSpy.mockClear() + + // Function syntax should work (this was already working) + store.$patch((state) => { + state.counter2 = { count: state.counter2.count + 1 } + }) + + await nextTick() + + expect(store.counter2.count).toBe(1) + expect(doubleCount.value).toBe(2) + expect(watcherSpy).toHaveBeenCalledWith(2) + + stopWatcher() + }) + + it('works with direct assignment (baseline test)', async () => { + const store = useShallowRefStore() + const watcherSpy = vi.fn() + + const doubleCount = computed(() => store.counter3.count * 2) + + const stopWatcher = watchEffect(() => { + watcherSpy(doubleCount.value) + }) + + expect(watcherSpy).toHaveBeenCalledWith(0) + watcherSpy.mockClear() + + // Direct assignment should work (this was already working) + store.counter3 = { count: 3 } + + await nextTick() + + expect(store.counter3.count).toBe(3) + expect(doubleCount.value).toBe(6) + expect(watcherSpy).toHaveBeenCalledWith(6) + + stopWatcher() + }) + + it('handles partial updates correctly', async () => { + const store = useShallowRefStore() + + // Set initial state with multiple properties + store.nestedCounter = { + nested: { count: 10 }, + simple: 20, + } + + // Patch only one property + store.$patch({ + nestedCounter: { + nested: { count: 15 }, + // Note: simple is not included, should remain unchanged + }, + }) + + expect(store.nestedCounter.nested.count).toBe(15) + expect(store.nestedCounter.simple).toBe(20) // Should remain unchanged + }) + + it('works with multiple shallowRefs in single patch', async () => { + const store = useShallowRefStore() + const watcherSpy1 = vi.fn() + const watcherSpy2 = vi.fn() + + const count1 = computed(() => store.counter.count) + const count2 = computed(() => store.counter2.count) + + const stopWatcher1 = watchEffect(() => { + watcherSpy1(count1.value) + }) + + const stopWatcher2 = watchEffect(() => { + watcherSpy2(count2.value) + }) + + watcherSpy1.mockClear() + watcherSpy2.mockClear() + + // Patch multiple shallowRefs at once + store.$patch({ + counter: { count: 10 }, + counter2: { count: 20 }, + }) + + await nextTick() + + expect(store.counter.count).toBe(10) + expect(store.counter2.count).toBe(20) + expect(watcherSpy1).toHaveBeenCalledWith(10) + expect(watcherSpy2).toHaveBeenCalledWith(20) + + stopWatcher1() + stopWatcher2() + }) + }) }) diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index 7ec0e17aeb..1d4aaa4f9e 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -20,6 +20,7 @@ import { Ref, ref, nextTick, + triggerRef, } from 'vue' import { StateTree, @@ -91,6 +92,7 @@ function mergeReactiveObjects< if (!patchToApply.hasOwnProperty(key)) continue const subPatch = patchToApply[key] const targetValue = target[key] + if ( isPlainObject(targetValue) && isPlainObject(subPatch) && @@ -146,6 +148,15 @@ function isComputed(o: any): o is ComputedRef { return !!(isRef(o) && (o as any).effect) } +/** + * Checks if a value is a shallowRef + * @param value - value to check + * @returns true if the value is a shallowRef + */ +function isShallowRef(value: any): value is Ref { + return isRef(value) && !!(value as any).__v_isShallow +} + function createOptionsStore< Id extends string, S extends StateTree, @@ -284,6 +295,10 @@ function createSetupStore< // avoid triggering too many listeners // https://github.com/vuejs/pinia/issues/1129 let activeListener: Symbol | undefined + + // Store reference for shallowRef handling - will be set after setupStore creation + let setupStoreRef: any = null + function $patch(stateMutation: (state: UnwrapRef) => void): void function $patch(partialState: _DeepPartial>): void function $patch( @@ -307,6 +322,28 @@ function createSetupStore< } } else { mergeReactiveObjects(pinia.state.value[$id], partialStateOrMutator) + + // Handle shallowRef reactivity: check if any patched properties are shallowRefs + // and trigger their reactivity manually + if (setupStoreRef) { + const shallowRefsToTrigger: any[] = [] + for (const key in partialStateOrMutator) { + if (partialStateOrMutator.hasOwnProperty(key)) { + // Check if the property in the setupStore is a shallowRef + const setupStoreProperty = setupStoreRef[key] + if ( + isShallowRef(setupStoreProperty) && + isPlainObject(partialStateOrMutator[key]) + ) { + shallowRefsToTrigger.push(setupStoreProperty) + } + } + } + + // Trigger reactivity for all shallowRefs that were patched + shallowRefsToTrigger.forEach(triggerRef) + } + subscriptionMutation = { type: MutationType.patchObject, payload: partialStateOrMutator, @@ -494,6 +531,9 @@ function createSetupStore< pinia._e.run(() => (scope = effectScope()).run(() => setup({ action }))!) )! + // Set setupStore reference for shallowRef handling in $patch + setupStoreRef = setupStore + // overwrite existing actions to support $onAction for (const key in setupStore) { const prop = setupStore[key] From 819f21cf3860254e392ba2f39247bc7662f43f9c Mon Sep 17 00:00:00 2001 From: doubledare704 Date: Mon, 15 Sep 2025 09:02:32 +0300 Subject: [PATCH 2/3] refactor: improve shallowRef handling based on CodeRabbit feedback - Replace setupStoreRef with toRaw(store) to eliminate coupling and state drift - Use Object.prototype.hasOwnProperty.call() for safer property checking - Improve type safety with Ref[] instead of any[] - Remove unnecessary setupStoreRef assignment and variable declaration - Maintain same functionality with more robust implementation --- packages/pinia/src/store.ts | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index 1d4aaa4f9e..7a2024799c 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -296,9 +296,6 @@ function createSetupStore< // https://github.com/vuejs/pinia/issues/1129 let activeListener: Symbol | undefined - // Store reference for shallowRef handling - will be set after setupStore creation - let setupStoreRef: any = null - function $patch(stateMutation: (state: UnwrapRef) => void): void function $patch(partialState: _DeepPartial>): void function $patch( @@ -323,24 +320,21 @@ function createSetupStore< } else { mergeReactiveObjects(pinia.state.value[$id], partialStateOrMutator) - // Handle shallowRef reactivity: check if any patched properties are shallowRefs - // and trigger their reactivity manually - if (setupStoreRef) { - const shallowRefsToTrigger: any[] = [] + // Handle shallowRef reactivity: inspect raw store to avoid ref unwrapping + { + const rawStore = toRaw(store) as Record + const shallowRefsToTrigger: Ref[] = [] for (const key in partialStateOrMutator) { - if (partialStateOrMutator.hasOwnProperty(key)) { - // Check if the property in the setupStore is a shallowRef - const setupStoreProperty = setupStoreRef[key] - if ( - isShallowRef(setupStoreProperty) && - isPlainObject(partialStateOrMutator[key]) - ) { - shallowRefsToTrigger.push(setupStoreProperty) - } + if (!Object.prototype.hasOwnProperty.call(partialStateOrMutator, key)) + continue + const prop = (rawStore as any)[key] + if ( + isShallowRef(prop) && + isPlainObject((partialStateOrMutator as any)[key]) + ) { + shallowRefsToTrigger.push(prop) } } - - // Trigger reactivity for all shallowRefs that were patched shallowRefsToTrigger.forEach(triggerRef) } @@ -531,8 +525,7 @@ function createSetupStore< pinia._e.run(() => (scope = effectScope()).run(() => setup({ action }))!) )! - // Set setupStore reference for shallowRef handling in $patch - setupStoreRef = setupStore + // no-op: `$patch` inspects refs via `toRaw(store)` // overwrite existing actions to support $onAction for (const key in setupStore) { From a66b12473f2b8ff36431f4c221d1f2f3b361ed27 Mon Sep 17 00:00:00 2001 From: doubledare704 Date: Mon, 15 Sep 2025 09:44:02 +0300 Subject: [PATCH 3/3] fix: use Vue public API and improve security - Replace internal __v_isShallow with public isShallow API for Vue version compatibility - Fix hasOwnProperty usage in mergeReactiveObjects to prevent prototype pollution - Remove unnecessary no-op comment for cleaner code - Maintain all existing functionality while improving code quality --- packages/pinia/src/store.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/pinia/src/store.ts b/packages/pinia/src/store.ts index 7a2024799c..b6dee66035 100644 --- a/packages/pinia/src/store.ts +++ b/packages/pinia/src/store.ts @@ -11,6 +11,7 @@ import { markRaw, isRef, isReactive, + isShallow, effectScope, EffectScope, ComputedRef, @@ -89,14 +90,14 @@ function mergeReactiveObjects< // no need to go through symbols because they cannot be serialized anyway for (const key in patchToApply) { - if (!patchToApply.hasOwnProperty(key)) continue + if (!Object.prototype.hasOwnProperty.call(patchToApply, key)) continue const subPatch = patchToApply[key] const targetValue = target[key] if ( isPlainObject(targetValue) && isPlainObject(subPatch) && - target.hasOwnProperty(key) && + Object.prototype.hasOwnProperty.call(target, key) && !isRef(subPatch) && !isReactive(subPatch) ) { @@ -154,7 +155,7 @@ function isComputed(o: any): o is ComputedRef { * @returns true if the value is a shallowRef */ function isShallowRef(value: any): value is Ref { - return isRef(value) && !!(value as any).__v_isShallow + return isRef(value) && isShallow(value) } function createOptionsStore< @@ -525,8 +526,6 @@ function createSetupStore< pinia._e.run(() => (scope = effectScope()).run(() => setup({ action }))!) )! - // no-op: `$patch` inspects refs via `toRaw(store)` - // overwrite existing actions to support $onAction for (const key in setupStore) { const prop = setupStore[key]