From 84719b9af1aed4cd284ed9f4d479add17bb8000e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 9 Oct 2020 10:53:15 +0200 Subject: [PATCH 1/4] fix(b-link): `href` handling inconsistencies to `` --- src/components/link/link.js | 14 ++-- .../pagination-nav/pagination-nav.js | 2 +- src/constants/regex.js | 47 +++++++----- src/utils/router.js | 72 ++++++------------- src/utils/router.spec.js | 23 +++--- 5 files changed, 73 insertions(+), 85 deletions(-) diff --git a/src/components/link/link.js b/src/components/link/link.js index 20665028ca8..e8c5172f7dc 100644 --- a/src/components/link/link.js +++ b/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), @@ -156,7 +158,9 @@ export const BLink = /*#__PURE__*/ Vue.extend({ // (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') && !isTag(routerTag, 'area') + ? {} + : { rel, target }), tabindex: disabled ? '-1' : isUndefined(bvAttrs.tabindex) ? null : bvAttrs.tabindex, 'aria-disabled': disabled ? 'true' : null } diff --git a/src/components/pagination-nav/pagination-nav.js b/src/components/pagination-nav/pagination-nav.js index de86b4aa4d1..5e309dc9875 100644 --- 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 }, '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) diff --git a/src/constants/regex.js b/src/constants/regex.js index c9b7b232359..742a47d5551 100644 --- a/src/constants/regex.js +++ b/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-.+/ diff --git a/src/utils/router.js b/src/utils/router.js index 521de9fd0bd..43f17ee0be6 100644 --- a/src/utils/router.js +++ b/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 diff --git a/src/utils/router.spec.js b/src/utils/router.spec.js index 78048a9424c..f7e51c49448 100644 --- a/src/utils/router.spec.js +++ b/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) }) }) From 3cf91026a525bf5c0c08a3471371e0196a1a7d5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 9 Oct 2020 10:56:25 +0200 Subject: [PATCH 2/4] Update regex.js --- src/constants/regex.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/constants/regex.js b/src/constants/regex.js index 742a47d5551..8450433470a 100644 --- a/src/constants/regex.js +++ b/src/constants/regex.js @@ -41,7 +41,7 @@ export const RX_ENCODED_COMMA = /%2C/g export const RX_ENCODE_REVERSE = /[!'()*]/g export const RX_QUERY_START = /^(\?|#|&)/ -// -- Aspect --- +// --- Aspect --- export const RX_ASPECT = /^\d+(\.\d*)?[/:]\d+(\.\d*)?$/ export const RX_ASPECT_SEPARATOR = /[/:]/ From f3882e78adcb276665c9b85a32d0511754af8f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Mon, 12 Oct 2020 17:11:41 +0200 Subject: [PATCH 3/4] Update link.js --- src/components/link/link.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/link/link.js b/src/components/link/link.js index e8c5172f7dc..2f1e6addcbe 100644 --- a/src/components/link/link.js +++ b/src/components/link/link.js @@ -158,9 +158,7 @@ export const BLink = /*#__PURE__*/ Vue.extend({ // (i.e. if `computedHref()` is truthy) ...(href ? { href } : {}), // We don't render `rel` or `target` on non link tags when using `vue-router` - ...(isRouterLink && !isTag(routerTag, 'a') && !isTag(routerTag, 'area') - ? {} - : { rel, target }), + ...(isRouterLink && !isTag(routerTag, 'a') ? {} : { rel, target }), tabindex: disabled ? '-1' : isUndefined(bvAttrs.tabindex) ? null : bvAttrs.tabindex, 'aria-disabled': disabled ? 'true' : null } From c5cd1545bffddbcc3557eb575171b6bca9d33c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Mon, 12 Oct 2020 17:16:49 +0200 Subject: [PATCH 4/4] Update link.js --- src/components/link/link.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/link/link.js b/src/components/link/link.js index 2f1e6addcbe..9b91743dada 100644 --- a/src/components/link/link.js +++ b/src/components/link/link.js @@ -155,7 +155,7 @@ export const BLink = /*#__PURE__*/ Vue.extend({ ...bvAttrs, // If `href` attribute exists on `` (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 && !isTag(routerTag, 'a') ? {} : { rel, target }),