Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: keep computed getter be reactive after registering new modules #2201

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/store-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -82,6 +89,24 @@ 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 => {
// 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 {
// Collect the dead effects.
deadEffects.push(effect)
}
})
// Dispose the dead effects.
oldScope.effects = deadEffects
oldScope.stop()
}
}
Expand Down
29 changes: 28 additions & 1 deletion test/unit/modules.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { h, nextTick } from 'vue'
import { computed, h, nextTick } from 'vue'
import { mount } from 'test/helpers'
import Vuex from '@/index'

Expand Down Expand Up @@ -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', () => {
Copy link
Author

@Azurewarth0920 Azurewarth0920 Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
})
})