Skip to content

Commit

Permalink
fix: getters being destroyed on component destroy (#1878) (#1883)
Browse files Browse the repository at this point in the history
fix #1878
  • Loading branch information
kiaking committed Oct 4, 2021
1 parent 3c56e9a commit b2f851f
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 98 deletions.
8 changes: 4 additions & 4 deletions package.json
Expand Up @@ -52,7 +52,7 @@
},
"homepage": "https://github.com/vuejs/vuex#readme",
"peerDependencies": {
"vue": "^3.0.2"
"vue": "^3.2.0"
},
"dependencies": {
"@vue/devtools-api": "^6.0.0-beta.11"
Expand All @@ -65,7 +65,7 @@
"@rollup/plugin-node-resolve": "^13.0.0",
"@rollup/plugin-replace": "^2.4.2",
"@types/node": "^15.6.0",
"@vue/compiler-sfc": "^3.0.11",
"@vue/compiler-sfc": "^3.2.4",
"babel-jest": "^26.6.3",
"babel-loader": "^8.2.2",
"brotli": "^1.3.2",
Expand All @@ -89,8 +89,8 @@
"todomvc-app-css": "^2.4.1",
"typescript": "^4.2.4",
"vitepress": "^0.11.5",
"vue": "^3.0.11",
"vue-loader": "^16.2.0",
"vue": "^3.2.4",
"vue-loader": "^16.5.0",
"vue-style-loader": "^4.1.3",
"webpack": "^4.43.0",
"webpack-dev-middleware": "^3.7.2",
Expand Down
39 changes: 28 additions & 11 deletions src/store-util.js
@@ -1,4 +1,4 @@
import { reactive, watch } from 'vue'
import { reactive, computed, watch, effectScope } from 'vue'
import { forEachValue, isObject, isPromise, assert, partial } from './util'

export function genericSubscribe (fn, subs, options) {
Expand Down Expand Up @@ -29,30 +29,42 @@ export function resetStore (store, hot) {

export function resetStoreState (store, state, hot) {
const oldState = store._state
const oldScope = store._scope

// bind store public getters
store.getters = {}
// reset local getters cache
store._makeLocalGettersCache = Object.create(null)
const wrappedGetters = store._wrappedGetters
const computedObj = {}
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)
Object.defineProperty(store.getters, key, {
// TODO: use `computed` when it's possible. at the moment we can't due to
// https://github.com/vuejs/vuex/pull/1883
get: () => computedObj[key](),
enumerable: true // for local getters
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
})
})
})

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 @@ -67,6 +79,11 @@ export function resetStoreState (store, state, hot) {
})
}
}

// dispose previously registered effect scope if there is one.
if (oldScope) {
oldScope.stop()
}
}

export function installModule (store, rootState, path, module, hot) {
Expand Down
6 changes: 6 additions & 0 deletions src/store.js
Expand Up @@ -39,6 +39,12 @@ export class Store {
this._modulesNamespaceMap = Object.create(null)
this._subscribers = []
this._makeLocalGettersCache = Object.create(null)

// EffectScope instance. when registering new getters, we wrap them inside
// EffectScope so that getters (computed) would not be destroyed on
// component unmount.
this._scope = null

this._devtools = devtools

// bind commit and dispatch to self
Expand Down
2 changes: 1 addition & 1 deletion test/helpers.js
Expand Up @@ -4,7 +4,7 @@ import puppeteer from 'puppeteer'
export function mount (store, component) {
const el = createElement()

component.render = () => {}
component.render = component.render || (() => {})

const app = createApp(component)

Expand Down
53 changes: 52 additions & 1 deletion test/unit/modules.spec.js
@@ -1,4 +1,5 @@
import { nextTick } from 'vue'
import { h, nextTick } from 'vue'
import { mount } from 'test/helpers'
import Vuex from '@/index'

const TEST = 'TEST'
Expand Down Expand Up @@ -124,6 +125,56 @@ describe('Modules', () => {
store.commit('a/foo')
expect(mutationSpy).toHaveBeenCalled()
})

it('should keep getters when component gets destroyed', async () => {
const store = new Vuex.Store()

const spy = jest.fn()

const moduleA = {
namespaced: true,
state: () => ({ value: 1 }),
getters: {
getState (state) {
spy()
return state.value
}
},
mutations: {
increment: (state) => { state.value++ }
}
}

const CompA = {
template: `<div />`,
created () {
this.$store.registerModule('moduleA', moduleA)
}
}

const CompB = {
template: `<div />`
}

const vm = mount(store, {
components: { CompA, CompB },
data: () => ({ show: 'a' }),
render () {
return this.show === 'a' ? h(CompA) : h(CompB)
}
})

expect(store.getters['moduleA/getState']).toBe(1)
expect(spy).toHaveBeenCalledTimes(1)

vm.show = 'b'
await nextTick()

store.commit('moduleA/increment')

expect(store.getters['moduleA/getState']).toBe(2)
expect(spy).toHaveBeenCalledTimes(2)
})
})

// #524
Expand Down

0 comments on commit b2f851f

Please sign in to comment.