-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Warn about conflicts between state and module #1365
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/store.js
Outdated
if (process.env.NODE_ENV !== 'production') { | ||
if (moduleName in parentState) { | ||
console.warn( | ||
`[vuex] state field "${moduleName}" was overridden by a module with the same name` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simplesmiler what do u think about including the module name in the latter part as well? [vuex] state field "${moduleName}" was overridden by a module with the same name ("${moduleName}")
@simplesmiler do you intend to address the review comment above / complete this pull request? |
@ktsn Given that you already approved, do you think this can be merged as is it? Or would you prefer applying the review comment above? |
@yyx990803 could you please take a look if this can be merged? |
@ktsn @yyx990803 Could you please merge this or comment what's missing? It has been approved over a year ago. 😟 |
b049121
to
db96c45
Compare
How can I close this warn, suddenly printed a lot of such warnings in my project., Although it doesn't matter, but the sudden warning was terrible for me. |
* feat: warn about conflicts between state and module * fix: show module path when it overwrite a state field
Fixes #1363.