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

Menu: fix router NavigationDuplicated error when using vue-router@^3.1.0 #17145

Closed
wants to merge 3 commits into from

Conversation

a631807682
Copy link
Contributor

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow Element's contributing guide (中文 | English | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

Close #17044

@element-bot
Copy link
Member

element-bot commented Aug 23, 2019

Deploy preview for element ready!

Built with commit 19bae7d

https://deploy-preview-17145--element.netlify.com

@a631807682
Copy link
Contributor Author

Reproduction Link

https://codepen.io/a631807682/pen/wvwgqmX?editors=1011

Fixed Link

https://codepen.io/a631807682/pen/rNBjGpj?editors=1011

Step

click same path menu, see the console.

Related Issues

there no unit test about menu router, and i have no idea how to write it.
my question is only menu work with vue-router plugin, so which way should i choose to write it?

  1. install vue-router in test/unit/util that run unit test with it everywhere.
  2. create a function to create other vue that install plugin only in menu router unit test, just like vue-test-utils/createLocalVue

@iamkun iamkun self-assigned this Sep 2, 2019
@iamkun
Copy link
Member

iamkun commented Sep 2, 2019

我们考虑用最低成本的方案解决,直接捕获这个错误而不是抛出来

@iamkun iamkun added this to the 2.13.0 milestone Sep 2, 2019
@a631807682
Copy link
Contributor Author

我们考虑用最低成本的方案解决,直接捕获这个错误而不是抛出来

我的理解是,要么不让这个错误发生,要么允许用户添加一个属性,让用户在发生错误的时候执行自定义的方法。而不是直接抛出或者静默。

@iamkun
Copy link
Member

iamkun commented Sep 2, 2019

首先 最完美的办法是 判断是否是相同的路由 如果相同则不跳转,但是 vuerouter 没有一个直接可用的 api, 而引入相关utils 又会对未来 新版本的兼容埋下潜在的坑,

其次这里 NavigationDuplicated 是 menu 组件自身的逻辑 导致 push 了相同的路由,那我们直接静默对用户应该也不回有不良的影响

@iamkun
Copy link
Member

iamkun commented Sep 2, 2019

我提了一个 兼容高低版本的修复方案 可以来 review 下

@a631807682
Copy link
Contributor Author

a631807682 commented Sep 2, 2019

首先 最完美的办法是 判断是否是相同的路由 如果相同则不跳转,但是 vuerouter 没有一个直接可用的 api, 而引入相关utils 又会对未来 新版本的兼容埋下潜在的坑,

其次这里 NavigationDuplicated 是 menu 组件自身的逻辑 导致 push 了相同的路由,那我们直接静默对用户应该也不回有不良的影响

1. 我同意第一点
2. 我不太同意只允许静默,原因是可能某些路由我会有,beforeEnter或者其他hook,比如next(false)会抛出一个Error,此时我希望在menu里有个可能叫作router-error的属性让我去处理我的逻辑,这个属性默认可以是静默的。

我看了router-link,也是静默所有错误,可能是我抓牛角尖了。

@a631807682 a631807682 closed this Sep 2, 2019
@iamkun
Copy link
Member

iamkun commented Sep 2, 2019

这里静默的只是 element menu NavigationDuplicated 错误 ,并不影响用户 router 的任何提示哈

@a631807682
Copy link
Contributor Author

a631807682 commented Sep 2, 2019

这里静默的只是 element menu NavigationDuplicated 错误 ,并不影响用户 router 的任何提示哈

我是指这种情况,我本来觉得这种情况可能要处理,但是我看RouterLink也没处理。
可能是不用处理的吧,这里应该也静默就行了吧?因为实在有需求自己在onError里处理也行,不管是不是NavigationDuplicated错误。

const router = new VueRouter({
  mode: 'history',
  routes: [
    { path: '/foo', name: 'foo', beforeEnter: (to, from, next) => {
        next(new Error('my error'))// 
      }, component: Foo }
  ]
})

this.$router.push({ name: 'foo' },()=>{},(err)=>console.warn(err))// my error here

@a631807682 a631807682 deleted the issues/17044 branch September 25, 2019 08:13
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.

[Bug Report] 升级vue-router至3.1以后版本,导航组件重复点击报错 NavigationDuplicated
3 participants