fix(b-link): href handling with live router (closes #5927) (#5934)

jacobmllr95 · web-flow · commit 8a367b6296b0 · 2020-10-21T20:22:47.000+02:00
* fix(b-link): `href` handling with live router

* Update router.spec.js
diff --git a/src/components/link/link.js b/src/components/link/link.js
@@ -126,7 +126,7 @@ export const BLink = /*#__PURE__*/ Vue.extend({
     computedHref() {
       // We don't pass `this` as the first arg as we need reactivity of the props
       const { to, href } = this
-      return computeHref({ to, href })
+      return computeHref({ to, href }, this.computedTag)
     },
     computedProps() {
       const { prefetch } = this
diff --git a/src/components/pagination-nav/pagination-nav.js b/src/components/pagination-nav/pagination-nav.js
@@ -213,7 +213,7 @@ export const BPaginationNav = /*#__PURE__*/ Vue.extend({
       try {
         // Convert the `to` to a HREF via a temporary `a` tag
         link = document.createElement('a')
-        link.href = computeHref({ to }, '/', '/')
+        link.href = computeHref({ to }, 'a', '/', '/')
         // We need to add the anchor to the document to make sure the
         // `pathname` is correctly detected in any browser (i.e. IE)
         document.body.appendChild(link)
diff --git a/src/utils/router.js b/src/utils/router.js
@@ -4,6 +4,8 @@ import { isArray, isNull, isPlainObject, isString, isUndefined } from './inspect
 import { keys } from './object'
 import { toString } from './string'
 
+const ANCHOR_TAG = 'a'
+
 // Method to replace reserved chars
 const encodeReserveReplacer = c => '%' + c.charCodeAt(0).toString(16)
 
@@ -88,7 +90,7 @@ export const isRouterLink = tag => !!(tag && !isTag(tag, 'a'))
 export const computeTag = ({ to, disabled, routerComponentName } = {}, thisOrParent) => {
   const hasRouter = !!thisOrParent.$router
   if (!hasRouter || (hasRouter && (disabled || !to))) {
-    return 'a'
+    return ANCHOR_TAG
   }
 
   // TODO:
@@ -105,12 +107,24 @@ export const computeTag = ({ to, disabled, routerComponentName } = {}, thisOrPar
 export const computeRel = ({ target, rel } = {}) =>
   target === '_blank' && isNull(rel) ? 'noopener' : rel || null
 
-export const computeHref = ({ href, to } = {}, fallback = '#', toFallback = '/') => {
+export const computeHref = (
+  { href, to } = {},
+  tag = ANCHOR_TAG,
+  fallback = '#',
+  toFallback = '/'
+) => {
   // Return `href` when explicitly provided
   if (href) {
     return href
   }
 
+  // We've checked for `$router` in `computeTag()`, so `isRouterLink()` indicates a live router
+  // When deferring to Vue Router's `<router-link>`, don't use the `href` attribute at all
+  // We return `null`, and then remove `href` from the attributes passed to `<router-link>`
+  if (isRouterLink(tag)) {
+    return null
+  }
+
   // Fallback to `to` prop (if `to` is a string)
   if (isString(to)) {
     return to || toFallback
diff --git a/src/utils/router.spec.js b/src/utils/router.spec.js
@@ -110,8 +110,14 @@ describe('utils/router', () => {
 
     it('parses nothing to default', async () => {
       expect(computeHref()).toEqual('#')
-      expect(computeHref(undefined, '/', '')).toEqual('/')
-      expect(computeHref(undefined, '', '')).toEqual('')
+      expect(computeHref(undefined, undefined, '/', '')).toEqual('/')
+      expect(computeHref(undefined, undefined, '', '')).toEqual('')
+    })
+
+    it('returns null when tag is not `a`', async () => {
+      expect(computeHref({}, 'div')).toEqual(null)
+      expect(computeHref(undefined, 'div', '/', '')).toEqual(null)
+      expect(computeHref(undefined, 'span', '', '/')).toEqual(null)
     })
 
     it('returns href when both href and to provided', async () => {
@@ -124,8 +130,8 @@ describe('utils/router', () => {
 
     it('parses empty `href` to default', async () => {
       expect(computeHref({ href: '' })).toEqual('#')
-      expect(computeHref({ href: '' }, '/', '')).toEqual('/')
-      expect(computeHref({ href: '' }, '', '')).toEqual('')
+      expect(computeHref({ href: '' }, 'a', '/', '')).toEqual('/')
+      expect(computeHref({ href: '' }, 'a', '', '')).toEqual('')
     })
 
     it('parses `to` when string', async () => {
@@ -173,8 +179,8 @@ describe('utils/router', () => {
 
     it('parses empty `to` to fallback default', async () => {
       expect(computeHref({ to: {} })).toEqual('#')
-      expect(computeHref({ to: {} }, '#', '')).toEqual('#')
-      expect(computeHref({ to: {} }, '/', '#')).toEqual('/')
+      expect(computeHref({ to: {} }, 'a', '#', '')).toEqual('#')
+      expect(computeHref({ to: {} }, 'a', '/', '#')).toEqual('/')
     })
 
     it('parses complete `to`', async () => {
