Add possibility to handle aborted navigations globally

[bponomarenko]

What problem does this feature solve?

Adds possibility to handle aborted navigations globally.

Initiated vue-router navigation could have different outcomes:

• successful navigation
• errored navigation (in case there is non-internal js error in one of the guards or hooks)
• aborted navigation

In cases, when application logic depends on these navigation results, vue-router has very limited support to catch aborted navigations, and no support to do it globally.

Successful navigation can be catched in global afterEach hook or in beforeResolve guard (however error still can be thrown there). Also there is optional onComplete callback for router.push, but there is no way to set it from <router-link>.
Navigation with error (because of JS error in one of the guards or hooks) can be catched in global onError hook. Also there is optional onAbort callback for router.push (which will be called for both navigation with error and aborted navigation), but there is no way to set it from <router-link>.
When it comes to aborted navigations, the only place where it can be catched – onAbort callback for router.push. However, as mentioned before, it is not possible to set it from <router-link>. Moreover, there is no global hook to catch aborted navigations at all.

Use case

In our application we would like to simply display loading indicator in the root App component while navigation is in progress. Also, we would like to have this functionality be generic, so it can be applied to different applications. The easiest way to achieve it is by creating separate ES module like this navigation-state-plugin.js:

import Vue from 'vue';

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

export default router => {
  // When navigation initiated – update state and proceed
  router.beforeEach((to, from, next) {
    state.navigating = true;
    next();
  });

  // When navigation successfully finished – update state accordingly
  router.afterEach(() => {
    state.navigating = false;
  });

  // When navigation failed –also update state
  router.onError(error => {
    state.navigating = false;
  });
};

In this way in can be applied in the main.js:

import VueRouter from 'vue-router';
import applyPlugin from './navigation-state-plugin.js`;

// do other initializations

const router = new VueRouter(...);
applyPlugin(router);

// init application

Also state from this file can be imported in App.vue and used to display loading indicator when state.navigating is true.

However, when navigation is aborted there is no way to track it in a global manner, which leads to loading indicator to stay visible on a screen.

As possible solution, it was suggested to "use router.push or the v-slot api and get the promise returned from navigate" (which is currently not available because #3042 has no PR). However this solution would require to create wrappers around vue-router API, which should always be used throughout entire application instead of native vue-router api. Even though it is easy to do for router.push, it is cumbersome to do for <router-link> because wrapper should be able to support the same rendering scenarios (without slot content, with slot content, with scoped slot content, etc.)
It is also not suitable for us, because we have shared set of router components, which are re-used in different vue applications. And not all applications will have navigation-state-plugin.js. So we need to have possibility to use native router.push and <router-link> with an option to globally manage navigation state.

What does the proposed API look like?

Most obvious would be to introduce global onAbort hook:

  router.onAbort(function callback(abortedRoute, reason) { ... });

where reason can be one of the errors from #3047

It may be possible to call existing onError or afterEach hooks also for aborted navigations, but that would be a breaking change, and probably would make API more confusing.

[posva]

I was working on an RFC to address a few problems and one of them is the one you are facing. The RFC is here vuejs/rfcs#150. Your input would be helpful 🙂

let's move the discussion there but I don't think it makes sense to add a new api to remove it right away. What you need can be implemented in user land by overriding Router.prototype.push. Something similar to this:

const onAborts = []

const push = Router.prototype.push
Router.prototype.push = function () {
  return push.apply(router, arguments).catch(err => {
    // trigger onAbort callbacks
    return Promise.reject(err)
  })
}

The behavior can be extended to reflect a similar behavior to the one discussed on the RFC

[bponomarenko]

@posva Thanks for your reply and proposed solution. Happy to see RFC to align those guards and callbacks in behavior.

As for proposed workaround, even though it works in some basic cases, unfortunately it doesn't cover all use cases. Let me explain.

Here is what I initially made to extend VueRouter api (based on your suggestion):

import VueRouter from 'vue-router';

const abortCallbacks = [];

const processAbortCallbacks = () => {
    abortCallbacks.forEach(callback => {
        callback();
    });
};

const getExtendedFunction = originalFn => function (...args) {
    // If more than 1 argument - method called with callbacks
    if (args.length > 1) {
        const [location, onComplete, onAbort] = args;

        return originalFn.call(this, location, onComplete, () => {
            processAbortCallbacks();
            onAbort && onAbort();
        });
    }

    // Otherwise it will return promise
    return originalFn.apply(this, args)
        .catch(error => {
            processAbortCallbacks();
            return Promise.reject(error);
        });
}

const routerPush = VueRouter.prototype.push;
const routerReplace = VueRouter.prototype.replace;

VueRouter.prototype.push = getExtendedFunction(routerPush);
VueRouter.prototype.replace = getExtendedFunction(routerReplace);

VueRouter.prototype.onAbort = callback => {
    abortCallbacks.push(callback);
};

Note that I added support for router.push calls with onComplete and onAbort callbacks, as internally <router-link> uses exactly this api.

As I mentioned, this only works for some simplest cases. Consider this routing setup:

new VueRouter({
  routes: [
    { path: '/home' },
    { path: '/redirect', beforeEnter(to, from, next) { next('/home') },
    { path: '/indirect-redirect', beforeEnter(to, from, next) { next('/redirect') },
  ],
});

Supported use case: When user is on /home path, and he clicks on a link which targets to /redirect – beforeEach will be called only once for /redirect route with following onAbort registered callback.

Unsupported use case: When user is on /home path, and he clicks on a link which targets to /indirect-redirect – beforeEach will be called for both /indirect-redirect and /redirect paths. While onAbort will be triggered only once for the first /indirect-redirect. This happens because internally router.push is not used for redirects like that, but history.push is. This problem could probably be solved by overriding History.prototype.push, however I don't see a way to import History class from the vue-router library.
Unfortunately, we have a lot of scenarios which can result in such indirect redirects because we actively utilize global beforeEach guards and route's beforeEnter guards.

This issue is not related to the RFC itself, so I posted it here. Do you think it make sense to re-open this issue, or should I move this reply to the RFC?

[posva]

The thing is the behavior you are describing about onAbort triggering only once while beforeEach triggering twice, is intended because a navigation with redirects inside it (e.g.: next('/home')) still counts as a single navigation from the push perspective as you want be able to know when it's finished, so the promise will resolve or reject at the end of it and onAbort should be consistent with that. Navigation guards, however, still need to trigger again because the navigation could get cancelled.

Note this is different from the point your raised in the RFC, which is valid

[bponomarenko]

@posva Don't get me wrong, I understand that next('/home') will trigger individual navigation, which in effect should trigger beforeEach navigation guard. And it make sense. The challenge I'm having is that next('/home') will not call overriden router.push(), but rather router.history.push() (because all the redirects are happening inside History, and not VueRouter itself. So I have to rather override History.prototype.push instead of VueRouter.prototype.push as you initially suggested. However I cannot import History class, as it is not exported from vue-router library.

I can try to do the following (which theoretically should work, didn't try yet):

export default function applyRouterShim (router) {
  const originalHistoryPush = router.history.constructor.prototype.push;

  router.history.constructor.prototype.push = function extendedPush(location, onComplete, onAbort) {
    return originalHistoryPush.call(this, location, onComplete, error => {
      // ...process abort callbacks
      onAbort && onAbort(error);
    });
  }
}

However I'm not fond of this solution as it is rather "fragile" and based on private, un-documented api (router.history).

[posva]

And it make sense. The challenge I'm having is that next('/home') will not call overriden router.push()

I don't understand why that's a problem, that would call onAbort twice for one call to router.push, which is something that shouldn't happen because of what I explained

[bponomarenko]

The problem that onAbort is not called twice – it is called only once. Let me create Codesandbox example to illustrate it.

[bponomarenko]

@posva Here it is: https://codesandbox.io/s/still-sun-p4tmn?expanddevtools=1
(it is read-only, but I can give you write access by email if needed)

Upon loading, user is on the Home ('/') route. Page should render two links, current view and text "Navigating: false". Current View name will be rendered only when navigation is not active.

Router initialization is in src/router.js.
Module which holds navigation state can be found in src/navigation-state.js.
Shim for onAbort hooks can be found in src/abort-callbacks-shim.js.

Scenario 1 (ok):
Action: Click on 'Direct redirect to Home'.

Expected: In the Console will be printed:

• "before each /redirect"
• "aborted"

Text on the screen should stay as "Navigating: false", view name is visible.

Result: Same as expected

Scenario 2 (not ok):
Action: Click on 'Indirect redirect to Home'.

Expected: In the Console will be printed:

• "before each /indirect-redirect"
• aborted
• "before each /redirect"
• "aborted

Text on the screen should stay as "Navigating: false", view name is visible.

Result: In the Console will be printed:

• "before each /indirect-redirect"
• aborted
• "before each /redirect"

Text on the screen will change to "Navigating: true", view name is not visible.

In real-life app this would result in showing loading indicator instead of actual View.

[posva]

Ah, I see what you mean now, thanks a lot for the detailed explanation. The expected behavior would be

• "before each /indirect-redirect"
• "before each /redirect"
• aborted

And that's how it should be on the next version vue-router (with afterEach instead of onAbort) per the RFC. This is part of the reason I created the RFC to make things more consistent, easier to follow/explain and use.
I don't see a way of handling this without it being a breaking change 🤔

[bponomarenko]

Happy to hear it example made it clear.

I don't see a way of handling this without it being a breaking change 🤔

What about introduction of the onAbort hook, as I proposed initially? It should result in minor version bump, unless I overlook something. Yes, it will be removed in the v4, as your proposed changes in RFC will make it obsolete, but it will solve immediate problem we have today. I open to help with PR implementation.

Btw, I'm not sure what is the guideline you have for VueRouter issues, but does it make sense to re-open this ticket since it is still an issue for current latest version?

[posva]

Introducing an api to remove it right away is, for me, worse that the breaking change because it's likely to create more confusion than the breaking change itself: returned Promise by router.push should be resolved/rejected after internal redirects (next('/other'))

[bponomarenko]

I understand your point. I would try to override router.history.constructor.prototype.push for now, as it seems the only possible workaround, and we cannot wait for release of the v4 with updated functionality. Described challenge with loading indicator is simplest, but not the only part of our application which depends on onAbort hooks – there are more complex scenarios, for which absence of onAbort hooks blocking the whole app.
Thanks for your time today!

[afwn90cj93201nixr2e1re]

@bponomarenko can u provide the code that u ended up with? I mean any real copy-paste examples.