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

add warnings when the different namespaced modules has the same names… #1554

Merged
merged 3 commits into from May 22, 2019

Conversation

liyangworld
Copy link
Contributor

Add warnings when the different namespaced modules has the same namespace.
There is a situation that different namespaced modules could have the same namespace, so when in the method of installModule, generate store._modulesNamespaceMap, the follow module will override the before module.
For example:

new Store({
  modules: {
    m1: {
      namespaced: true,
      state: {
        a: "a1"
      }
    },
    m2: {
      modules: {
        m1: {
          namespaced: true,
          state: {
            a: "a2"
          }
        }
      }
    }
  }
})

the module m1 has the namespace 'm1',
the module m2/m1 has the same namespace 'm1'
so in the store._modulesNamespaceMap the module m2/m1 will override the module m1

src/store.js Outdated
if (process.env.NODE_ENV !== 'production') {
console.error(`[vuex] duplicate namespace ${namespace} for the namespaced module ${path.join('/')}`)
}
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't return here please. It will change the current behavior which need major version bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes,I see.
I have removed the return.Thanks.

src/store.js Outdated
@@ -301,6 +301,12 @@ function installModule (store, rootState, path, module, hot) {

// register in namespace map
if (module.namespaced) {
const existedModule = store._modulesNamespaceMap[namespace]
if (existedModule) {
if (process.env.NODE_ENV !== 'production') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge both if statement into one so that uglify will remove entire statement on production build?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks.
I have changed that.

@ktsn ktsn merged commit 91f3e69 into vuejs:dev May 22, 2019
@ktsn
Copy link
Member

ktsn commented May 22, 2019

Thank you!

@liyangworld liyangworld deleted the patch-1 branch May 22, 2019 02:31
@vue-bot
Copy link

vue-bot commented May 22, 2019

Thanks again! 💚

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.

None yet

3 participants