fix: afterNavigation logic (its never set in options)

Author: pimlie

src/shared/mixin.js

@@ -1,7 +1,8 @@
 import triggerUpdate from '../client/triggerUpdate'
-import hasMetaInfo from './hasMetaInfo'
+import { hasMetaInfo } from './meta-helpers'
 import { isUndefined, isFunction } from './is-type'
 import { ensuredPush } from './ensure'
+import { addNavGuards } from './nav-guards'
 
 export default function createMixin(Vue, options) {
   // for which Vue lifecycle hooks should the metaInfo be refreshed
@@ -14,7 +15,7 @@ export default function createMixin(Vue, options) {
         get() {
           // Show deprecation warning once when devtools enabled
           if (Vue.config.devtools && !this.$root._vueMeta.hasMetaInfoDeprecationWarningShown) {
-            console.warn('VueMeta DeprecationWarning: _hasMetaInfo has been deprecated and will be removed in a future version. Please import hasMetaInfo and use hasMetaInfo(vm) instead') // eslint-disable-line no-console
+            console.warn('VueMeta DeprecationWarning: _hasMetaInfo has been deprecated and will be removed in a future version. Please use hasMetaInfo(vm) instead') // eslint-disable-line no-console
             this.$root._vueMeta.hasMetaInfoDeprecationWarningShown = true
           }
           return hasMetaInfo(this)
@@ -71,39 +72,32 @@ export default function createMixin(Vue, options) {
           this.$root._vueMeta.initialized = this.$isServer
 
           if (!this.$root._vueMeta.initialized) {
-            const $rootMeta = this.$root.$meta()
-
             ensuredPush(this.$options, 'mounted', () => {
               if (!this.$root._vueMeta.initialized) {
                 // refresh meta in nextTick so all child components have loaded
                 this.$nextTick(function () {
-                  $rootMeta.refresh()
+                  this.$root.$meta().refresh()
                   this.$root._vueMeta.initialized = true
                 })
               }
             })
 
-            // add vue-router navigation guard to prevent multiple updates during navigation
-            // only usefull on the client side
-            if (options.refreshOnceOnNavigation && this.$root.$router) {
-              const $router = this.$root.$router
-              $router.beforeEach((to, from, next) => {
-                $rootMeta.pause()
-                next()
-              })
-
-              $router.afterEach(() => {
-                const { vm, metaInfo } = $rootMeta.resume()
-                if (metaInfo && metaInfo.afterNavigation && isFunction(metaInfo.afterNavigation)) {
-                  metaInfo.afterNavigation.call(vm, metaInfo)
-                }
-              })
+            // add the navigation guards if they havent been added yet
+            if (options.refreshOnceOnNavigation) {
+              addNavGuards(this)
             }
           }
         }
 
         // do not trigger refresh on the server side
         if (!this.$isServer) {
+          // add the navigation guards if they havent been added yet
+          // if metaInfo is defined as a function, this does call the computed fn redundantly
+          // but as Vue internally caches the results of computed props it shouldnt hurt performance
+          if (this.$options[options.keyName].afterNavigation) {
+            addNavGuards(this)
+          }
+
           // no need to add this hooks on server side
           updateOnLifecycleHook.forEach((lifecycleHook) => {
             ensuredPush(this.$options, lifecycleHook, () => triggerUpdate(this, lifecycleHook))

src/shared/nav-guards.js

@@ -0,0 +1,25 @@
+import { isFunction } from './is-type'
+
+export function addNavGuards(vm) {
+  // return when nav guards already added or no router exists
+  if (vm.$root._vueMeta.navGuards || !vm.$root.$router) {
+    return
+  }
+
+  vm.$root._vueMeta.navGuards = true
+
+  const $router = vm.$root.$router
+  const $meta = vm.$root.$meta()
+
+  $router.beforeEach((to, from, next) => {
+    $meta.pause()
+    next()
+  })
+
+  $router.afterEach(() => {
+    const { metaInfo } = $meta.resume()
+    if (metaInfo && metaInfo.afterNavigation && isFunction(metaInfo.afterNavigation)) {
+      metaInfo.afterNavigation(metaInfo)
+    }
+  })
+}

src/shared/options.js

@@ -1,4 +1,4 @@
-import { isObject, isFunction } from './is-type'
+import { isObject } from './is-type'
 import { defaultOptions } from './constants'
 
 export default function setOptions(options) {
@@ -11,15 +11,5 @@ export default function setOptions(options) {
     }
   }
 
-  if (options.afterNavigation && !isFunction(options.afterNavigation)) {
-    console.warn(`afterNavigation should be a function, received ${typeof options.afterNavigation} instead`) // eslint-disable-line no-console
-    options.afterNavigation = void 0
-    return options
-  }
-
-  if (options.afterNavigation && !options.refreshOnceOnNavigation) {
-    options.refreshOnceOnNavigation = true
-  }
-
   return options
 }

test/unit/shared.test.js

@@ -6,8 +6,6 @@ import setOptions from '../../src/shared/options'
 import { hasGlobalWindowFn } from '../../src/shared/window'
 import { defaultOptions } from '../../src/shared/constants'
 
-const noop = () => {}
-
 describe('shared', () => {
   test('ensureIsArray ensures var is array', () => {
     let a = { p: 1 }
@@ -38,27 +36,4 @@ describe('shared', () => {
     expect(options.contentKeyName).toBeDefined()
     expect(options.contentKeyName).toBe(defaultOptions.contentKeyName)
   })
-
-  test('setOptions warns when afterNavigation not fn', () => {
-    const warn = jest.spyOn(console, 'warn').mockImplementation(noop)
-
-    let options = { afterNavigation: true }
-    options = setOptions(options)
-
-    expect(warn).toHaveBeenCalledTimes(1)
-    expect(options.afterNavigation).toBeUndefined()
-
-    warn.mockRestore()
-  })
-  test('setOptions sets refreshOnceOnNavigation when afterNavigation defined', () => {
-    const warn = jest.spyOn(console, 'warn').mockImplementation(noop)
-
-    let options = { afterNavigation: noop }
-    options = setOptions(options)
-
-    expect(warn).not.toHaveBeenCalled()
-    expect(options.refreshOnceOnNavigation).toBe(true)
-
-    warn.mockRestore()
-  })
 })