fix(vue-router): clear navigation info when a guard aborts navigation

packages/vue-router/src/router.ts

@@ -29,6 +29,7 @@ export const createIonRouter = (
     direction: undefined,
     action: undefined,
     delta: undefined,
+    to: undefined,
   };
 
   /**
@@ -46,7 +47,11 @@ export const createIonRouter = (
       _: RouteLocationNormalized,
       failure?: NavigationFailure | void
     ) => {
-      if (failure) return;
+      if (failure) {
+        discardStagedStateFor(to);
+
+        return;
+      }
 
       const { direction, action, delta } = currentNavigationInfo;
 
@@ -68,10 +73,26 @@ export const createIonRouter = (
         direction: undefined,
         action: undefined,
         delta: undefined,
+        to: undefined,
       };
     }
   );
 
+  /**
+   * A guard that throws, including an await on a session check that rejects,
+   * never reaches afterEach. vue-router rejects the navigation promise
+   * instead, so there is no failure to inspect there and the staged state
+   * would survive. This does not handle the error, so navigation outcomes are
+   * unchanged.
+   *
+   * A guard that returns a location is still not covered, because that
+   * redirects rather than fails and afterEach is never called for the original
+   * navigation.
+   */
+  router.onError((_error: unknown, to: RouteLocationNormalized) => {
+    discardStagedStateFor(to);
+  });
+
   const locationHistory = createLocationHistory();
 
   /**
@@ -89,6 +110,99 @@ export const createIonRouter = (
    * Cleared once `handleHistoryChange` has consumed them.
    */
   let incomingRouteParams: RouteParams | undefined;
+  /**
+   * The location the staged params were meant for. Kept beside the params
+   * rather than on them so it is never spread onto a RouteInfo. Left undefined
+   * by the helpers that hand off to history and so cannot know the target yet,
+   * which is `goBack`, `goForward` and `handleNavigateBack`. Those fall back to
+   * the delta's target, which history always records for them.
+   */
+  let incomingRouteParamsTo: string | undefined;
+
+  /**
+   * `resolve` encodes a query differently depending on whether it was handed a
+   * string or an object, so a space in a value survives the string form and
+   * becomes a plus in the object form. The location `afterEach` reports always
+   * uses the object form, so resolve a second time to normalize it.
+   */
+  const resolveFullPath = (to: RouteLocationRaw) => {
+    const resolved = router.resolve(to);
+
+    return router.resolve({
+      path: resolved.path,
+      query: resolved.query,
+      hash: resolved.hash,
+    }).fullPath;
+  };
+
+  /**
+   * The only place that stages route params, so the recorded location can
+   * never be left over from an earlier navigation. Pass the target when it is
+   * known, and omit it to fall back to the delta.
+   */
+  const stageRouteParams = (params: RouteParams, to?: RouteLocationRaw) => {
+    incomingRouteParams = params;
+    incomingRouteParamsTo = to ? resolveFullPath(to) : undefined;
+  };
+
+  /**
+   * The only place that clears them, so a target can never outlive the params
+   * it was recorded for and go on to match an unrelated navigation.
+   */
+  const clearStagedParams = () => {
+    incomingRouteParams = undefined;
+    incomingRouteParamsTo = undefined;
+  };
+
+  /**
+   * State staged for a navigation that did not complete describes something
+   * that did not happen. handleHistoryChange normally consumes it, but it does
+   * not run for a navigation that failed, so it has to be discarded here or
+   * the next navigation picks it up instead.
+   *
+   * A delta is only staged for a history navigation, and a stale one makes the
+   * next navigation look like traversal, which stops the incoming route from
+   * being added. A stale set of params carries an action, a direction and
+   * sometimes a tab or a previous route's id into whatever runs next.
+   *
+   * Only discard state belonging to this navigation, and check the two slots
+   * separately. Another navigation can replace this one and stage its own
+   * state first, in which case discarding would strip that state from the
+   * navigation still running. Params staged without a target fall back to the
+   * delta's target, which history always records for the helpers that omit
+   * one.
+   *
+   * The params were staged from what the caller asked for, so a `redirect:`
+   * record leaves them recorded against the location before the redirect while
+   * `afterEach` reports the one after it. `redirectedFrom` is what the two have
+   * in common. The delta needs no such allowance, since history records the
+   * location the browser actually moved to, which is already the redirected
+   * one.
+   */
+  const discardStagedStateFor = (to: RouteLocationNormalized) => {
+    const deltaIsForThisNavigation =
+      currentNavigationInfo.to === undefined ||
+      currentNavigationInfo.to === to.fullPath;
+
+    const paramsAreForThisNavigation =
+      incomingRouteParamsTo === undefined
+        ? deltaIsForThisNavigation
+        : incomingRouteParamsTo === to.fullPath ||
+          incomingRouteParamsTo === to.redirectedFrom?.fullPath;
+
+    if (deltaIsForThisNavigation) {
+      currentNavigationInfo = {
+        direction: undefined,
+        action: undefined,
+        delta: undefined,
+        to: undefined,
+      };
+    }
+
+    if (paramsAreForThisNavigation) {
+      clearStagedParams();
+    }
+  };
 
   const historyChangeListeners: any[] = [];
 
