fix(b-link): href handling inconsistencies to <router-link> (closes

#5820) (#5876)

* fix(b-link): `href` handling inconsistencies to `<router-link>`

* Update regex.js

* Update link.js

* Update link.js

src/components/link/link.js

@@ -2,7 +2,7 @@ import { NAME_LINK } from '../../constants/components'
 import Vue from '../../utils/vue'
 import { concat } from '../../utils/array'
 import { getComponentConfig } from '../../utils/config'
-import { attemptBlur, attemptFocus } from '../../utils/dom'
+import { attemptBlur, attemptFocus, isTag } from '../../utils/dom'
 import { stopEvent } from '../../utils/events'
 import { isBoolean, isEvent, isFunction, isUndefined } from '../../utils/inspect'
 import { pluckProps } from '../../utils/props'
@@ -120,14 +120,16 @@ export const BLink = /*#__PURE__*/ Vue.extend({
     },
     computedRel() {
       // We don't pass `this` as the first arg as we need reactivity of the props
-      return computeRel({ target: this.target, rel: this.rel })
+      const { target, rel } = this
+      return computeRel({ target, rel })
     },
     computedHref() {
       // We don't pass `this` as the first arg as we need reactivity of the props
-      return computeHref({ to: this.to, href: this.href }, this.computedTag)
+      const { to, href } = this
+      return computeHref({ to, href })
     },
     computedProps() {
-      const prefetch = this.prefetch
+      const { prefetch } = this
       return this.isRouterLink
         ? {
             ...pluckProps({ ...routerLinkProps, ...nuxtLinkProps }, this),
@@ -153,10 +155,10 @@ export const BLink = /*#__PURE__*/ Vue.extend({
         ...bvAttrs,
         // If `href` attribute exists on `<router-link>` (even `undefined` or `null`)
         // it fails working on SSR, so we explicitly add it here if needed
-        // (i.e. if `computedHref()` is truthy)
+        // (i.e. if `computedHref` is truthy)
         ...(href ? { href } : {}),
         // We don't render `rel` or `target` on non link tags when using `vue-router`
-        ...(isRouterLink && routerTag !== 'a' && routerTag !== 'area' ? {} : { rel, target }),
+        ...(isRouterLink && !isTag(routerTag, 'a') ? {} : { rel, target }),
         tabindex: disabled ? '-1' : isUndefined(bvAttrs.tabindex) ? null : bvAttrs.tabindex,
         'aria-disabled': disabled ? 'true' : null
       }

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 }, 'a', '/', '/')
+        link.href = computeHref({ to }, '/', '/')
         // 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)

src/constants/regex.js

@@ -1,15 +1,4 @@
-// Loose YYYY-MM-DD matching, ignores any appended time inforation
-// Matches '1999-12-20', '1999-1-1', '1999-01-20T22:51:49.118Z', '1999-01-02 13:00:00'
-export const RX_DATE = /^\d+-\d\d?-\d\d?(?:\s|T|$)/
-
-// Used to split off the date parts of the YYYY-MM-DD string
-export const RX_DATE_SPLIT = /-|\s|T/
-
-// Time string RegEx (optional seconds)
-export const RX_TIME = /^([0-1]?[0-9]|2[0-3]):[0-5]?[0-9](:[0-5]?[0-9])?$/
-
-// HREFs must end with a hash followed by at least one non-hash character
-export const RX_HREF = /^.*(#[^#]+)$/
+// --- General ---
 
 export const RX_ARRAY_NOTATION = /\[(\d+)]/g
 export const RX_DIGITS = /^\d+$/
@@ -20,6 +9,7 @@ export const RX_HTML_TAGS = /(<([^>]+)>)/gi
 export const RX_HYPHENATE = /\B([A-Z])/g
 export const RX_LOWER_UPPER = /([a-z])([A-Z])/g
 export const RX_NUMBER = /^[0-9]*\.?[0-9]+$/
+export const RX_PLUS = /\+/g
 export const RX_REGEXP_REPLACE = /[-/\\^$*+?.()|[\]{}]/g
 export const RX_SPACES = /[\s\uFEFF\xA0]+/g
 export const RX_SPACE_SPLIT = /\s+/
@@ -30,15 +20,40 @@ export const RX_TRIM_RIGHT = /\s+$/
 export const RX_UNDERSCORE = /_/g
 export const RX_UN_KEBAB = /-(\w)/g
 
-// Aspect
+// --- Date ---
+
+// Loose YYYY-MM-DD matching, ignores any appended time inforation
+// Matches '1999-12-20', '1999-1-1', '1999-01-20T22:51:49.118Z', '1999-01-02 13:00:00'
+export const RX_DATE = /^\d+-\d\d?-\d\d?(?:\s|T|$)/
+
+// Used to split off the date parts of the YYYY-MM-DD string
+export const RX_DATE_SPLIT = /-|\s|T/
+
+// Time string RegEx (optional seconds)
+export const RX_TIME = /^([0-1]?[0-9]|2[0-3]):[0-5]?[0-9](:[0-5]?[0-9])?$/
+
+// --- URL ---
+
+// HREFs must end with a hash followed by at least one non-hash character
+export const RX_HREF = /^.*(#[^#]+)$/
+
+export const RX_ENCODED_COMMA = /%2C/g
+export const RX_ENCODE_REVERSE = /[!'()*]/g
+export const RX_QUERY_START = /^(\?|#|&)/
+
+// --- Aspect ---
+
 export const RX_ASPECT = /^\d+(\.\d*)?[/:]\d+(\.\d*)?$/
 export const RX_ASPECT_SEPARATOR = /[/:]/
 
-// Grid
+// --- Grid ---
+
 export const RX_COL_CLASS = /^col-/
 
-// Icon
+// --- Icon ---
+
 export const RX_ICON_PREFIX = /^BIcon/
 
-// Locale
+// --- Locale ---
+
 export const RX_STRIP_LOCALE_MODS = /-u-.+/

src/utils/router.js

@@ -1,16 +1,9 @@
+import { RX_ENCODED_COMMA, RX_ENCODE_REVERSE, RX_PLUS, RX_QUERY_START } from '../constants/regex'
 import { isTag } from './dom'
 import { isArray, isNull, isPlainObject, isString, isUndefined } from './inspect'
 import { keys } from './object'
 import { toString } from './string'
 
-const ANCHOR_TAG = 'a'
-
-// Precompile RegExp
-const commaRE = /%2C/g
-const encodeReserveRE = /[!'()*]/g
-const plusRE = /\+/g
-const queryStartRE = /^(\?|#|&)/
-
 // Method to replace reserved chars
 const encodeReserveReplacer = c => '%' + c.charCodeAt(0).toString(16)
 
@@ -19,8 +12,8 @@ const encodeReserveReplacer = c => '%' + c.charCodeAt(0).toString(16)
 // - preserve commas
 const encode = str =>
   encodeURIComponent(toString(str))
-    .replace(encodeReserveRE, encodeReserveReplacer)
-    .replace(commaRE, ',')
+    .replace(RX_ENCODE_REVERSE, encodeReserveReplacer)
+    .replace(RX_ENCODED_COMMA, ',')
 
 const decode = decodeURIComponent
 
@@ -65,14 +58,14 @@ export const parseQuery = query => {
   const parsed = {}
   query = toString(query)
     .trim()
-    .replace(queryStartRE, '')
+    .replace(RX_QUERY_START, '')
 
   if (!query) {
     return parsed
   }
 
   query.split('&').forEach(param => {
-    const parts = param.replace(plusRE, ' ').split('=')
+    const parts = param.replace(RX_PLUS, ' ').split('=')
     const key = decode(parts.shift())
     const val = parts.length > 0 ? decode(parts.join('=')) : null
 
@@ -90,12 +83,12 @@ export const parseQuery = query => {
 
 export const isLink = props => !!(props.href || props.to)
 
-export const isRouterLink = tag => !isTag(tag, ANCHOR_TAG)
+export const isRouterLink = tag => !!(tag && !isTag(tag, 'a'))
 
 export const computeTag = ({ to, disabled, routerComponentName } = {}, thisOrParent) => {
-  const hasRouter = thisOrParent.$router
-  if (!hasRouter || (hasRouter && disabled) || (hasRouter && !to)) {
-    return ANCHOR_TAG
+  const hasRouter = !!thisOrParent.$router
+  if (!hasRouter || (hasRouter && (disabled || !to))) {
+    return 'a'
   }
 
   // TODO:
@@ -109,45 +102,26 @@ export const computeTag = ({ to, disabled, routerComponentName } = {}, thisOrPar
   return routerComponentName || (thisOrParent.$nuxt ? 'nuxt-link' : 'router-link')
 }
 
-export const computeRel = ({ target, rel } = {}) => {
-  if (target === '_blank' && isNull(rel)) {
-    return 'noopener'
-  }
-  return rel || null
-}
-
-export const computeHref = (
-  { href, to } = {},
-  tag = ANCHOR_TAG,
-  fallback = '#',
-  toFallback = '/'
-) => {
-  // We've already checked the $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
-  }
+export const computeRel = ({ target, rel } = {}) =>
+  target === '_blank' && isNull(rel) ? 'noopener' : rel || null
 
+export const computeHref = ({ href, to } = {}, fallback = '#', toFallback = '/') => {
   // Return `href` when explicitly provided
   if (href) {
     return href
   }
 
-  // Reconstruct `href` when `to` used, but no router
-  if (to) {
-    // Fallback to `to` prop (if `to` is a string)
-    if (isString(to)) {
-      return to || toFallback
-    }
-    // Fallback to `to.path + to.query + to.hash` prop (if `to` is an object)
-    if (isPlainObject(to) && (to.path || to.query || to.hash)) {
-      const path = toString(to.path)
-      const query = stringifyQueryObj(to.query)
-      let hash = toString(to.hash)
-      hash = !hash || hash.charAt(0) === '#' ? hash : `#${hash}`
-      return `${path}${query}${hash}` || toFallback
-    }
+  // Fallback to `to` prop (if `to` is a string)
+  if (isString(to)) {
+    return to || toFallback
+  }
+  // Fallback to `to.path' + `to.query` + `to.hash` prop (if `to` is an object)
+  if (isPlainObject(to) && (to.path || to.query || to.hash)) {
+    const path = toString(to.path)
+    const query = stringifyQueryObj(to.query)
+    let hash = toString(to.hash)
+    hash = !hash || hash.charAt(0) === '#' ? hash : `#${hash}`
+    return `${path}${query}${hash}` || toFallback
   }
 
   // If nothing is provided return the fallback

src/utils/router.spec.js

@@ -110,14 +110,8 @@ describe('utils/router', () => {
 
     it('parses nothing to default', async () => {
       expect(computeHref()).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)
+      expect(computeHref(undefined, '/', '')).toEqual('/')
+      expect(computeHref(undefined, '', '')).toEqual('')
     })
 
     it('returns href when both href and to provided', async () => {
@@ -130,8 +124,8 @@ describe('utils/router', () => {
 
     it('parses empty `href` to default', async () => {
       expect(computeHref({ href: '' })).toEqual('#')
-      expect(computeHref({ href: '' }, 'a', '/', '')).toEqual('/')
-      expect(computeHref({ href: '' }, 'a', '', '')).toEqual('')
+      expect(computeHref({ href: '' }, '/', '')).toEqual('/')
+      expect(computeHref({ href: '' }, '', '')).toEqual('')
     })
 
     it('parses `to` when string', async () => {
@@ -179,8 +173,8 @@ describe('utils/router', () => {
 
     it('parses empty `to` to fallback default', async () => {
       expect(computeHref({ to: {} })).toEqual('#')
-      expect(computeHref({ to: {} }, 'a', '#', '')).toEqual('#')
-      expect(computeHref({ to: {} }, 'a', '/', '#')).toEqual('/')
+      expect(computeHref({ to: {} }, '#', '')).toEqual('#')
+      expect(computeHref({ to: {} }, '/', '#')).toEqual('/')
     })
 
     it('parses complete `to`', async () => {
@@ -204,8 +198,9 @@ describe('utils/router', () => {
   describe('isRouterLink()', () => {
     it('works', async () => {
       expect(isRouterLink('a')).toBe(false)
-      expect(isRouterLink('div')).toBe(true)
-      expect(isRouterLink()).toBe(true)
+      expect(isRouterLink('router-link')).toBe(true)
+      expect(isRouterLink('nuxt-link')).toBe(true)
+      expect(isRouterLink()).toBe(false)
     })
   })