]> git.ipfire.org Git - thirdparty/vuejs/router.git/commitdiff
fix(router-view): reuse saved instances in different records (#446)
authorEduardo San Martin Morote <posva@users.noreply.github.com>
Tue, 1 Sep 2020 15:15:46 +0000 (17:15 +0200)
committerGitHub <noreply@github.com>
Tue, 1 Sep 2020 15:15:46 +0000 (17:15 +0200)
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` (https://github.com/vuejs/vue-next/pull/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

__tests__/guards/onBeforeRouteLeave.spec.ts
__tests__/guards/onBeforeRouteUpdate.spec.ts
e2e/guards-instances/index.html [new file with mode: 0644]
e2e/guards-instances/index.ts [new file with mode: 0644]
e2e/multi-app/index.ts
e2e/specs/guards-instances.js [new file with mode: 0644]
playground/views/ComponentWithData.vue
src/RouterView.ts
src/navigationGuards.ts
src/router.ts

index 17165eb9909705e4ab678e06414ef75eaa6c054c..139cf5eaf0ee42d4fc6e932ddcdd30a6070989f8 100644 (file)
@@ -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)
       },
index ba6560dce9fb691707383e0b771ee76b6cccc07f..dab3d99a0db95a2b6b6128ad8106cc1cecb73621 100644 (file)
@@ -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
new file mode 100644 (file)
index 0000000..3c257ad
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..0905451
--- /dev/null
@@ -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')
index 65a2ea7499b438b5e620118c45450a05a0dd66ea..b52ba409ba61f16f7885ac563295b5a518bd45e6 100644 (file)
@@ -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 }
   },
 }
diff --git a/e2e/specs/guards-instances.js b/e2e/specs/guards-instances.js
new file mode 100644 (file)
index 0000000..ae3bccc
--- /dev/null
@@ -0,0 +1,232 @@
+const bsStatus = require('../browserstack-send-status')
+
+function testCase(browser) {
+  return browser
+    .click('li:nth-child(2) a')
+    .assert.containsText('#enterCbs', '1')
+    .assert.containsText('#update', '0')
+    .assert.containsText('#leave', '0')
+    .click('li:nth-child(3) a')
+    .assert.containsText('#enterCbs', '2')
+    .assert.containsText('#update', '0')
+    .assert.containsText('#leave', '1')
+    .click('li:nth-child(4) a')
+    .assert.containsText('#enterCbs', '2')
+    .assert.containsText('#update', '1')
+    .assert.containsText('#leave', '1')
+    .click('li:nth-child(5) a')
+    .assert.containsText('#enterCbs', '2')
+    .assert.containsText('#update', '2')
+    .assert.containsText('#leave', '1')
+    .click('li:nth-child(2) a')
+    .assert.containsText('#enterCbs', '3')
+    .assert.containsText('#update', '2')
+    .assert.containsText('#leave', '2')
+    .expect.element('#logs')
+    .text.to.equal(
+      [
+        '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')
+    )
+}
+
+module.exports = {
+  ...bsStatus(),
+
+  '@tags': [],
+
+  /** @type {import('nightwatch').NightwatchTest} */
+  'guards instances': function (browser) {
+    browser
+      .url('http://localhost:8080/guards-instances/')
+      .waitForElementPresent('#app > *', 1000)
+
+      .click('#test-normal')
+
+    testCase(browser)
+
+    browser
+      .click('li:nth-child(1) a')
+      // the enters are reset when leaving a reused component
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '1')
+
+    browser.end()
+  },
+
+  /** @type {import('nightwatch').NightwatchTest} */
+  'guards instances transition': function (browser) {
+    browser
+      .url('http://localhost:8080/guards-instances/')
+      .waitForElementPresent('#app > *', 1000)
+
+      .click('#test-transition')
+
+    testCase(browser)
+
+    browser.end()
+  },
+
+  /** @type {import('nightwatch').NightwatchTest} */
+  'guards instances keep alive': function (browser) {
+    browser
+      .url('http://localhost:8080/guards-instances/')
+      .waitForElementPresent('#app > *', 1000)
+
+      .click('#test-keepalive')
+
+    testCase(browser)
+
+    browser
+      .click('li:nth-child(1) a')
+      // keep alive keeps the correct instance
+      .click('li:nth-child(2) a')
+      .expect.element('#enterCbs')
+      .text.equals('4')
+    browser
+      .click('li:nth-child(1) a')
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '5')
+      .click('li:nth-child(3) a')
+      .assert.containsText('#enterCbs', '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')
+      .expect.element('#logs')
+      .text.to.equal(
+        [
+          'update /f/1 - /f/2',
+          'setup:update /f/1 - /f/2',
+          'leave /f/2 - /',
+          'setup:leave /f/2 - /',
+        ].join('\n')
+      )
+
+    browser.end()
+  },
+
+  /** @type {import('nightwatch').NightwatchTest} */
+  'guards instances keyed': function (browser) {
+    browser
+      .url('http://localhost:8080/guards-instances/')
+      .waitForElementPresent('#app > *', 1000)
+
+      .click('#test-keyed')
+
+    testCase(browser)
+
+    browser
+      .click('li:nth-child(5) a')
+      // the query is used as a key resetting the enter count
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '0')
+      // changing both the route and mounting the component
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '1')
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '1')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '1')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .click('li:nth-child(6) a')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .click('#resetLogs')
+      .click('li:nth-child(7) a')
+      .assert.containsText('#enterCbs', '0')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .expect.element('#logs')
+      .text.to.equal(
+        ['update /f/2 - /f/2', 'setup:update /f/2 - /f/2'].join('\n')
+      )
+    browser.click('li:nth-child(6) a').assert.containsText('#enterCbs', '0')
+
+    browser.end()
+  },
+
+  /** @type {import('nightwatch').NightwatchTest} */
+  'guards instances keepalive keyed': function (browser) {
+    browser
+      .url('http://localhost:8080/guards-instances/')
+      .waitForElementPresent('#app > *', 1000)
+
+      .click('#test-keepalivekeyed')
+
+    testCase(browser)
+
+    browser
+      .click('li:nth-child(1) a')
+      // keep alive keeps the correct instance
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '4')
+      .click('li:nth-child(1) a')
+      .click('li:nth-child(2) a')
+      .assert.containsText('#enterCbs', '5')
+      .click('li:nth-child(3) a')
+      .assert.containsText('#enterCbs', '6')
+
+      .click('li:nth-child(5) a')
+      // the query is used as a key resetting the enter count
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '0')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .click('li:nth-child(1) a')
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '1')
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '1')
+      .click('li:nth-child(5) a')
+      .assert.containsText('#enterCbs', '6')
+      // on reused instance
+      .click('li:nth-child(2) a')
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '2')
+      .assert.containsText('#update', '1')
+      .assert.containsText('#leave', '1')
+      .click('#resetLogs')
+      .click('li:nth-child(7) a')
+      .assert.containsText('#enterCbs', '0')
+      // the previous instance was updated but not this one
+      .assert.containsText('#update', '0')
+      .assert.containsText('#leave', '0')
+      .expect.element('#logs')
+      // should only trigger active guards
+      .text.to.equal(
+        ['update /f/2 - /f/2', 'setup:update /f/2 - /f/2'].join('\n')
+      )
+    browser
+      .click('li:nth-child(6) a')
+      .assert.containsText('#enterCbs', '2')
+      .assert.containsText('#update', '2')
+      .assert.containsText('#leave', '1')
+      .expect.element('#logs')
+      .text.to.equal(
+        [
+          'update /f/2 - /f/2',
+          'setup:update /f/2 - /f/2',
+          'update /f/2 - /f/2',
+          'setup:update /f/2 - /f/2',
+        ].join('\n')
+      )
+
+    browser.end()
+  },
+}
index 10a0b3991e1272da8ab3a78abfda078a4bf1bea0..5c20790859ae148a6b89202692064e87ec19f166 100644 (file)
@@ -14,13 +14,14 @@ export default defineComponent({
   name: 'ComponentWithData',
   async setup() {
     const data = reactive({ other: 'old', fromApi: null })
-    data.fromApi = await getData()
 
     onBeforeRouteUpdate(async (to, from, next) => {
       data.fromApi = await getData()
       next()
     })
 
+    data.fromApi = await getData()
+
     return {
       ...toRefs(data),
     }
index 5f5dba406c1831d8cc268d6938c6092ef8256a46..3909764076ca2c8119f26867fcc9f2b98bb32e9b 100644 (file)
@@ -11,8 +11,13 @@ import {
   computed,
   AllowedComponentProps,
   ComponentCustomProps,
+  watch,
 } from 'vue'
-import { RouteLocationNormalized, RouteLocationNormalizedLoaded } from './types'
+import {
+  RouteLocationNormalized,
+  RouteLocationNormalizedLoaded,
+  RouteLocationMatched,
+} from './types'
 import {
   matchedRouteKey,
   viewDepthKey,
@@ -20,6 +25,7 @@ import {
 } from './injectionSymbols'
 import { assign } from './utils'
 import { warn } from './warning'
+import { isSameRouteRecord } from './location'
 
 export interface RouterViewProps {
   name?: string
@@ -42,7 +48,7 @@ export const RouterViewImpl = defineComponent({
 
     const injectedRoute = inject(routeLocationKey)!
     const depth = inject(viewDepthKey, 0)
-    const matchedRouteRef = computed(
+    const matchedRouteRef = computed<RouteLocationMatched | undefined>(
       () => (props.route || injectedRoute).matched[depth]
     )
 
@@ -51,10 +57,46 @@ export const RouterViewImpl = defineComponent({
 
     const viewRef = ref<ComponentPublicInstance>()
 
+    // watch at the same time the component instance, the route record we are
+    // rendering, and the name
+    watch(
+      () => [viewRef.value, matchedRouteRef.value, props.name] as const,
+      ([instance, to, name], [oldInstance, from, oldName]) => {
+        // copy reused instances
+        if (to) {
+          // this will update the instance for new instances as well as reused
+          // instances when navigating to a new route
+          to.instances[name] = instance
+          // the component instance is reused for a different route or name so
+          // we copy any saved update or leave guards
+          if (from && instance === oldInstance) {
+            to.leaveGuards = from.leaveGuards
+            to.updateGuards = from.updateGuards
+          }
+        }
+
+        // trigger beforeRouteEnter next callbacks
+        if (
+          instance &&
+          to &&
+          // if there is no instance but to and from are the same this might be
+          // the first visit
+          (!from || !isSameRouteRecord(to, from) || !oldInstance)
+        ) {
+          ;(to.enterCallbacks[name] || []).forEach(callback =>
+            callback(instance)
+          )
+        }
+      }
+    )
+
     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
 
       if (!ViewComponent) {
         return slots.default
@@ -63,7 +105,7 @@ export const RouterViewImpl = defineComponent({
       }
 
       // props from route configuration
-      const routePropsOption = matchedRoute.props[props.name]
+      const routePropsOption = matchedRoute!.props[props.name]
       const routeProps = routePropsOption
         ? routePropsOption === true
           ? route.params
@@ -72,24 +114,16 @@ export const RouterViewImpl = defineComponent({
           : routePropsOption
         : null
 
-      // 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 onVnodeMounted = () => {
-        matchedRoute.instances[currentName] = viewRef.value
-        ;(matchedRoute.enterCallbacks[currentName] || []).forEach(callback =>
-          callback(viewRef.value!)
-        )
-      }
-      const onVnodeUnmounted = () => {
+      const onVnodeUnmounted: VNodeProps['onVnodeUnmounted'] = vnode => {
         // remove the instance reference to prevent leak
-        matchedRoute.instances[currentName] = null
+        if (vnode.component!.isUnmounted) {
+          matchedRoute!.instances[currentName] = null
+        }
       }
 
       const component = h(
         ViewComponent,
         assign({}, routeProps, attrs, {
-          onVnodeMounted,
           onVnodeUnmounted,
           ref: viewRef,
         })
index 4093e189cc76bea2eeebb2c5db046da94c06da4e..a69929fc0e6520003489d7e146dc5abdc3a8c25b 100644 (file)
@@ -17,12 +17,29 @@ import {
   NavigationFailure,
   NavigationRedirectError,
 } from './errors'
-import { ComponentOptions } from 'vue'
+import { ComponentOptions, onUnmounted, onActivated, onDeactivated } from 'vue'
 import { inject, getCurrentInstance, warn } from 'vue'
 import { matchedRouteKey } from './injectionSymbols'
 import { RouteRecordNormalized } from './matcher/types'
 import { isESModule } from './utils'
 
+function registerGuard(list: NavigationGuard[], guard: NavigationGuard) {
+  const removeFromList = () => {
+    const index = list.indexOf(guard)
+    if (index > -1) list.splice(index, 1)
+  }
+
+  onUnmounted(removeFromList)
+  onDeactivated(removeFromList)
+
+  onActivated(() => {
+    const index = list.indexOf(guard)
+    if (index < 0) list.push(guard)
+  })
+
+  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 +48,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,10 +64,7 @@ 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)
-  )
+  registerGuard(activeRecord.leaveGuards, leaveGuard)
 }
 
 /**
@@ -63,10 +75,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
   }
 
@@ -81,10 +91,7 @@ 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)
-  )
+  registerGuard(activeRecord.updateGuards, updateGuard)
 }
 
 export function guardToPromiseFn(
index b724217c31d96fe74faad01b53d12b53d11e8588..8575a12dacd45f8d97b1f85398d4e4ad7e853378 100644 (file)
@@ -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