refactor(guards): remove this access on composition guards

BREAKING CHANGE: `onBeforeRouteLeave` and `onBeforeRouteUpdate` used to
have access to the component instance through `instance.proxy` but given
that:
1. It has been marked as `internal` (vuejs/core#1849)
2. When using `setup`, all variables are accessible on the scope (and
   should be accessed that way because the code minimizes better)
It has been removed to prevent wrong usage and lighten Vue Router

Author: posva

__tests__/guards/onBeforeRouteLeave.spec.ts

@@ -13,58 +13,10 @@ const component = {
 }
 
 describe('onBeforeRouteLeave', () => {
-  it('invokes with the component context', async () => {
-    expect.assertions(2)
-    const spy = jest
-      .fn()
-      .mockImplementationOnce(function (this: any, to, from, next) {
-        expect(typeof this.counter).toBe('number')
-        next()
-      })
-    const WithLeave = defineComponent({
-      template: `text`,
-      // we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
-      data: () => ({ counter: 0 }),
-      setup() {
-        onBeforeRouteLeave(spy)
-      },
-    })
-
-    const router = createRouter({
-      history: createMemoryHistory(),
-      routes: [
-        { path: '/', component },
-        { path: '/leave', component: WithLeave as any },
-      ],
-    })
-    const app = createApp({
-      template: `
-      <router-view />
-      `,
-    })
-    app.use(router)
-    const rootEl = document.createElement('div')
-    document.body.appendChild(rootEl)
-    app.mount(rootEl)
-
-    await router.isReady()
-    await router.push('/leave')
-    await router.push('/')
-    expect(spy).toHaveBeenCalledTimes(1)
-  })
-
   it('removes guards when leaving the route', async () => {
-    expect.assertions(3)
-    const spy = jest
-      .fn()
-      .mockImplementation(function (this: any, to, from, next) {
-        expect(typeof this.counter).toBe('number')
-        next()
-      })
+    const spy = jest.fn()
     const WithLeave = defineComponent({
       template: `text`,
-      // we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
-      data: () => ({ counter: 0 }),
       setup() {
         onBeforeRouteLeave(spy)
       },

__tests__/guards/onBeforeRouteUpdate.spec.ts

@@ -13,58 +13,10 @@ const component = {
 }
 
 describe('onBeforeRouteUpdate', () => {
-  it('invokes with the component context', async () => {
-    expect.assertions(2)
-    const spy = jest
-      .fn()
-      .mockImplementationOnce(function (this: any, to, from, next) {
-        expect(typeof this.counter).toBe('number')
-        next()
-      })
-    const WithLeave = defineComponent({
-      template: `text`,
-      // we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
-      data: () => ({ counter: 0 }),
-      setup() {
-        onBeforeRouteUpdate(spy)
-      },
-    })
-
-    const router = createRouter({
-      history: createMemoryHistory(),
-      routes: [
-        { path: '/', component },
-        { path: '/foo', component: WithLeave as any },
-      ],
-    })
-    const app = createApp({
-      template: `
-      <router-view />
-      `,
-    })
-    app.use(router)
-    const rootEl = document.createElement('div')
-    document.body.appendChild(rootEl)
-    app.mount(rootEl)
-
-    await router.isReady()
-    await router.push('/foo')
-    await router.push('/foo?q')
-    expect(spy).toHaveBeenCalledTimes(1)
-  })
-
   it('removes update guards when leaving', async () => {
-    expect.assertions(3)
-    const spy = jest
-      .fn()
-      .mockImplementation(function (this: any, to, from, next) {
-        expect(typeof this.counter).toBe('number')
-        next()
-      })
+    const spy = jest.fn()
     const WithLeave = defineComponent({
       template: `text`,
-      // we use data to check if the context is the right one because saving `this` in a variable logs a few warnings
-      data: () => ({ counter: 0 }),
       setup() {
         onBeforeRouteUpdate(spy)
       },

e2e/multi-app/index.ts

@@ -1,4 +1,4 @@
-import { createRouter, createWebHistory, onBeforeRouteUpdate } from '../../src'
+import { createRouter, createWebHistory } from '../../src'
 import { RouteComponent } from '../../src/types'
 import { createApp, ref, watchEffect, App, inject } from 'vue'
 
@@ -26,15 +26,6 @@ const User: RouteComponent = {
   setup() {
     const id = inject('id')!
 
-    if (id !== 1)
-      onBeforeRouteUpdate(function (to, from, next) {
-        // @ts-ignore
-        console.log('update from ', id, this.id)
-        // @ts-ignore
-        // this.count++
-        next()
-      })
-
     return { id }
   },
 }

src/navigationGuards.ts

@@ -23,6 +23,15 @@ import { matchedRouteKey } from './injectionSymbols'
 import { RouteRecordNormalized } from './matcher/types'
 import { isESModule } from './utils'
 
+function registerGuard(list: NavigationGuard[], guard: NavigationGuard) {
+  onUnmounted(() => {
+    const index = list.indexOf(guard)
+    if (index > -1) list.splice(index, 1)
+  })
+
+  list.push(guard)
+}
+
 /**
  * Add a navigation guard that triggers whenever the current location is
  * left. Similarly to {@link beforeRouteLeave}, it has access to the
@@ -31,10 +40,8 @@ import { isESModule } from './utils'
  * @param leaveGuard - {@link NavigationGuard}
  */
 export function onBeforeRouteLeave(leaveGuard: NavigationGuard) {
-  const instance = getCurrentInstance()
-  if (!instance) {
-    __DEV__ &&
-      warn('onBeforeRouteLeave must be called at the top of a setup function')
+  if (__DEV__ && !getCurrentInstance()) {
+    warn('onBeforeRouteLeave must be called at the top of a setup function')
     return
   }
 
@@ -49,14 +56,7 @@ export function onBeforeRouteLeave(leaveGuard: NavigationGuard) {
     return
   }
 
-  // @ts-ignore do we even want to allow that? Passing the context in a composition api hook doesn't make sense
-  const fn = leaveGuard.bind(instance.proxy)
-  onUnmounted(() => {
-    const index = activeRecord.leaveGuards.indexOf(fn)
-    if (index > -1) activeRecord.leaveGuards.splice(index, 1)
-  })
-
-  activeRecord.leaveGuards.push(fn)
+  registerGuard(activeRecord.leaveGuards, leaveGuard)
 }
 
 /**
@@ -67,10 +67,8 @@ export function onBeforeRouteLeave(leaveGuard: NavigationGuard) {
  * @param updateGuard - {@link NavigationGuard}
  */
 export function onBeforeRouteUpdate(updateGuard: NavigationGuard) {
-  const instance = getCurrentInstance()
-  if (!instance) {
-    __DEV__ &&
-      warn('onBeforeRouteUpdate must be called at the top of a setup function')
+  if (__DEV__ && !getCurrentInstance()) {
+    warn('onBeforeRouteUpdate must be called at the top of a setup function')
     return
   }
 
@@ -85,14 +83,7 @@ export function onBeforeRouteUpdate(updateGuard: NavigationGuard) {
     return
   }
 
-  // @ts-ignore do we even want to allow that? Passing the context in a composition api hook doesn't make sense
-  const fn = updateGuard.bind(instance.proxy)
-  onUnmounted(() => {
-    const index = activeRecord.updateGuards.indexOf(fn)
-    if (index > -1) activeRecord.updateGuards.splice(index, 1)
-  })
-
-  activeRecord.updateGuards.push(fn)
+  registerGuard(activeRecord.updateGuards, updateGuard)
 }
 
 export function guardToPromiseFn(