From 6a99b35ee9f3dc4d55b77aa25951d56660bca284 Mon Sep 17 00:00:00 2001 From: "pei.fan" Date: Fri, 9 Dec 2022 16:26:29 +0900 Subject: [PATCH 1/4] fix: keep computed getter be reactive after registering new module --- src/store-util.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/store-util.js b/src/store-util.js index e3ba18609..afd9c44cc 100644 --- a/src/store-util.js +++ b/src/store-util.js @@ -82,6 +82,9 @@ export function resetStoreState (store, state, hot) { // dispose previously registered effect scope if there is one. if (oldScope) { + // Keep the effect that already have dependencies from being killed, + // which will set the existed computed property unreactive. + oldScope.effects = oldScope.effects.filter(({ deps }) => !deps.length) oldScope.stop() } } From 9be7cf7b3bd4a0c9b9473d2501ae334512dd89a7 Mon Sep 17 00:00:00 2001 From: "pei.fan" Date: Fri, 9 Dec 2022 16:49:05 +0900 Subject: [PATCH 2/4] test: add test case --- test/unit/modules.spec.js | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/unit/modules.spec.js b/test/unit/modules.spec.js index 9c6e96e46..7eae663dc 100644 --- a/test/unit/modules.spec.js +++ b/test/unit/modules.spec.js @@ -1,4 +1,4 @@ -import { h, nextTick } from 'vue' +import { computed, h, nextTick } from 'vue' import { mount } from 'test/helpers' import Vuex from '@/index' @@ -925,4 +925,31 @@ describe('Modules', () => { /getters should be function but "getters\.test" in module "foo\.bar" is true/ ) }) + + it('module: computed getter should be reactive after module registration', () => { + const store = new Vuex.Store({ + state: { + foo: 0 + }, + getters: { + getFoo: state => state.foo + }, + mutations: { + incrementFoo: state => state.foo++ + } + }) + + const computedFoo = computed(() => store.getters.getFoo) + store.commit('incrementFoo') + expect(computedFoo.value).toBe(1) + + store.registerModule('bar', { + state: { + bar: 0 + } + }) + + store.commit('incrementFoo') + expect(computedFoo.value).toBe(2) + }) }) From 649854b7a9ba51e985f533f8bf598a6e0b4cf96b Mon Sep 17 00:00:00 2001 From: "pei.fan" Date: Fri, 9 Dec 2022 21:11:09 +0900 Subject: [PATCH 3/4] fix: dynamically merge the alive effects to new scope --- src/store-util.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/store-util.js b/src/store-util.js index afd9c44cc..0d8269e8d 100644 --- a/src/store-util.js +++ b/src/store-util.js @@ -82,9 +82,18 @@ export function resetStoreState (store, state, hot) { // dispose previously registered effect scope if there is one. if (oldScope) { - // Keep the effect that already have dependencies from being killed, - // which will set the existed computed property unreactive. - oldScope.effects = oldScope.effects.filter(({ deps }) => !deps.length) + const deadEffects = [] + oldScope.effects.forEach(effect => { + if (effect.deps.length) { + // Merge the effect that already have dependencies and prevent from being killed. + scope.effects.push(effect) + } else { + // Collect the dead effects. + deadEffects.push(effect) + } + }) + // Dispose the dead effects. + oldScope.effects = deadEffects oldScope.stop() } } From a59d5cd57e9b1d03b0d07e822edd8baef4cda572 Mon Sep 17 00:00:00 2001 From: "pei.fan" Date: Sun, 18 Dec 2022 22:40:52 +0900 Subject: [PATCH 4/4] fix: Remove stale reactive effect from modules after unregistering the module --- src/store-util.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/store-util.js b/src/store-util.js index 0d8269e8d..c138a49f9 100644 --- a/src/store-util.js +++ b/src/store-util.js @@ -30,6 +30,8 @@ export function resetStore (store, hot) { export function resetStoreState (store, state, hot) { const oldState = store._state const oldScope = store._scope + const oldCache = store._computedCache + const oldGettersKeySet = new Set(store.getters ? Object.keys(store.getters) : []) // bind store public getters store.getters = {} @@ -45,6 +47,10 @@ export function resetStoreState (store, state, hot) { scope.run(() => { forEachValue(wrappedGetters, (fn, key) => { + // Filter stale getters' key by comparing oldGetters and wrappedGetters, + // the key does not be removed from oldGettersKeySet are the key of stale computed cache. + // Stale computed cache: the computed cache should be removed as the corresponding module is removed. + oldGettersKeySet.delete(key) // use computed to leverage its lazy-caching mechanism // direct inline function use will lead to closure preserving oldState. // using partial to return function with only arguments preserved in closure environment. @@ -64,6 +70,7 @@ export function resetStoreState (store, state, hot) { // register the newly created effect scope to the store so that we can // dispose the effects when this method runs again in the future. store._scope = scope + store._computedCache = computedCache // enable strict mode for new state if (store.strict) { @@ -83,8 +90,14 @@ export function resetStoreState (store, state, hot) { // dispose previously registered effect scope if there is one. if (oldScope) { const deadEffects = [] + const staleComputedCache = new Set() + oldGettersKeySet.forEach((staleKey) => { + staleComputedCache.add(oldCache[staleKey]) + }) oldScope.effects.forEach(effect => { - if (effect.deps.length) { + // Use the staleComputedCache match the computed property of reactiveEffect, + // to specify the stale cache + if (effect.deps.length && !staleComputedCache.has(effect.computed)) { // Merge the effect that already have dependencies and prevent from being killed. scope.effects.push(effect) } else {