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

New Promise API seems to break next(false) implementation in guard #2873

Closed
hazzo opened this issue Aug 7, 2019 · 13 comments
Closed

New Promise API seems to break next(false) implementation in guard #2873

hazzo opened this issue Aug 7, 2019 · 13 comments

Comments

@hazzo
Copy link

hazzo commented Aug 7, 2019

Version

3.1.1

Reproduction link

https://jsfiddle.net/ox6qpz9t/1/

Steps to reproduce

  • Open console
  • Click in foo button that will navigate programmatically to foo route (with replace method) that has a beforeEnter guard.
  • In the console de error Uncaught (in promise) false will appear when navigation is being aborted in guard with next(false)

What is expected?

Abort navigation without error

What is actually happening?

Aborted navigation with unhandled promise error in console.


In version 3.0.7, with the old API (not the new one that returns a promise) was working fine.

@posva
Copy link
Member

posva commented Aug 7, 2019

This is actually intended, before the error was not visible unless a callback was passed to router.replace. If you want to silence the error you should catch it:

this.$router.replace({ name: 'foo' }).catch(err => {
      console.log('all good')
      })

A more detailed explanation: #2881 (comment)

@posva posva closed this as completed Aug 7, 2019
@hazzo
Copy link
Author

hazzo commented Aug 7, 2019

So a navigation abort is an error?
You abort a navigation because there are missing credentials because user is not logged in and I have to catch an error in navigation even though is an intentional abortion, I find it a little bit strange.
But if that's is how new API works its good to know.
Thanks!

@posva
Copy link
Member

posva commented Aug 8, 2019

It's built as an Error but it doesn't necessarily mean it's an error in your app, although it could be in some situations.

push and replace both do an asynchronous navigation (because of guards), if the navigation resolves, they resolve the promise, if the navigation is cancelled, the promise is rejected. Errors are used to keep stack traces and because they are the way to expose reasons for rejected promises, we use an error instance, but we are not throwing an error, we are rejecting the promise of the navigation because it aborted. The reason could be a duplicated navigation, an aborted navigation (per a guard) or an error thrown in a guard
The real error is not catching a rejected promise and that's the error you see in the console: Unhandled Promise Rejection

@hazzo
Copy link
Author

hazzo commented Aug 8, 2019

I understand it and you are right, the error is not catching the promise rejection.

But if this is a "major" API change (because is not backwards compatible with previous implementation) it not should be, as semversion states, 4.0.0 instead of 3.1.*?
I'm not trying to start a brawl, just asking.

@posva
Copy link
Member

posva commented Aug 8, 2019

it's not a major api change. It is even backwards compatible, we can still use callbacks but now when no callbacks are provided, the navigation abortion is visible unless the promise is caught

@ashtonian
Copy link

It is a breaking change if somebody has to change code to implement the new library and maintain the same behavior.

@skinnybeans
Copy link

It's built as an Error but it doesn't necessarily mean it's an error in your app, although it could be in some situations.

push and replace both do an asynchronous navigation (because of guards), if the navigation resolves, they resolve the promise, if the navigation is cancelled, the promise is rejected. Errors are used to keep stack traces and because they are the way to expose reasons for rejected promises, we use an error instance, but we are not throwing an error, we are rejecting the promise of the navigation because it aborted. The reason could be a duplicated navigation, an aborted navigation (per a guard) or an error thrown in a guard
The real error is not catching a rejected promise and that's the error you see in the console: Unhandled Promise Rejection

Is there a way to catch this rejection when navigating using <router-link>?

Or do I need to have a link call a method that calls push() directly so I can catch the promise if it rejects?

@krgaurav1992
Copy link

The hyperlinks generated with <router-link> is not clickable on mac os on any browser but works perfect on widows. Can this issue be a reason for that ?

@posva
Copy link
Member

posva commented Aug 13, 2019 via email

@hs-moosa
Copy link

this is also breaking jest route guard tests. cant there be a global handler for the promise rejection ?

@posva
Copy link
Member

posva commented Aug 22, 2019

@hs-moosa #2881 (comment)

@sstenn
Copy link

sstenn commented Nov 4, 2019

I don't know if it is a good idea, but just return false works for me.

router.beforeEach((to, from, next) => {
  if(condition){
    return false;
  }else{
    next();
  }
})

@posva
Copy link
Member

posva commented Nov 4, 2019

@sstenn no, always call next exactly one time in guards
Locking as this has been resolved and explained in the linked issue

@vuejs vuejs locked as resolved and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants