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 Sep 1, 2023
1 parent b5d790a commit 7058043
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 25 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
9 changes: 6 additions & 3 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 @@ -228,8 +229,10 @@ 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 namespace = this._modules.getNamespace(path)
const getterKeys = Object.keys(rawModule.getters || {}).map((key) => namespace + key)
registerGetters(this, getterKeys)
}

unregisterModule (path) {
Expand Down

0 comments on commit 7058043

Please sign in to comment.