refactor(router-view): fix instances without a key

Author: posva

e2e/guards-instances/index.ts

@@ -3,8 +3,10 @@ import {
   createWebHistory,
   onBeforeRouteUpdate,
   onBeforeRouteLeave,
+  useRoute,
+  useRouter,
 } from '../../src'
-import { createApp, ref, reactive, defineComponent } from 'vue'
+import { createApp, ref, reactive, defineComponent, computed } from 'vue'
 
 // override existing style on dev with shorter times
 if (!__CI__) {
@@ -42,9 +44,9 @@ const state = reactive({
 const Foo = defineComponent({
   template: '<div>foo {{ enterCallback }}</div>',
   data: () => ({ key: 'Foo', enterCallback: 0 }),
-  mounted() {
-    console.log('mounted Foo')
-  },
+  // mounted() {
+  //   console.log('mounted Foo')
+  // },
   beforeRouteEnter(to, from, next) {
     state.enter++
     logs.value.push(`enter ${from.path} - ${to.path}`)
@@ -91,6 +93,29 @@ const router = createRouter({
   ],
 })
 
+// preserve existing query
+const originalPush = router.push
+router.push = to => {
+  if (typeof to === 'string') {
+    const resolved = router.resolve(to)
+    return router.push({
+      path: to,
+      query: {
+        testCase: router.currentRoute.value.query.testCase,
+        ...resolved.query,
+      },
+    })
+  } else {
+    return originalPush({
+      ...to,
+      query: {
+        testCase: router.currentRoute.value.query.testCase,
+        ...to.query,
+      },
+    })
+  }
+}
+
 const app = createApp({
   template: `
     <div id="app">
@@ -136,9 +161,9 @@ leaves: {{ state.leave }}
         <router-view :key="$route.query.foo" class="view" />
       </template>
       <template v-else-if="testCase === 'keepalivekeyed'">
-        <router-view v-slot="{ Component, key }" >
+        <router-view v-slot="{ Component }" >
           <keep-alive>
-            <component :is="Component" :key="$route.query.foo || key" class="view" />
+            <component :is="Component" :key="$route.query.foo" class="view" />
           </keep-alive>
         </router-view>
       </template>
@@ -149,7 +174,18 @@ leaves: {{ state.leave }}
     </div>
   `,
   setup() {
-    const testCase = ref('')
+    const router = useRouter()
+    const route = useRoute()
+
+    const testCase = computed<string>({
+      get: () => {
+        let { testCase } = route.query
+        return !testCase || Array.isArray(testCase) ? '' : testCase
+      },
+      set(testCase) {
+        router.push({ query: { ...route.query, testCase } })
+      },
+    })
 
     return { state, logs, testCase }
   },

e2e/specs/guards-instances.js

@@ -5,15 +5,15 @@ function testCase(browser) {
     .click('li:nth-child(2) a')
     .assert.containsText('.view', 'foo 1')
     .click('li:nth-child(3) a')
-    .assert.containsText('.view', 'foo 1')
+    .assert.containsText('.view', 'foo 2')
     .click('li:nth-child(4) a')
-    .assert.containsText('.view', 'foo 1')
+    .assert.containsText('.view', 'foo 2')
     .click('li:nth-child(5) a')
-    .assert.containsText('.view', 'foo 1')
+    .assert.containsText('.view', 'foo 2')
     .click('li:nth-child(2) a')
-    .assert.containsText('.view', 'foo')
-    .assert.containsText(
-      '#logs',
+    .assert.containsText('.view', 'foo 3')
+    .expect.element('#logs')
+    .text.to.equal(
       [
         'enter / - /foo',
         'leave /foo - /f/1',
@@ -68,20 +68,22 @@ module.exports = {
       .click('li:nth-child(1) a')
       // keep alive keeps the correct instance
       .click('li:nth-child(2) a')
-      .assert.containsText('.view', 'foo 3')
+      .expect.element('.view')
+      .text.equals('foo 4')
+    browser
       .click('li:nth-child(1) a')
       .click('li:nth-child(2) a')
-      .assert.containsText('.view', 'foo 4')
+      .assert.containsText('.view', 'foo 5')
       .click('li:nth-child(3) a')
-      .assert.containsText('.view', 'foo 2')
+      .assert.containsText('.view', 'foo 6')
       // 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',
+      .expect.element('#logs')
+      .text.to.equal(
         [
           'update /f/1 - /f/2',
           'setup:update /f/1 - /f/2',
@@ -146,12 +148,12 @@ module.exports = {
       .click('li:nth-child(1) a')
       // keep alive keeps the correct instance
       .click('li:nth-child(2) a')
-      .assert.containsText('.view', 'foo 3')
+      .assert.containsText('.view', 'foo 4')
       .click('li:nth-child(1) a')
       .click('li:nth-child(2) a')
-      .assert.containsText('.view', 'foo 4')
+      .assert.containsText('.view', 'foo 5')
       .click('li:nth-child(3) a')
-      .assert.containsText('.view', 'foo 2')
+      .assert.containsText('.view', 'foo 6')
 
       .click('li:nth-child(5) a')
       // the query is used as a key resetting the enter count
@@ -161,6 +163,10 @@ module.exports = {
       .click('li:nth-child(6) a')
       .assert.containsText('.view', 'foo 1')
       .click('li:nth-child(5) a')
+      .assert.containsText('.view', 'foo 6')
+      // on reused instance
+      .click('li:nth-child(2) a')
+      .click('li:nth-child(6) a')
       .assert.containsText('.view', 'foo 2')
 
     browser.end()

src/RouterView.ts

@@ -11,6 +11,7 @@ import {
   computed,
   AllowedComponentProps,
   ComponentCustomProps,
+  watch,
 } from 'vue'
 import {
   RouteLocationNormalized,
@@ -24,6 +25,7 @@ import {
 } from './injectionSymbols'
 import { assign } from './utils'
 import { warn } from './warning'
+import { isSameRouteRecord } from './location'
 
 export interface RouterViewProps {
   name?: string
@@ -55,18 +57,42 @@ export const RouterViewImpl = defineComponent({
 
     const viewRef = ref<ComponentPublicInstance>()
 
+    watch(
+      () => [viewRef.value, matchedRouteRef.value, props.name] as const,
+      ([instance, to, name], [oldInstance, from, oldName]) => {
+        // copy reused instances
+        if (to) {
+          to.instances[name] = instance
+          // the component instance is reused for a different route or name
+          if (from && instance === oldInstance) {
+            to.leaveGuards = from.leaveGuards
+            to.updateGuards = from.updateGuards
+          }
+        }
+
+        if (
+          instance &&
+          to &&
+          (!from || !isSameRouteRecord(to, from) || !oldInstance)
+        ) {
+          ;(to.enterCallbacks[name] || []).forEach(callback =>
+            callback(viewRef.value!)
+          )
+        }
+      }
+    )
+
     return () => {
       const route = props.route || injectedRoute
       const matchedRoute = matchedRouteRef.value
       const ViewComponent = matchedRoute && matchedRoute.components[props.name]
       // we need the value at the time we render because when we unmount, we
       // navigated to a different location so the value is different
       const currentName = props.name
-      const key = matchedRoute && currentName + matchedRoute.path
 
       if (!ViewComponent) {
         return slots.default
-          ? slots.default({ Component: ViewComponent, route, key })
+          ? slots.default({ Component: ViewComponent, route })
           : null
       }
 
@@ -80,21 +106,14 @@ export const RouterViewImpl = defineComponent({
           : routePropsOption
         : null
 
-      const onVnodeMounted = () => {
-        matchedRoute!.instances[currentName] = viewRef.value
-        ;(matchedRoute!.enterCallbacks[currentName] || []).forEach(callback =>
-          callback(viewRef.value!)
-        )
-      }
       const onVnodeUnmounted = () => {
         // remove the instance reference to prevent leak
         matchedRoute!.instances[currentName] = null
       }
 
       const component = h(
         ViewComponent,
-        assign({ key }, routeProps, attrs, {
-          onVnodeMounted,
+        assign({}, routeProps, attrs, {
           onVnodeUnmounted,
           ref: viewRef,
         })
@@ -104,7 +123,7 @@ export const RouterViewImpl = defineComponent({
         // pass the vnode to the slot as a prop.
         // h and <component :is="..."> both accept vnodes
         slots.default
-          ? slots.default({ Component: component, route, key })
+          ? slots.default({ Component: component, route })
           : component
       )
     }