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: avoid resetting store state when registering a dynamic module #2234

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alecgibson
Copy link

Fixes #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.

This change is based on the test case added in #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.

@netlify
Copy link

netlify bot commented Aug 31, 2023

Deploy Preview for vuex-docs ready!

Name Link
🔨 Latest commit f46e028
🔍 Latest deploy log https://app.netlify.com/sites/vuex-docs/deploys/64f0a85148bda40008442370
😎 Deploy Preview https://deploy-preview-2234--vuex-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

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
@alecgibson
Copy link
Author

Okay I thought this would be cleaner than #2201 but it unfortunately doesn't handle module unregistration (which also clobbers the state). I think the approach in this PR would need basically a full rewrite of the unregisterModule() method to avoid completely resetting the rest of the store state, which is a pretty big change. Happy to do it if it's a direction that people want to go in, but otherwise let's go for #2201

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getters - Reactivity broken after registering new module
2 participants