Skip to content

Commit

Permalink
fix: avoid resetting store state when registering a dynamic module
Browse files Browse the repository at this point in the history
Fixes vuejs#2197

At the moment, when registering a dynamic module, we call
`resetStoreState()` just to register the getters for the new module.

It seems unnecessary to reset the entire store state in this case, and
this actually also leads to [other issues][1].

This change is based on the test case added in
vuejs#2201

The approach taken in this change is to refactor the getter registration
into its own function, and call that new method when registering a
dynamic module instead of resetting the store state.

[1]: vuejs#2197
  • Loading branch information
alecgibson committed Aug 31, 2023
1 parent 99d0b99 commit 125c229
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 26 deletions.
47 changes: 25 additions & 22 deletions src/store-util.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { reactive, computed, watch, effectScope } from 'vue'
import { forEachValue, isObject, isPromise, assert, partial } from './util'
import { isObject, isPromise, assert, partial } from './util'

export function genericSubscribe (fn, subs, options) {
if (subs.indexOf(fn) < 0) {
Expand Down Expand Up @@ -35,36 +35,19 @@ export function resetStoreState (store, state, hot) {
store.getters = {}
// reset local getters cache
store._makeLocalGettersCache = Object.create(null)
const wrappedGetters = store._wrappedGetters
const computedObj = {}
const computedCache = {}

// create a new effect scope and create computed object inside it to avoid
// getters (computed) getting destroyed on component unmount.
const scope = effectScope(true)

scope.run(() => {
forEachValue(wrappedGetters, (fn, 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.
computedObj[key] = partial(fn, store)
computedCache[key] = computed(() => computedObj[key]())
Object.defineProperty(store.getters, key, {
get: () => computedCache[key].value,
enumerable: true // for local getters
})
})
})
// 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
registerGetters(store, Object.keys(store._wrappedGetters))

store._state = reactive({
data: state
})

// 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

// enable strict mode for new state
if (store.strict) {
enableStrictMode(store)
Expand All @@ -86,6 +69,26 @@ export function resetStoreState (store, state, hot) {
}
}

export function registerGetters (store, getterKeys) {
const computedObj = {}
const computedCache = {}

store._scope.run(() => {
getterKeys.forEach((key) => {
const fn = store._wrappedGetters[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.
computedObj[key] = partial(fn, store)
computedCache[key] = computed(() => computedObj[key]())
Object.defineProperty(store.getters, key, {
get: () => computedCache[key].value,
enumerable: true // for local getters
})
})
})
}

export function installModule (store, rootState, path, module, hot) {
const isRoot = !path.length
const namespace = store._modules.getNamespace(path)
Expand Down
15 changes: 11 additions & 4 deletions src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
installModule,
resetStore,
resetStoreState,
unifyObjectStyle
unifyObjectStyle,
registerGetters
} from './store-util'

export function createStore (options) {
Expand Down Expand Up @@ -227,9 +228,15 @@ export class Store {
}

this._modules.register(path, rawModule)
installModule(this, this.state, path, this._modules.get(path), options.preserveState)
// reset store to update getters...
resetStoreState(this, this.state)
const module = this._modules.get(path)
const namespace = this._modules.getNamespace(path)
installModule(this, this.state, path, module, options.preserveState)

const getterKeys = []
module.forEachGetter((getter, key) => {
getterKeys.push(namespace + key)
})
registerGetters(this, getterKeys)
}

unregisterModule (path) {
Expand Down

0 comments on commit 125c229

Please sign in to comment.