@@ -101,7 +215,7 @@ export const createIonRouter = (
     });
   }
 
-  opts.history.listen((_: any, _x: any, info: any) => {
+  opts.history.listen((to: any, _x: any, info: any) => {
     /**
      * history.listen only fires on certain
      * event such as when the user clicks the
@@ -123,6 +237,12 @@ export const createIonRouter = (
        */
       action: info.type === "pop" && info.delta >= 1 ? "push" : info.type,
       direction: info.direction === "" ? "forward" : info.direction,
+
+      /**
+       * Recorded so that a failed navigation can tell whether this
+       * information is its own before clearing it.
+       */
+      to,
     };
   });
 
@@ -142,12 +262,12 @@ export const createIonRouter = (
     if (routeInfo && routeInfo.pushedByRoute) {
       const prevInfo = locationHistory.findLastLocation(routeInfo);
       if (prevInfo) {
-        incomingRouteParams = {
+        stageRouteParams({
           ...prevInfo,
           routerAction: "pop",
           routerDirection: "back",
           routerAnimation: routerAnimation || routeInfo.routerAnimation,
-        };
+        });
         if (
           routeInfo.lastPathname === routeInfo.pushedByRoute ||
           /**
@@ -222,7 +342,7 @@ export const createIonRouter = (
              * There is nowhere to navigate, so drop the params rather than
              * letting them leak into the next navigation.
              */
-            incomingRouteParams = undefined;
+            clearStagedParams();
           }
         }
       } else if (defaultHref) {
@@ -240,7 +360,13 @@ export const createIonRouter = (
     routerAnimation?: AnimationBuilder,
     tab?: string
   ) => {
-    setIncomingRouteParams(routerAction, routerDirection, routerAnimation, tab);
+    setIncomingRouteParams(
+      routerAction,
+      routerDirection,
+      routerAnimation,
+      tab,
+      path
+    );
 
     if (routerAction === "push") {
       router.push(path);
@@ -585,7 +711,7 @@ export const createIonRouter = (
 
       currentRouteInfo = routeInfo;
     }
-    incomingRouteParams = undefined;
+    clearStagedParams();
     historyChangeListeners.forEach((cb) => cb(currentRouteInfo));
   };
 
@@ -601,7 +727,13 @@ export const createIonRouter = (
   const navigate = (navigationOptions: ExternalNavigationOptions) => {
     const { routerAnimation, routerDirection, routerLink } = navigationOptions;
 
-    setIncomingRouteParams("push", routerDirection, routerAnimation);
+    setIncomingRouteParams(
+      "push",
+      routerDirection,
+      routerAnimation,
+      undefined,
+      routerLink
+    );
 
     router.push(routerLink);
   };
@@ -665,13 +797,6 @@ export const createIonRouter = (
     const hrefSearch = search ? `?${search}` : "";
 
     if (routeInfo) {
-      incomingRouteParams = {
-        ...incomingRouteParams,
-        routerAction: "push",
-        routerDirection: "none",
-        tab,
-      };
-
       /**
        * When going back to a tab
        * you just left, it's possible
@@ -684,15 +809,23 @@ export const createIonRouter = (
        * are honored when re-selecting the tab.
        */
       const effectiveSearch = hrefSearch || routeInfo.search || "";
-      const push = {
+      const target = {
+        path: routeInfo.pathname === pathname ? routeInfo.pathname : pathname,
         query: parseQuery(effectiveSearch),
         ...(hrefHash ? { hash: hrefHash } : {}),
       };
-      if (routeInfo.pathname === pathname) {
-        router.push({ path: routeInfo.pathname, ...push });
-      } else {
-        router.push({ path: pathname, ...push });
-      }
+
+      stageRouteParams(
+        {
+          ...incomingRouteParams,
+          routerAction: "push",
+          routerDirection: "none",
+          tab,
+        },
+        target
+      );
+
+      router.push(target);
     } else {
       handleNavigate(
         pathname + hrefSearch + hrefHash,
@@ -790,14 +923,18 @@ export const createIonRouter = (
     routerAction: RouteAction = "push",
     routerDirection: RouteDirection = "forward",
     routerAnimation?: AnimationBuilder,
-    tab?: string
+    tab?: string,
+    to?: RouteLocationRaw
   ) => {
-    incomingRouteParams = {
-      routerAction,
-      routerDirection,
-      routerAnimation,
-      tab,
-    };
+    stageRouteParams(
+      {
+        routerAction,
+        routerDirection,
+        routerAnimation,
+        tab,
+      },
+      to
+    );
   };
 
   const goBack = (routerAnimation?: AnimationBuilder) => {

packages/vue-router/src/types.ts

@@ -91,4 +91,10 @@ export interface NavigationInformation {
   action?: RouteAction;
   direction?: RouteDirection;
   delta?: number;
+  /**
+   * The location the browser moved to when this information was staged. Used to
+   * tell whether the information belongs to a particular navigation, since a
+   * second history navigation can stage its own before the first one settles.
+   */
+  to?: string;
 }