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 onComplete & onAbort to router-link #3027

Open
XAMPPRocky opened this issue Nov 10, 2019 · 16 comments
Open

Add onComplete & onAbort to router-link #3027

XAMPPRocky opened this issue Nov 10, 2019 · 16 comments
Labels
discussion needs RFC This feature request needs to go through the RFC process to gather more information

Comments

@XAMPPRocky
Copy link

XAMPPRocky commented Nov 10, 2019

What problem does this feature solve?

In 2.2.0 Vue added onComplete and onAbort callbacks to router.push, however there seems to be no equivalvent when using <router-link>. I've seen suggestions that can you use beforeRouteEnter as an alteranative in some cases, however I have a case where I have a <header> containing <router-link>s that remains on the page after a user clicks on the router link, I want to be able to change a property of the header after the user has clicked the link. So beforeRouteEnter would not work for my case. Ideally I'd like to be able to supply onComplete and onAbort to the router-link directly.

What does the proposed API look like?

Personally I have no preference over how this should look, I'm not experienced enough with vue router to say what is the most idiomatic, however I would expect to be something like the following.

<router-link to="" on-complete="callback" on-abort="callback"></router-link>
<!-- or -->
<router-link to="" complete="callback" abort="callback"></router-link>
@kelunik
Copy link

kelunik commented Feb 3, 2020

Implemented by #3114.

@posva
Copy link
Member

posva commented Feb 4, 2020

I completely forgot about this issue. This kind of thing should go through the thinking process of an RFC in our RFC repo at vuejs/rfcs. One of the things I see is using events instead of callbacks, which is more adapted to Vue. Another thing would be giving more specific examples about what is being done onAbort/onComplete that is specific to that router-link. To me this is the kind of thing that would go in an afterEach hook because it looks global.

Right now this can be achieved with an event listener that calls $router.push and building the href with $router.resolve (which is a workaround).

@kelunik
Copy link

kelunik commented Feb 4, 2020

@posva: Could you please clarify how the workaround would look like? Currently the best solution without this PR seems to be forking the RouterLink for me.

@bponomarenko
Copy link

bponomarenko commented Mar 6, 2020

We have one use case, which might be connected to this issue, but correct me if I'm wrong.
What I'm trying to achieve in an application, is to simply display loading indicator on the app level while navigation is in progress. In most cases this can be covered by doing:

const state = Vue.observable({ navigating: false });

router.beforeEach((to, from, next) {
  state.navigating = true;
  next();
});

router.afterEach(() => {
  state.navigating = false;
});

By registering these guards before any other guards, I can make sure that loading indicator will be shown until all other global, in-component and router guards are resolved, async components are loaded, etc.
If one of the guards calls next('another-route'), previous navigation will be aborted. But a new navigation will start right away, so loading indicator will stay visible until it is done. In situation when one of the guards calls next(new Error(...)), afterEach guard will not be called. To cover this use case, we can:

router.onError(() => {
  state.navigating = false;
});

That is good. However, if one of the guards will do next(false) or user redirected to the same link at which he currently is, there is no way to track it. It feels like global router.onAbort() guard will be sufficient here. Or, as mentioned above, afterEach will be called with indication about aborted navigation. But there are non of them, and I don't how to workaround this challenge.

@bponomarenko
Copy link

@posva I can work on Pull Request to add router's onAbort() guard, if you agree about it conceptually. What do you think?

@posva
Copy link
Member

posva commented Mar 11, 2020

This is not about a guard 😅
I think there was a different issue for that. It does need to wait until the error naming is settled

@bponomarenko
Copy link

Is there any issue, ticket or rfc where progress for this feature can be tracked? I linked PR above, which seems related and can potentially solve this issue.
There is no workaround for it atm, but I would be glad to help implement it once it is clear how exactly it should be done.

@bponomarenko
Copy link

@posva Is the changes in connected PR #3047 are related to "wait until the error naming is settled", or there is different discussion somewhere in RFC or GitHub Issue thread?

@posva
Copy link
Member

posva commented Mar 16, 2020

@bponomarenko this issue is about adding two event handlers to router-link, it's still at a discussion state. I don't think it's worth adding because if you need to react to the navigation state, you will be better off using router.push or the v-slot api and get the promise returned from navigate (#3042 which is up for a PR 👍).
The PR you are linking is related to Errors to recognise why the navigation failed and it's waiting for naming to settle at vuejs/router#123 to reduce breaking changes

@XAMPPRocky
Copy link
Author

XAMPPRocky commented Mar 16, 2020

@posva As a user it feel odd to me that there is this limitation between what's available through <router-link> vs router.push. Adding on-complete and on-abort feel like bringing the declarative API to parity with the imperative API, it's not creating a new API. I prefer using the declarative <router-link> API where possible. The v-slot API does appear to provide a solution, though with some more boilerplate than on-complete.

I'm not familiar if that level of change requires an RFC or not, but I already know I wouldn't be able to commit to the time to write it. If someone else wants to take this on feel free.

@bponomarenko
Copy link

@posva You are right, this issue is about changes to <router-link> api, while I was talking about slightly different solution which I thought would help to solve this issue as well.

Proposed changes in referred #3042 doesn't really solve a problem for us, as it is not possible to always use router.push or v-slot api of <router-link> with custom onAbort handlers. I will create another issue with more detailed explanation. Thanks.

@afwn90cj93201nixr2e1re
Copy link

Also need global OnAbort router hook.

@afwn90cj93201nixr2e1re
Copy link

Lol, i explained same thing one year ago...
#3027 (comment)
@posva

And today you said again that i should read docs, lol...

@posva posva added the needs RFC This feature request needs to go through the RFC process to gather more information label Sep 9, 2020
@mitar
Copy link

mitar commented Mar 27, 2022

These days one can do await router.push(...) and then after await returns, you can do whatever. Or if it raises an exception, you can catch it.

But nothing like that seems to exist for router-link. :-(

@mitar
Copy link

mitar commented Mar 27, 2022

(My use case is that I want to focus a particular element on the new page after the user clicks on a particular link.)

@mitar
Copy link

mitar commented Mar 28, 2022

OK, it seems that in v4, navigate returns a promise, so one can make a custom router-link which awaits on navigate and then does additional logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needs RFC This feature request needs to go through the RFC process to gather more information
Projects
None yet
Development

No branches or pull requests

6 participants