fix: keep computed getter be reactive after registering new modules #2201
Conversation
✅ Deploy Preview for vuex-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| @@ -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', () => { | |||
There was a problem hiding this comment.
The test will fail when removing these lines: https://github.com/vuejs/vuex/pull/2201/files#diff-d7ab881f3cc9bbff70c15e5574df58176996a0b834bc174d3f3cea7a64ad8a95R85-R96
|
@Azurewarth0920 Hi! There may be a problem with the unregisterModule function. |
|
@1085383233 Hi! Can you provide a test case about the problem? |
|
@1085383233
Is the |
@Azurewarth0920 Yeah, but I didn't test it. |
|
Any news on this? The bug is really a bummer. |
|
@catalin-bratu At this time, the workaround for my project is that downgrade the vuex to v4.0.2. |
|
@Azurewarth0920 is there any known bug for this fix you want to merge? |
|
@kiaking any word on this? |
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
|
|
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
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
|
I've closed my alternative solution, which fails this test case: it('module: computed getter should be reactive after module unregistration', () => {
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.unregisterModule('bar')
store.commit('incrementFoo')
expect(computedFoo.value).toBe(2)
})@Azurewarth0920 do you think it's worth adding this test case to your PR? (Your code passes it 👏🏼 ) |
close: #2197
The computed getters will be unreactive after registering modules,
Approach:
When stopping effectScope, only stop the effect that has no dependencies.