refactor: remove unnecessary frees

Author: posva

e2e/guards-instances/index.ts

@@ -66,10 +66,10 @@ const Foo = defineComponent({
 
   setup() {
     onBeforeRouteUpdate((to, from) => {
-      console.log(`setup update ${from.path} - ${to.path}`)
+      logs.value.push(`setup:update ${from.path} - ${to.path}`)
     })
     onBeforeRouteLeave((to, from) => {
-      console.log(`setup leave ${from.path} - ${to.path}`)
+      logs.value.push(`setup:leave ${from.path} - ${to.path}`)
     })
     return {}
   },

e2e/specs/guards-instances.js

@@ -17,10 +17,14 @@ function testCase(browser) {
       [
         'enter / - /foo',
         'leave /foo - /f/1',
+        'setup:leave /foo - /f/1',
         'enter /foo - /f/1',
         'update /f/1 - /f/2',
+        'setup:update /f/1 - /f/2',
         'update /f/2 - /f/2',
+        'setup:update /f/2 - /f/2',
         'leave /f/2 - /foo',
+        'setup:leave /f/2 - /foo',
         'enter /f/2 - /foo',
       ].join('\n')
     )
@@ -70,6 +74,21 @@ module.exports = {
       .assert.containsText('.view', 'foo 4')
       .click('li:nth-child(3) a')
       .assert.containsText('.view', 'foo 2')
+      // leave the update view and enter it again
+      .click('li:nth-child(1) a')
+      .click('li:nth-child(3) a')
+      .click('#resetLogs')
+      .click('li:nth-child(4) a')
+      .click('li:nth-child(1) a')
+      .assert.containsText(
+        '#logs',
+        [
+          'update /f/1 - /f/2',
+          'setup:update /f/1 - /f/2',
+          'leave /f/2 - /',
+          'setup:leave /f/2 - /',
+        ].join('\n')
+      )
 
     browser.end()
   },

src/RouterView.ts

@@ -55,25 +55,6 @@ export const RouterViewImpl = defineComponent({
 
     const viewRef = ref<ComponentPublicInstance>()
 
-    // when the same component is used in different routes, the onVnodeMounted
-    // hook doesn't trigger, so we need to observe the changing route to update
-    // the instance on the record
-    // watch(matchedRouteRef, to => {
-    //   const currentName = props.name
-    //   // to can be null if there isn't a matched route, e.g. not found
-    //   if (to && !to.instances[currentName]) {
-    //     to.instances[currentName] = viewRef.value
-    //     // trigger enter callbacks when different routes only
-    //     if (viewRef.value) {
-    //       ;(to.enterCallbacks[currentName] || []).forEach(callback =>
-    //         callback(viewRef.value!)
-    //       )
-    //       // avoid double calls since watch is called before the onVnodeMounted
-    //       to.enterCallbacks[currentName] = []
-    //     }
-    //   }
-    // })
-
     return () => {
       const route = props.route || injectedRoute
       const matchedRoute = matchedRouteRef.value

src/navigationGuards.ts

@@ -17,7 +17,7 @@ import {
   NavigationFailure,
   NavigationRedirectError,
 } from './errors'
-import { ComponentOptions } from 'vue'
+import { ComponentOptions, onUnmounted } from 'vue'
 import { inject, getCurrentInstance, warn } from 'vue'
 import { matchedRouteKey } from './injectionSymbols'
 import { RouteRecordNormalized } from './matcher/types'
@@ -49,10 +49,14 @@ export function onBeforeRouteLeave(leaveGuard: NavigationGuard) {
     return
   }
 
-  activeRecord.leaveGuards.push(
-    // @ts-ignore do we even want to allow that? Passing the context in a composition api hook doesn't make sense
-    leaveGuard.bind(instance.proxy)
-  )
+  // @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)
 }
 
 /**
@@ -81,10 +85,14 @@ export function onBeforeRouteUpdate(updateGuard: NavigationGuard) {
     return
   }
 
-  activeRecord.updateGuards.push(
-    // @ts-ignore do we even want to allow that? Passing the context in a composition api hook doesn't make sense
-    updateGuard.bind(instance.proxy)
-  )
+  // @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)
 }
 
 export function guardToPromiseFn(

src/router.ts

@@ -709,16 +709,6 @@ export function createRouter(options: RouterOptions): Router {
     const error = checkCanceledNavigation(toLocation, from)
     if (error) return error
 
-    const [leavingRecords] = extractChangingRecords(toLocation, from)
-    for (const record of leavingRecords) {
-      // remove registered guards from removed matched records
-      record.leaveGuards = []
-      record.updateGuards = []
-      // free the references
-      record.instances = {}
-      record.enterCallbacks = {}
-    }
-
     // only consider as push if it's not the first navigation
     const isFirstNavigation = from === START_LOCATION_NORMALIZED
     const state = !isBrowser ? {} : history.state