fix(router-view): reuse saved instances in different records (#446)

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
vuejs · Sep 1, 2020 · 6554171 · 6554171
1 parent 6d6d454
commit 6554171
Show file tree

Hide file tree

Showing 10 changed files with 543 additions and 151 deletions.
diff --git a/__tests__/guards/onBeforeRouteLeave.spec.ts b/__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)
       },

diff --git a/__tests__/guards/onBeforeRouteUpdate.spec.ts b/__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)
       },

diff --git a/e2e/guards-instances/index.html b/e2e/guards-instances/index.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <meta charset="UTF-8" />
+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <meta http-equiv="X-UA-Compatible" content="ie=edge" />
+    <title>Vue Router e2e tests - instances in guards</title>
+    <!-- TODO: replace with local imports for promises and anything else needed -->
+    <script src="https://polyfill.io/v3/polyfill.min.js?features=default%2Ces2015"></script>
+    <style>
+      .fade-enter-active,
+      .fade-leave-active {
+        transition: opacity 2s ease;
+      }
+      .fade-enter-from,
+      .fade-leave-active {
+        opacity: 0;
+      }
+    </style>
+  </head>
+  <body>
+    <a href="/">&lt;&lt; Back to Homepage</a>
+    <hr />
+
+    <main id="app"></main>
+  </body>
+</html>
diff --git a/e2e/guards-instances/index.ts b/e2e/guards-instances/index.ts
@@ -0,0 +1,206 @@
+import {
+  createRouter,
+  createWebHistory,
+  onBeforeRouteUpdate,
+  onBeforeRouteLeave,
+  useRoute,
+  useRouter,
+} from '../../src'
+import { createApp, ref, reactive, defineComponent, computed } from 'vue'
+
+// override existing style on dev with shorter times
+if (!__CI__) {
+  const transitionDuration = '0.5s'
+  const styleEl = document.createElement('style')
+  styleEl.innerHTML = `
+.fade-enter-active,
+.fade-leave-active {
+  transition: opacity ${transitionDuration} ease;
+}
+.child-view {
+  position: absolute;
+  transition: all ${transitionDuration} cubic-bezier(0.55, 0, 0.1, 1);
+}
+`
+  document.head.append(styleEl)
+}
+
+const Home = defineComponent({
+  template: `
+    <div>
+      <h2>Home</h2>
+    </div>
+  `,
+})
+
+const logs = ref<string[]>([])
+
+const state = reactive({
+  enter: 0,
+  update: 0,
+  leave: 0,
+})
+
+const Foo = defineComponent({
+  template: `
+    <div>
+    foo
+    <p id="enterCbs">{{ enterCallback }}</p>
+    <p id="update">{{ selfUpdates }}</p>
+    <p id="leave">{{ selfLeaves }}</p>
+    </div>
+  `,
+  data: () => ({ key: 'Foo', enterCallback: 0, selfUpdates: 0, selfLeaves: 0 }),
+  // mounted() {
+  //   console.log('mounted Foo')
+  // },
+  beforeRouteEnter(to, from, next) {
+    state.enter++
+    logs.value.push(`enter ${from.path} - ${to.path}`)
+    next(vm => {
+      // @ts-ignore
+      vm.enterCallback++
+    })
+  },
+  beforeRouteUpdate(to, from) {
+    if (!this || this.key !== 'Foo') throw new Error('no this')
+    state.update++
+    this.selfUpdates++
+    logs.value.push(`update ${from.path} - ${to.path}`)
+  },
+  beforeRouteLeave(to, from) {
+    if (!this || this.key !== 'Foo') throw new Error('no this')
+    state.leave++
+    this.selfLeaves++
+    logs.value.push(`leave ${from.path} - ${to.path}`)
+  },
+
+  setup() {
+    onBeforeRouteUpdate((to, from) => {
+      logs.value.push(`setup:update ${from.path} - ${to.path}`)
+    })
+    onBeforeRouteLeave((to, from) => {
+      logs.value.push(`setup:leave ${from.path} - ${to.path}`)
+    })
+    return {}
+  },
+})
+
+const webHistory = createWebHistory('/' + __dirname)
+const router = createRouter({
+  history: webHistory,
+  routes: [
+    { path: '/', component: Home },
+    {
+      path: '/foo',
+      component: Foo,
+    },
+    {
+      path: '/f/:id',
+      component: Foo,
+    },
+  ],
+})
+
+// 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">
+      <h1>Instances</h1>
+      <p>Using {{ testCase || 'default' }}</p>
+      <button id="test-normal" @click="testCase = ''">Use Normal</button>
+      <button id="test-keepalive" @click="testCase = 'keepalive'">Use Keep Alive</button>
+      <button id="test-transition" @click="testCase = 'transition'">Use Transition</button>
+      <button id="test-keyed" @click="testCase = 'keyed'">Use keyed</button>
+      <button id="test-keepalivekeyed" @click="testCase = 'keepalivekeyed'">Use Keep Alive Keyed</button>
+      <pre>
+route: {{ $route.fullPath }}
+enters: {{ state.enter }}
+updates: {{ state.update }}
+leaves: {{ state.leave }}
+      </pre>
+      <pre id="logs">{{ logs.join('\\n') }}</pre>
+      <button id="resetLogs" @click="logs = []">Reset Logs</button>
+      <ul>
+        <li><router-link to="/">/</router-link></li>
+        <li><router-link to="/foo">/foo</router-link></li>
+        <li><router-link to="/f/1">/f/1</router-link></li>
+        <li><router-link to="/f/2">/f/2</router-link></li>
+        <li><router-link to="/f/2?bar=foo">/f/2?bar=foo</router-link></li>
+        <li><router-link to="/f/2?foo=key">/f/2?foo=key</router-link></li>
+        <li><router-link to="/f/2?foo=key2">/f/2?foo=key2</router-link></li>
+      </ul>
+
+      <template v-if="testCase === 'keepalive'">
+        <router-view v-slot="{ Component }" >
+          <keep-alive>
+            <component :is="Component" class="view" />
+          </keep-alive>
+        </router-view>
+      </template>
+      <template v-else-if="testCase === 'transition'">
+        <router-view v-slot="{ Component }" >
+          <transition name="fade" mode="">
+            <component :is="Component" class="view" />
+          </transition>
+        </router-view>
+      </template>
+      <template v-else-if="testCase === 'keyed'">
+        <router-view :key="$route.query.foo" class="view" />
+      </template>
+      <template v-else-if="testCase === 'keepalivekeyed'">
+        <router-view v-slot="{ Component }" >
+          <keep-alive>
+            <component :is="Component" :key="$route.query.foo" class="view" />
+          </keep-alive>
+        </router-view>
+      </template>
+      <template v-else>
+        <router-view class="view" />
+      </template>
+
+    </div>
+  `,
+  setup() {
+    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 }
+  },
+})
+
+app.use(router)
+
+app.mount('#app')
diff --git a/e2e/multi-app/index.ts b/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 }
   },
 }