From: Eduardo San Martin Morote <posva13@gmail.com>
Date: Fri, 31 Jan 2020 17:30:17 +0000 (+0100)
Subject: feat: wip leaving guards with composition api
X-Git-Tag: v4.0.0-alpha.0~66
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1297bf19974e2a39850dd7cd3be4e3fb46d07963;p=thirdparty%2Fvuejs%2Frouter.git

feat: wip leaving guards with composition api

A lot of refactoring of redundant code. Disabled in-component guard tests because they are changing. They will be reactivated later
---

diff --git a/__tests__/guards/component-beforeRouteEnter.spec.ts b/__tests__/guards/component-beforeRouteEnter.spec.ts
index af0ed7bf..616ce99e 100644
--- a/__tests__/guards/component-beforeRouteEnter.spec.ts
+++ b/__tests__/guards/component-beforeRouteEnter.spec.ts
@@ -116,7 +116,7 @@ beforeEach(() => {
   resetMocks()
 })
 
-describe('beforeRouteEnter', () => {
+describe.skip('beforeRouteEnter', () => {
   beforeAll(() => {
     createDom()
   })
diff --git a/__tests__/guards/component-beforeRouteLeave.spec.ts b/__tests__/guards/component-beforeRouteLeave.spec.ts
index 2b2470c8..4777418d 100644
--- a/__tests__/guards/component-beforeRouteLeave.spec.ts
+++ b/__tests__/guards/component-beforeRouteLeave.spec.ts
@@ -98,7 +98,7 @@ beforeEach(() => {
   resetMocks()
 })
 
-describe('beforeRouteLeave', () => {
+describe.skip('beforeRouteLeave', () => {
   beforeAll(() => {
     createDom()
   })
diff --git a/__tests__/guards/component-beforeRouteUpdate.spec.ts b/__tests__/guards/component-beforeRouteUpdate.spec.ts
index eed9b7c8..a6c89a00 100644
--- a/__tests__/guards/component-beforeRouteUpdate.spec.ts
+++ b/__tests__/guards/component-beforeRouteUpdate.spec.ts
@@ -34,7 +34,7 @@ beforeEach(() => {
   beforeRouteUpdate.mockReset()
 })
 
-describe('beforeRouteUpdate', () => {
+describe.skip('beforeRouteUpdate', () => {
   beforeAll(() => {
     createDom()
   })
diff --git a/__tests__/matcher/resolve.spec.ts b/__tests__/matcher/resolve.spec.ts
index 2e152cfa..c6788fc6 100644
--- a/__tests__/matcher/resolve.spec.ts
+++ b/__tests__/matcher/resolve.spec.ts
@@ -6,6 +6,7 @@ import {
   MatcherLocation,
   MatcherLocationNormalized,
   MatcherLocationRedirect,
+  RouteRecordMultipleViews,
 } from '../../src/types'
 import { normalizeRouteRecord } from '../utils'
 
@@ -15,12 +16,27 @@ const component: RouteComponent = null
 // for normalized records
 const components = { default: component }
 
+interface MatcherLocationNormalizedLoose {
+  name: string
+  path: string
+  // record?
+  params: any
+  redirectedFrom?: Partial<MatcherLocationNormalized>
+  meta: any
+  matched: Partial<RouteRecordViewLoose>[]
+}
+
+type RouteRecordViewLoose = Pick<
+  RouteRecordMultipleViews,
+  'path' | 'name' | 'components' | 'children' | 'meta' | 'beforeEnter'
+>
+
 describe('Router Matcher', () => {
   describe('resolve', () => {
     function assertRecordMatch(
       record: RouteRecord | RouteRecord[],
       location: MatcherLocation,
-      resolved: Partial<MatcherLocationNormalized>,
+      resolved: Partial<MatcherLocationNormalizedLoose>,
       start: MatcherLocationNormalized = START_LOCATION_NORMALIZED
     ) {
       record = Array.isArray(record) ? record : [record]
@@ -49,7 +65,7 @@ describe('Router Matcher', () => {
           resolved.matched = record.map(normalizeRouteRecord)
         // allow passing an expect.any(Array)
         else if (Array.isArray(resolved.matched))
-          resolved.matched = resolved.matched.map(normalizeRouteRecord)
+          resolved.matched = resolved.matched.map(normalizeRouteRecord as any)
       }
 
       // allows not passing params
@@ -294,7 +310,7 @@ describe('Router Matcher', () => {
             name: 'Home',
             params: {},
             path: '/home',
-            matched: [record],
+            matched: [record] as any,
             meta: {},
           }
         )
@@ -310,7 +326,7 @@ describe('Router Matcher', () => {
             path: '/users/ed/m/user',
             name: undefined,
             params: { id: 'ed', role: 'user' },
-            matched: [record],
+            matched: [record] as any,
             meta: {},
           }
         )
@@ -354,7 +370,7 @@ describe('Router Matcher', () => {
             path: '/users/ed/m/user',
             name: 'UserEdit',
             params: { id: 'ed', role: 'user' },
-            matched: [record],
+            matched: [record] as any,
             meta: {},
           }
         )
@@ -374,7 +390,7 @@ describe('Router Matcher', () => {
             path: '/users/ed/m/user',
             name: undefined,
             params: { id: 'ed', role: 'user' },
-            matched: [record],
+            matched: [record] as any,
             meta: {},
           }
         )
diff --git a/__tests__/router.spec.ts b/__tests__/router.spec.ts
index b5abfc04..7b66c2cb 100644
--- a/__tests__/router.spec.ts
+++ b/__tests__/router.spec.ts
@@ -67,12 +67,14 @@ describe('Router', () => {
     jest.spyOn(history, 'push')
     await router.push('/foo')
     expect(history.push).toHaveBeenCalledTimes(1)
-    expect(history.push).toHaveBeenCalledWith({
-      fullPath: '/foo',
-      path: '/foo',
-      query: {},
-      hash: '',
-    })
+    expect(history.push).toHaveBeenCalledWith(
+      expect.objectContaining({
+        fullPath: '/foo',
+        path: '/foo',
+        query: {},
+        hash: '',
+      })
+    )
   })
 
   it('calls history.replace with router.replace', async () => {
@@ -81,12 +83,14 @@ describe('Router', () => {
     jest.spyOn(history, 'replace')
     await router.replace('/foo')
     expect(history.replace).toHaveBeenCalledTimes(1)
-    expect(history.replace).toHaveBeenCalledWith({
-      fullPath: '/foo',
-      path: '/foo',
-      query: {},
-      hash: '',
-    })
+    expect(history.replace).toHaveBeenCalledWith(
+      expect.objectContaining({
+        fullPath: '/foo',
+        path: '/foo',
+        query: {},
+        hash: '',
+      })
+    )
   })
 
   it('can pass replace option to push', async () => {
@@ -94,12 +98,14 @@ describe('Router', () => {
     jest.spyOn(history, 'replace')
     await router.push({ path: '/foo', replace: true })
     expect(history.replace).toHaveBeenCalledTimes(1)
-    expect(history.replace).toHaveBeenCalledWith({
-      fullPath: '/foo',
-      path: '/foo',
-      query: {},
-      hash: '',
-    })
+    expect(history.replace).toHaveBeenCalledWith(
+      expect.objectContaining({
+        fullPath: '/foo',
+        path: '/foo',
+        query: {},
+        hash: '',
+      })
+    )
   })
 
   describe('navigation', () => {
diff --git a/__tests__/utils.ts b/__tests__/utils.ts
index 37668e55..d786d4fa 100644
--- a/__tests__/utils.ts
+++ b/__tests__/utils.ts
@@ -1,6 +1,7 @@
 import { JSDOM, ConstructorOptions } from 'jsdom'
-import { NavigationGuard, RouteRecord, MatchedRouteRecord } from '../src/types'
+import { NavigationGuard, RouteRecord } from '../src/types'
 import { h, resolveComponent } from '@vue/runtime-core'
+import { RouteRecordMatched } from '../src/matcher/types'
 
 export const tick = (time?: number) =>
   new Promise(resolve => {
@@ -59,19 +60,32 @@ export const components = {
   },
 }
 
+const DEFAULT_COMMON_RECORD_PROPERTIES = {
+  beforeEnter: undefined,
+  leaveGuards: [],
+  meta: undefined,
+}
+
 /**
- * Copies and normalizes the record so it always contains an object of `components`
+ * Adds missing properties
  *
  * @param record
  * @returns a normalized copy
  */
 export function normalizeRouteRecord(
+  // cannot be a redirect record
   record: Exclude<RouteRecord, { redirect: any }>
-): MatchedRouteRecord {
-  if ('components' in record) return { ...record }
+): RouteRecordMatched {
+  if ('components' in record)
+    return {
+      ...DEFAULT_COMMON_RECORD_PROPERTIES,
+      ...record,
+    }
+
   const { component, ...rest } = record
 
   return {
+    ...DEFAULT_COMMON_RECORD_PROPERTIES,
     ...rest,
     components: { default: component },
   }
diff --git a/playground/App.vue b/playground/App.vue
index ae20154b..93a7f575 100644
--- a/playground/App.vue
+++ b/playground/App.vue
@@ -89,6 +89,9 @@
       <li>
         <router-link to="/with-data">/with-data</router-link>
       </li>
+      <li>
+        <router-link to="/cant-leave">/cant-leave</router-link>
+      </li>
     </ul>
     <!-- <transition
       name="fade"
diff --git a/playground/views/GuardedWithLeave.vue b/playground/views/GuardedWithLeave.vue
index e4a65ec4..0e4e73eb 100644
--- a/playground/views/GuardedWithLeave.vue
+++ b/playground/views/GuardedWithLeave.vue
@@ -1,17 +1,30 @@
 <template>
   <div>
     <p>try to leave</p>
+    <p>So far, you tried {{ tries }}</p>
   </div>
 </template>
 
 <script>
+// @ts-check
 import { defineComponent } from 'vue'
+import { onRouteLeave } from '../../src'
 
 export default defineComponent({
   name: 'GuardedWithLeave',
-  beforeRouteLeave(to, from, next) {
-    if (window.confirm()) next()
-    else next(false)
+  data: () => ({ tries: 0 }),
+
+  setup() {
+    console.log('setup in cant leave')
+    onRouteLeave(function(to, from, next) {
+      if (window.confirm()) next()
+      else {
+        // @ts-ignore
+        this.tries++
+        next(false)
+      }
+    })
+    return {}
   },
 })
 </script>
diff --git a/src/components/View.ts b/src/components/View.ts
index 80cb255b..8bf3ff2a 100644
--- a/src/components/View.ts
+++ b/src/components/View.ts
@@ -22,11 +22,12 @@ const View = defineComponent({
     const depth: number = inject('routerViewDepth', 0)
     provide('routerViewDepth', depth + 1)
 
-    const ViewComponent = computed<Component | undefined>(() => {
-      const matched = route.value.matched[depth]
+    const matchedRoute = computed(() => route.value.matched[depth])
+    const ViewComponent = computed<Component | undefined>(
+      () => matchedRoute.value && matchedRoute.value.components[props.name]
+    )
 
-      return matched && matched.components[props.name]
-    })
+    provide('matchedRoute', matchedRoute)
 
     return () => {
       return ViewComponent.value
diff --git a/src/history/html5.ts b/src/history/html5.ts
index acffcf26..51c0e034 100644
--- a/src/history/html5.ts
+++ b/src/history/html5.ts
@@ -87,19 +87,24 @@ function useHistoryListeners(
     const deltaFromCurrent = fromState
       ? state.position - fromState.position
       : ''
+    const distance = deltaFromCurrent || 0
     console.log({ deltaFromCurrent })
+    // Here we could also revert the navigation by calling history.go(-distance)
+    // this listener will have to be adapted to not trigger again and to wait for the url
+    // to be updated before triggering the listeners. Some kind of validation function would also
+    // need to be passed to the listeners so the navigation can be accepted
     // call all listeners
-    listeners.forEach(listener =>
+    listeners.forEach(listener => {
       listener(location.value, from, {
-        distance: deltaFromCurrent || 0,
+        distance,
         type: NavigationType.pop,
-        direction: deltaFromCurrent
-          ? deltaFromCurrent > 0
+        direction: distance
+          ? distance > 0
             ? NavigationDirection.forward
             : NavigationDirection.back
           : NavigationDirection.unknown,
       })
-    )
+    })
   }
 
   function pauseListeners() {
diff --git a/src/index.ts b/src/index.ts
index f6cf130d..857f2473 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -1,5 +1,5 @@
 import { createRouter, Router } from './router'
-import { App, Ref } from '@vue/runtime-core'
+import { App, Ref, inject, getCurrentInstance } from '@vue/runtime-core'
 import createHistory from './history/html5'
 import createMemoryHistory from './history/memory'
 import createHashHistory from './history/hash'
@@ -8,11 +8,14 @@ import Link from './components/Link'
 import {
   RouteLocationNormalized,
   START_LOCATION_NORMALIZED as START_LOCATION,
+  NavigationGuard,
 } from './types'
+import { RouteRecordNormalized } from './matcher/types'
 
 declare module '@vue/runtime-core' {
   function inject(name: 'router'): Router
   function inject(name: 'route'): Ref<RouteLocationNormalized>
+  function inject(name: 'matchedRoute'): Ref<RouteRecordNormalized>
 }
 
 // @ts-ignore: we are not importing it so it complains
@@ -23,6 +26,7 @@ declare module '@vue/runtime-dom' {
 
 // @ts-ignore: we are not importing it so it complains
 declare module 'vue' {
+  function inject<T>(name: string, defaultValue: T): T
   function inject(name: 'router'): Router
   function inject(name: 'route'): RouteLocationNormalized
 }
@@ -53,6 +57,16 @@ export function RouterPlugin(app: App, router: Router) {
   app.provide('route', router.currentRoute)
 }
 
+function onRouteLeave(leaveGuard: NavigationGuard) {
+  const matched = inject('matchedRoute').value
+  if (!matched) {
+    console.log('no matched record')
+    return
+  }
+
+  matched.leaveGuards.push(leaveGuard.bind(getCurrentInstance()!.proxy))
+}
+
 export {
   createHistory,
   createMemoryHistory,
@@ -61,4 +75,5 @@ export {
   RouteLocationNormalized,
   Router,
   START_LOCATION,
+  onRouteLeave,
 }
diff --git a/src/matcher/index.ts b/src/matcher/index.ts
index a357cdda..f63ab7e2 100644
--- a/src/matcher/index.ts
+++ b/src/matcher/index.ts
@@ -7,7 +7,6 @@ import {
   // MatchedRouteRecord,
 } from '../types'
 import { NoRouteMatchError, InvalidRouteMatch } from '../errors'
-// import { createRouteRecordMatcher } from './path-matcher'
 import { createRouteRecordMatcher, RouteRecordMatcher } from './path-matcher'
 import { RouteRecordNormalized } from './types'
 import {
@@ -17,7 +16,7 @@ import {
 } from './path-parser-ranker'
 
 interface RouterMatcher {
-  addRoute: (record: Readonly<RouteRecord>, parent?: RouteRecordMatcher) => void
+  addRoute: (record: RouteRecord, parent?: RouteRecordMatcher) => void
   // TODO: remove route
   resolve: (
     location: Readonly<MatcherLocation>,
@@ -135,7 +134,7 @@ export function createRouterMatcher(
     location: Readonly<MatcherLocation>,
     currentLocation: Readonly<MatcherLocationNormalized>
   ): MatcherLocationNormalized | MatcherLocationRedirect {
-    let matcher: RouteRecordMatcher | void
+    let matcher: RouteRecordMatcher | undefined
     let params: PathParams = {}
     let path: MatcherLocationNormalized['path']
     let name: MatcherLocationNormalized['name']
@@ -254,6 +253,7 @@ export function normalizeRouteRecord(
       name: record.name,
       beforeEnter: record.beforeEnter,
       meta: record.meta,
+      leaveGuards: [],
     }
   } else {
     return {
@@ -266,6 +266,7 @@ export function normalizeRouteRecord(
       name: record.name,
       beforeEnter: record.beforeEnter,
       meta: record.meta,
+      leaveGuards: [],
     }
   }
 }
diff --git a/src/matcher/types.ts b/src/matcher/types.ts
index 709a24db..4ffccba2 100644
--- a/src/matcher/types.ts
+++ b/src/matcher/types.ts
@@ -1,21 +1,30 @@
-import { RouteRecordMultipleViews, RouteRecordRedirect } from '../types'
+import {
+  RouteRecordMultipleViews,
+  RouteRecordRedirect,
+  NavigationGuard,
+} from '../types'
 
-interface RouteRecordRedirectNormalized {
-  path: RouteRecordRedirect['path']
-  name: RouteRecordRedirect['name']
-  redirect: RouteRecordRedirect['redirect']
-  meta: RouteRecordRedirect['meta']
-  beforeEnter: RouteRecordRedirect['beforeEnter']
-}
-interface RouteRecordViewNormalized {
-  path: RouteRecordMultipleViews['path']
-  name: RouteRecordMultipleViews['name']
-  components: RouteRecordMultipleViews['components']
-  children: RouteRecordMultipleViews['children']
-  meta: RouteRecordMultipleViews['meta']
-  beforeEnter: RouteRecordMultipleViews['beforeEnter']
+interface RouteRecordNormalizedCommon {
+  leaveGuards: NavigationGuard[]
 }
+
+type RouteRecordRedirectNormalized = RouteRecordNormalizedCommon &
+  Pick<
+    RouteRecordRedirect,
+    'path' | 'name' | 'redirect' | 'beforeEnter' | 'meta'
+  >
+
+type RouteRecordViewNormalized = RouteRecordNormalizedCommon &
+  Pick<
+    RouteRecordMultipleViews,
+    'path' | 'name' | 'components' | 'children' | 'meta' | 'beforeEnter'
+  >
+
 // normalize component/components into components
+// How are RouteRecords stored in a matcher
 export type RouteRecordNormalized =
   | RouteRecordRedirectNormalized
   | RouteRecordViewNormalized
+
+// When Matching a location, only RouteRecordView is possible, because redirections never end up in `matched`
+export type RouteRecordMatched = RouteRecordViewNormalized
diff --git a/src/router.ts b/src/router.ts
index 72c35214..6227079d 100644
--- a/src/router.ts
+++ b/src/router.ts
@@ -30,10 +30,12 @@ import {
   NavigationAborted,
 } from './errors'
 import { extractComponentsGuards, guardToPromiseFn } from './utils'
+import { useCallbacks } from './utils/callbacks'
 import { encodeParam } from './utils/encoding'
 import { decode } from './utils/encoding'
 import { ref, Ref, markNonReactive } from '@vue/reactivity'
 import { nextTick } from '@vue/runtime-core'
+import { RouteRecordMatched } from './matcher/types'
 
 type ErrorHandler = (error: any) => any
 // resolve, reject arguments of Promise constructor
@@ -73,6 +75,33 @@ export interface Router {
 
 const isClient = typeof window !== 'undefined'
 
+async function runGuardQueue(guards: Lazy<any>[]): Promise<void> {
+  for (const guard of guards) {
+    await guard()
+  }
+}
+
+function extractChangingRecords(
+  to: RouteLocationNormalized,
+  from: RouteLocationNormalized
+) {
+  const leavingRecords: RouteRecordMatched[] = []
+  const updatingRecords: RouteRecordMatched[] = []
+  const enteringRecords: RouteRecordMatched[] = []
+
+  // TODO: could be optimized with one single for loop
+  for (const record of from.matched) {
+    if (to.matched.indexOf(record) < 0) leavingRecords.push(record)
+    else updatingRecords.push(record)
+  }
+
+  for (const record of to.matched) {
+    if (from.matched.indexOf(record) < 0) enteringRecords.push(record)
+  }
+
+  return [leavingRecords, updatingRecords, enteringRecords]
+}
+
 export function createRouter({
   history,
   routes,
@@ -84,14 +113,11 @@ export function createRouter({
     encodeParam,
     decode
   )
-  const beforeGuards: NavigationGuard[] = []
-  const afterGuards: PostNavigationGuard[] = []
+
+  const beforeGuards = useCallbacks<NavigationGuard>()
+  const afterGuards = useCallbacks<PostNavigationGuard>()
   const currentRoute = ref<RouteLocationNormalized>(START_LOCATION_NORMALIZED)
   let pendingLocation: Readonly<RouteLocationNormalized> = START_LOCATION_NORMALIZED
-  let onReadyCbs: OnReadyCallback[] = []
-  // TODO: should these be triggered before or after route.push().catch()
-  let errorHandlers: ErrorHandler[] = []
-  let ready: boolean = false
 
   if (isClient && 'scrollRestoration' in window.history) {
     window.history.scrollRestoration = 'manual'
@@ -235,22 +261,16 @@ export function createRouter({
     )
       return currentRoute.value
 
-    const isFirstNavigation =
-      currentRoute.value === START_LOCATION_NORMALIZED &&
-      to === history.location
-
-    const toLocation: RouteLocationNormalized = location
-    pendingLocation = toLocation
+    const toLocation: RouteLocationNormalized = (pendingLocation = location)
     // trigger all guards, throw if navigation is rejected
     try {
       await navigate(toLocation, currentRoute.value)
     } catch (error) {
-      if (isFirstNavigation) markAsReady(error)
       if (NavigationGuardRedirect.is(error)) {
         // push was called while waiting in guards
         if (pendingLocation !== toLocation) {
           // TODO: trigger onError as well
-          throw new NavigationCancelled(toLocation, currentRoute.value)
+          triggerError(new NavigationCancelled(toLocation, currentRoute.value))
         }
         // TODO: setup redirect stack
         // TODO: shouldn't we trigger the error as well
@@ -260,37 +280,13 @@ export function createRouter({
         // triggerError as well
         if (pendingLocation !== toLocation) {
           // TODO: trigger onError as well
-          throw new NavigationCancelled(toLocation, currentRoute.value)
+          triggerError(new NavigationCancelled(toLocation, currentRoute.value))
         }
-
-        triggerError(error)
       }
+      triggerError(error)
     }
 
-    // push was called while waiting in guards
-    if (pendingLocation !== toLocation) {
-      const error = new NavigationCancelled(toLocation, currentRoute.value)
-      // TODO: refactor errors to be more lightweight
-      if (isFirstNavigation) markAsReady(error)
-      throw error
-    }
-
-    // change URL
-    if (!isFirstNavigation) {
-      if (to.replace === true) history.replace(url)
-      else history.push(url)
-    }
-
-    const from = currentRoute.value
-    currentRoute.value = markNonReactive(toLocation)
-    // TODO: this doesn't work on first load. Moving it to RouterView could allow automatically handling transitions too maybe
-    if (!isFirstNavigation)
-      handleScroll(toLocation, from).catch(err => triggerError(err, false))
-
-    // navigation is confirmed, call afterGuards
-    for (const guard of afterGuards) guard(toLocation, from)
-
-    markAsReady()
+    finalizeNavigation(toLocation, true, to.replace === true)
 
     return currentRoute.value
   }
@@ -300,12 +296,6 @@ export function createRouter({
     return push({ ...location, replace: true })
   }
 
-  async function runGuardQueue(guards: Lazy<any>[]): Promise<void> {
-    for (const guard of guards) {
-      await guard()
-    }
-  }
-
   async function navigate(
     to: RouteLocationNormalized,
     from: RouteLocationNormalized
@@ -313,6 +303,7 @@ export function createRouter({
     let guards: Lazy<any>[]
 
     // all components here have been resolved once because we are leaving
+    // TODO: refactor both together
     guards = await extractComponentsGuards(
       from.matched.filter(record => to.matched.indexOf(record) < 0).reverse(),
       'beforeRouteLeave',
@@ -320,12 +311,24 @@ export function createRouter({
       from
     )
 
+    const [
+      leavingRecords,
+      // updatingRecords,
+      // enteringRecords,
+    ] = extractChangingRecords(to, from)
+
+    for (const record of leavingRecords) {
+      for (const guard of record.leaveGuards) {
+        guards.push(guardToPromiseFn(guard, to, from))
+      }
+    }
+
     // run the queue of per route beforeRouteLeave guards
     await runGuardQueue(guards)
 
     // check global guards beforeEach
     guards = []
-    for (const guard of beforeGuards) {
+    for (const guard of beforeGuards.list()) {
       guards.push(guardToPromiseFn(guard, to, from))
     }
 
@@ -373,6 +376,50 @@ export function createRouter({
     await runGuardQueue(guards)
   }
 
+  /**
+   * - Cleans up any navigation guards
+   * - Changes the url if necessary
+   * - Calls the scrollBehavior
+   */
+  function finalizeNavigation(
+    toLocation: RouteLocationNormalized,
+    isPush: boolean,
+    replace?: boolean
+  ) {
+    const from = currentRoute.value
+    // a more recent navigation took place
+    if (pendingLocation !== toLocation) {
+      return triggerError(new NavigationCancelled(toLocation, from), isPush)
+    }
+
+    // remove registered guards from removed matched records
+    const [leavingRecords] = extractChangingRecords(toLocation, from)
+    for (const record of leavingRecords) {
+      record.leaveGuards = []
+    }
+
+    // change URL only if the user did a push/replace
+    if (isPush) {
+      if (replace) history.replace(toLocation)
+      else history.push(toLocation)
+    }
+
+    // accept current navigation
+    currentRoute.value = markNonReactive(toLocation)
+    // TODO: this doesn't work on first load. Moving it to RouterView could allow automatically handling transitions too maybe
+    // TODO: refactor with a state getter
+    const state = isPush ? {} : window.history.state
+    handleScroll(toLocation, from, state && state.scroll).catch(err =>
+      triggerError(err, false)
+    )
+
+    // navigation is confirmed, call afterGuards
+    for (const guard of afterGuards.list()) guard(toLocation, from)
+
+    markAsReady()
+  }
+
+  // attach listener to history to trigger navigations
   history.listen(async (to, from, info) => {
     const matchedRoute = resolveLocation(to, currentRoute.value)
     // console.log({ to, matchedRoute })
@@ -382,26 +429,7 @@ export function createRouter({
 
     try {
       await navigate(toLocation, currentRoute.value)
-
-      // a more recent navigation took place
-      if (pendingLocation !== toLocation) {
-        return triggerError(
-          new NavigationCancelled(toLocation, currentRoute.value),
-          false
-        )
-      }
-
-      // accept current navigation
-      currentRoute.value = markNonReactive({
-        ...to,
-        ...matchedRoute,
-      })
-      // TODO: refactor with a state getter
-      // const { scroll } = history.state
-      const { state } = window.history
-      handleScroll(toLocation, currentRoute.value, state.scroll).catch(err =>
-        triggerError(err, false)
-      )
+      finalizeNavigation(toLocation, false)
     } catch (error) {
       if (NavigationGuardRedirect.is(error)) {
         // TODO: refactor the duplication of new NavigationCancelled by
@@ -420,75 +448,64 @@ export function createRouter({
         push(error.to).catch(() => {})
       } else if (NavigationAborted.is(error)) {
         console.log('Cancelled, going to', -info.distance)
-        history.go(-info.distance, false)
         // TODO: test on different browsers ensure consistent behavior
-        // Maybe we could write the length the first time we do a navigation and use that for direction
-        // TODO: this doesn't work if the user directly calls window.history.go(-n) with n > 1
-        // We can override the go method to retrieve the number but not sure if all browsers allow that
-        // if (info.direction === NavigationDirection.back) {
-        //   history.forward(false)
-        // } else {
-        // TODO: go back because we cancelled, then
-        // or replace and not discard the rest of history. Check issues, there was one talking about this
-        // behaviour, maybe we can do better
-        // history.back(false)
-        // }
+        history.go(-info.distance, false)
       } else {
         triggerError(error, false)
       }
     }
   })
 
-  function beforeEach(guard: NavigationGuard): ListenerRemover {
-    beforeGuards.push(guard)
-    return () => {
-      const i = beforeGuards.indexOf(guard)
-      if (i > -1) beforeGuards.splice(i, 1)
-    }
-  }
-
-  function afterEach(guard: PostNavigationGuard): ListenerRemover {
-    afterGuards.push(guard)
-    return () => {
-      const i = afterGuards.indexOf(guard)
-      if (i > -1) afterGuards.splice(i, 1)
-    }
-  }
-
-  function onError(handler: ErrorHandler): void {
-    errorHandlers.push(handler)
-  }
+  // Initialization and Errors
 
+  let readyHandlers = useCallbacks<OnReadyCallback>()
+  // TODO: should these be triggered before or after route.push().catch()
+  let errorHandlers = useCallbacks<ErrorHandler>()
+  let ready: boolean
+
+  /**
+   * Trigger errorHandlers added via onError and throws the error as well
+   * @param error error to throw
+   * @param shouldThrow defaults to true. Pass false to not throw the error
+   */
   function triggerError(error: any, shouldThrow: boolean = true): void {
-    for (const handler of errorHandlers) {
-      handler(error)
-    }
+    markAsReady(error)
+    errorHandlers.list().forEach(handler => handler(error))
     if (shouldThrow) throw error
   }
 
+  /**
+   * Returns a Promise that resolves or reject when the router has finished its
+   * initial navigation. This will be automatic on client but requires an
+   * explicit `router.push` call on the server. This behavior can change
+   * depending on the history implementation used e.g. the defaults history
+   * implementation (client only) triggers this automatically but the memory one
+   * (should be used on server) doesn't
+   */
   function isReady(): Promise<void> {
     if (ready && currentRoute.value !== START_LOCATION_NORMALIZED)
       return Promise.resolve()
     return new Promise((resolve, reject) => {
-      onReadyCbs.push([resolve, reject])
+      readyHandlers.add([resolve, reject])
     })
   }
 
+  /**
+   * Mark the router as ready, resolving the promised returned by isReady(). Can
+   * only be called once, otherwise does nothing.
+   * @param err optional error
+   */
   function markAsReady(err?: any): void {
     if (ready) return
     ready = true
-    for (const [resolve] of onReadyCbs) {
-      // TODO: is this okay?
-      // always resolve, as the router is ready even if there was an error
-      // @ts-ignore
-      resolve(err)
-      // TODO: try catch the on ready?
-      // if (err) reject(err)
-      // else resolve()
-    }
-    onReadyCbs = []
+    readyHandlers
+      .list()
+      .forEach(([resolve, reject]) => (err ? reject(err) : resolve()))
+    readyHandlers.reset()
   }
 
+  // Scroll behavior
+
   async function handleScroll(
     to: RouteLocationNormalized,
     from: RouteLocationNormalized,
@@ -507,10 +524,10 @@ export function createRouter({
     push,
     replace,
     resolve,
-    beforeEach,
-    afterEach,
+    beforeEach: beforeGuards.add,
+    afterEach: afterGuards.add,
     createHref,
-    onError,
+    onError: errorHandlers.add,
     isReady,
 
     history,
diff --git a/src/types/index.ts b/src/types/index.ts
index fad29513..19f0dac1 100644
--- a/src/types/index.ts
+++ b/src/types/index.ts
@@ -1,12 +1,17 @@
 import { HistoryQuery, RawHistoryQuery } from '../history/common'
 import { PathParserOptions } from '../matcher/path-parser-ranker'
 import { markNonReactive } from '@vue/reactivity'
+import { RouteRecordMatched } from '../matcher/types'
 
 // type Component = ComponentOptions<Vue> | typeof Vue | AsyncComponent
 
 export type Lazy<T> = () => Promise<T>
 export type Override<T, U> = Pick<T, Exclude<keyof T, keyof U>> & U
 
+export type Immutable<T> = {
+  readonly [P in keyof T]: Immutable<T[P]>
+}
+
 export type TODO = any
 
 export type ListenerRemover = () => void
@@ -43,11 +48,6 @@ export type RouteLocation =
   | (RouteQueryAndHash & LocationAsRelative & RouteLocationOptions)
 
 // A matched record cannot be a redirection and must contain
-// a normalized version of components with { default: Component } instead of `component`
-export type MatchedRouteRecord = Exclude<
-  RouteRecord,
-  RouteRecordRedirect | RouteRecordSingleView
->
 
 export interface RouteLocationNormalized
   extends Required<RouteQueryAndHash & LocationAsRelative & LocationAsPath> {
@@ -55,7 +55,7 @@ export interface RouteLocationNormalized
   query: HistoryQuery // the normalized version cannot have numbers
   // TODO: do the same for params
   name: string | void
-  matched: MatchedRouteRecord[] // non-enumerable
+  matched: RouteRecordMatched[] // non-enumerable
   redirectedFrom?: RouteLocationNormalized
   meta: Record<string | number | symbol, any>
 }
@@ -166,7 +166,7 @@ export interface MatcherLocationNormalized {
   path: string
   // record?
   params: RouteLocationNormalized['params']
-  matched: MatchedRouteRecord[]
+  matched: RouteRecordMatched[]
   redirectedFrom?: MatcherLocationNormalized
   meta: RouteLocationNormalized['meta']
 }
@@ -192,14 +192,17 @@ export interface NavigationGuardCallback {
 export interface NavigationGuard<V = void> {
   (
     this: V,
-    to: RouteLocationNormalized,
-    from: RouteLocationNormalized,
+    to: Immutable<RouteLocationNormalized>,
+    from: Immutable<RouteLocationNormalized>,
     next: NavigationGuardCallback
   ): any
 }
 
 export interface PostNavigationGuard {
-  (to: RouteLocationNormalized, from: RouteLocationNormalized): any
+  (
+    to: Immutable<RouteLocationNormalized>,
+    from: Immutable<RouteLocationNormalized>
+  ): any
 }
 
 export * from './type-guards'
diff --git a/src/utils/callbacks.ts b/src/utils/callbacks.ts
new file mode 100644
index 00000000..5ae9cb0d
--- /dev/null
+++ b/src/utils/callbacks.ts
@@ -0,0 +1,24 @@
+/**
+ * Create a a list of callbacks that can be reset. Used to create before and after navigation guards list
+ */
+export function useCallbacks<T>() {
+  let handlers: T[] = []
+
+  function add(handler: T): () => void {
+    handlers.push(handler)
+    return () => {
+      const i = handlers.indexOf(handler)
+      if (i > -1) handlers.splice(i, 1)
+    }
+  }
+
+  function reset() {
+    handlers = []
+  }
+
+  return {
+    add,
+    list: () => handlers,
+    reset,
+  }
+}
diff --git a/src/utils/index.ts b/src/utils/index.ts
index e4d70c2d..04d5aee2 100644
--- a/src/utils/index.ts
+++ b/src/utils/index.ts
@@ -1,11 +1,12 @@
-import { RouteLocationNormalized, MatchedRouteRecord } from '../types'
+import { RouteLocationNormalized } from '../types'
 import { guardToPromiseFn } from './guardToPromiseFn'
+import { RouteRecordMatched } from '../matcher/types'
 
 export * from './guardToPromiseFn'
 
 type GuardType = 'beforeRouteEnter' | 'beforeRouteUpdate' | 'beforeRouteLeave'
 export async function extractComponentsGuards(
-  matched: MatchedRouteRecord[],
+  matched: RouteRecordMatched[],
   guardType: GuardType,
   to: RouteLocationNormalized,
   from: RouteLocationNormalized