Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/components/link/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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),
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/pagination-nav/pagination-nav.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
47 changes: 31 additions & 16 deletions src/constants/regex.js
Original file line number Diff line number Diff line change
@@ -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+$/
Expand All @@ -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+/
Expand All @@ -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-.+/
72 changes: 23 additions & 49 deletions src/utils/router.js
Original file line number Diff line number Diff line change
@@ -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)

Expand All @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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
Expand Down
23 changes: 9 additions & 14 deletions src/utils/router.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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)
})
})

Expand Down