fix(nuxt): handle more edge cases with external/custom links (#27487)

This commit is contained in:
Daniel Roe 2024-06-13 16:39:37 +01:00 committed by GitHub
parent 452d43ab23
commit 95458af9a1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 135 additions and 41 deletions

View File

@ -8,7 +8,7 @@ import type {
} from 'vue'
import { computed, defineComponent, h, inject, onBeforeUnmount, onMounted, provide, ref, resolveComponent } from 'vue'
import type { RouteLocation, RouteLocationRaw, Router, RouterLink, RouterLinkProps, useLink } from '#vue-router'
import { hasProtocol, joinURL, parseQuery, withTrailingSlash, withoutTrailingSlash } from 'ufo'
import { hasProtocol, joinURL, parseQuery, withQuery, withTrailingSlash, withoutTrailingSlash } from 'ufo'
import { preloadRouteComponents } from '../composables/preload'
import { onNuxtReady } from '../composables/ready'
import { navigateTo, useRouter } from '../composables/router'
@ -70,8 +70,8 @@ export interface NuxtLinkProps extends Omit<RouterLinkProps, 'to'> {
* @see https://nuxt.com/docs/api/components/nuxt-link
*/
export interface NuxtLinkOptions extends
Pick<RouterLinkProps, 'activeClass' | 'exactActiveClass'>,
Pick<NuxtLinkProps, 'prefetchedClass'> {
Partial<Pick<RouterLinkProps, 'activeClass' | 'exactActiveClass'>>,
Partial<Pick<NuxtLinkProps, 'prefetchedClass'>> {
/**
* The name of the component.
* @default "NuxtLink"
@ -124,34 +124,17 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
const router = useRouter()
const config = useRuntimeConfig()
// Resolving `to` value from `to` and `href` props
const to: ComputedRef<string | RouteLocationRaw> = computed(() => {
checkPropConflicts(props, 'to', 'href')
const path = props.to || props.href || '' // Defaults to empty string (won't render any `href` attribute)
return resolveTrailingSlashBehavior(path, router.resolve)
})
const hasTarget = computed(() => !!props.target && props.target !== '_self')
// Lazily check whether to.value has a protocol
const isAbsoluteUrl = computed(() => typeof to.value === 'string' && hasProtocol(to.value, { acceptRelative: true }))
// Resolves `to` value if it's a route location object
const href = computed(() => (typeof to.value === 'object'
? router.resolve(to.value)?.href ?? null
: (to.value && !props.external && !isAbsoluteUrl.value)
? resolveTrailingSlashBehavior(joinURL(config.app.baseURL, to.value), router.resolve) as string
: to.value
))
const isAbsoluteUrl = computed(() => {
const path = props.to || props.href || ''
return typeof path === 'string' && hasProtocol(path, { acceptRelative: true })
})
const builtinRouterLink = resolveComponent('RouterLink') as string | typeof RouterLink
const useBuiltinLink = builtinRouterLink && typeof builtinRouterLink !== 'string' ? builtinRouterLink.useLink : undefined
const link = useBuiltinLink?.({
...props,
to: to.value,
})
const hasTarget = computed(() => props.target && props.target !== '_self')
// Resolving link type
const isExternal = computed<boolean>(() => {
// External prop is explicitly set
@ -159,17 +142,40 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
return true
}
// When `target` prop is set, link is external
if (hasTarget.value) {
return true
}
const path = props.to || props.href || ''
// When `to` is a route object then it's an internal link
if (typeof to.value === 'object') {
if (typeof path === 'object') {
return false
}
return to.value === '' || isAbsoluteUrl.value
return path === '' || isAbsoluteUrl.value
})
// Resolving `to` value from `to` and `href` props
const to: ComputedRef<RouteLocationRaw> = computed(() => {
checkPropConflicts(props, 'to', 'href')
const path = props.to || props.href || '' // Defaults to empty string (won't render any `href` attribute)
if (isExternal.value) { return path }
return resolveTrailingSlashBehavior(path, router.resolve)
})
const link = isExternal.value ? undefined : useBuiltinLink?.({ ...props, to })
// Resolves `to` value if it's a route location object
const href = computed(() => {
if (!to.value || isAbsoluteUrl.value) { return to.value as string }
if (isExternal.value) {
const path = typeof to.value === 'object' ? resolveRouteObject(to.value) : to.value
return resolveTrailingSlashBehavior(path, router.resolve /* will not be called */) as string
}
if (typeof to.value === 'object') {
return router.resolve(to.value)?.href ?? null
}
return resolveTrailingSlashBehavior(joinURL(config.app.baseURL, to.value), router.resolve /* will not be called */)
})
return {
@ -183,10 +189,10 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
isExactActive: link?.isExactActive ?? computed(() => to.value === router.currentRoute.value.path),
route: link?.route ?? computed(() => router.resolve(to.value)),
async navigate () {
await navigateTo(href.value, { replace: props.replace, external: props.external })
await navigateTo(href.value, { replace: props.replace, external: isExternal.value || hasTarget.value })
},
} satisfies ReturnType<typeof useLink> & {
to: ComputedRef<string | RouteLocationRaw>
to: ComputedRef<RouteLocationRaw>
hasTarget: ComputedRef<boolean | null | undefined>
isAbsoluteUrl: ComputedRef<boolean>
isExternal: ComputedRef<boolean>
@ -307,10 +313,12 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
unobserve?.()
unobserve = null
const path = typeof to.value === 'string' ? to.value : router.resolve(to.value).fullPath
const path = typeof to.value === 'string'
? to.value
: isExternal.value ? resolveRouteObject(to.value) : router.resolve(to.value).fullPath
await Promise.all([
nuxtApp.hooks.callHook('link:prefetch', path).catch(() => {}),
!isExternal.value && preloadRouteComponents(to.value as string, router).catch(() => {}),
!isExternal.value && !hasTarget.value && preloadRouteComponents(to.value as string, router).catch(() => {}),
])
prefetched.value = true
})
@ -336,7 +344,7 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
}
return () => {
if (!isExternal.value) {
if (!isExternal.value && !hasTarget.value) {
const routerLinkProps: RouterLinkProps & VNodeProps & AllowedComponentProps & AnchorHTMLAttributes = {
ref: elRef,
to: to.value,
@ -408,7 +416,7 @@ export function defineNuxtLink (options: NuxtLinkOptions) {
},
rel,
target,
isExternal: isExternal.value,
isExternal: isExternal.value || hasTarget.value,
isActive: false,
isExactActive: false,
})
@ -487,3 +495,7 @@ function isSlowConnection () {
if (cn && (cn.saveData || /2g/.test(cn.effectiveType))) { return true }
return false
}
function resolveRouteObject (to: Exclude<RouteLocationRaw, string>) {
return withQuery(to.path || '', to.query || {}) + (to.hash ? '#' + to.hash : '')
}

View File

@ -1,5 +1,5 @@
import { describe, expect, it, vi } from 'vitest'
import type { RouteLocation, RouteLocationRaw } from 'vue-router'
import type { RouteLocation } from 'vue-router'
import type { NuxtLinkOptions, NuxtLinkProps } from '../src/app/components/nuxt-link'
import { defineNuxtLink } from '../src/app/components/nuxt-link'
import { useRuntimeConfig } from '../src/app/nuxt'
@ -99,7 +99,11 @@ describe('nuxt-link:isExternal', () => {
})
it('returns `false` when `to` is a route location object', () => {
expect(nuxtLink({ to: { to: '/to' } as RouteLocationRaw }).type).toBe(INTERNAL)
expect(nuxtLink({ to: { path: '/to' } }).type).toBe(INTERNAL)
})
it('returns `true` when `to` has a `target`', () => {
expect(nuxtLink({ to: { path: '/to' }, target: '_blank' }).type).toBe(EXTERNAL)
})
it('honors `external` prop', () => {
@ -122,7 +126,12 @@ describe('nuxt-link:propsOrAttributes', () => {
})
it('resolves route location object', () => {
expect(nuxtLink({ to: { to: '/to' } as RouteLocationRaw, external: true }).props.href).toBe('/to')
expect(nuxtLink({ to: { path: '/to' }, external: true }).props.href).toBe('/to')
})
it('applies trailing slash behaviour', () => {
expect(nuxtLink({ to: { path: '/to' }, external: true }, { trailingSlash: 'append' }).props.href).toBe('/to/')
expect(nuxtLink({ to: '/to', external: true }, { trailingSlash: 'append' }).props.href).toBe('/to/')
})
})
@ -167,6 +176,8 @@ describe('nuxt-link:propsOrAttributes', () => {
}, () => {
expect(nuxtLink({ to: 'http://nuxtjs.org/app/about', target: '_blank' }).props.href).toBe('http://nuxtjs.org/app/about')
expect(nuxtLink({ to: '//nuxtjs.org/app/about', target: '_blank' }).props.href).toBe('//nuxtjs.org/app/about')
expect(nuxtLink({ to: { path: '/' }, external: true }).props.href).toBe('/')
expect(nuxtLink({ to: '/', external: true }).props.href).toBe('/')
})
})
})
@ -209,7 +220,7 @@ describe('nuxt-link:propsOrAttributes', () => {
describe('to', () => {
it('forwards `to` prop', () => {
expect(nuxtLink({ to: '/to' }).props.to).toBe('/to')
expect(nuxtLink({ to: { to: '/to' } as RouteLocationRaw }).props.to).toEqual({ to: '/to' })
expect(nuxtLink({ to: { path: '/to' } }).props.to).toEqual({ path: '/to' })
})
})

View File

@ -745,6 +745,24 @@ describe('nuxt links', () => {
`)
})
it('respects external links in edge cases', async () => {
const html = await $fetch<string>('/nuxt-link/custom-external')
const hrefs = html.match(/<a[^>]*href="([^"]+)"/g)
expect(hrefs).toMatchInlineSnapshot(`
[
"<a href="https://thehackernews.com/2024/01/urgent-upgrade-gitlab-critical.html"",
"<a href="https://thehackernews.com/2024/01/urgent-upgrade-gitlab-critical.html"",
"<a href="/missing-page/"",
"<a href="/missing-page/"",
]
`)
const { page, consoleLogs } = await renderPage('/nuxt-link/custom-external')
const warnings = consoleLogs.filter(c => c.text.includes('No match found for location'))
expect(warnings).toMatchInlineSnapshot(`[]`)
await page.close()
})
it('preserves route state', async () => {
const { page } = await renderPage('/nuxt-link/trailing-slash')

View File

@ -0,0 +1,53 @@
<script setup lang="ts">
const MyLink = defineNuxtLink({
componentName: 'MyLink',
trailingSlash: 'append',
})
</script>
<template>
<div>
<div>
<MyLink to="https://thehackernews.com/2024/01/urgent-upgrade-gitlab-critical.html">
Trailing slashes should not be applied to implicit external links
</MyLink>
</div>
<div>
<NuxtLink
:to="{ path: 'https://thehackernews.com/2024/01/urgent-upgrade-gitlab-critical.html' }"
external
>
External links within route objects should be respected and not have trailing slashes applied
</NuxtLink>
</div>
<div>
<MyLink
:to="{ path: '/missing-page' }"
external
>
External links within route objects should be respected and have trailing slashes applied
</MyLink>
</div>
<div>
<MyLink
to="/missing-page"
external
>
External links should be respected and have trailing slashes applied
</MyLink>
</div>
<div>
<NuxtLink
custom
to="https://google.com"
external
>
<template #default="{ navigate }">
<button @click="navigate()">
Using navigate() with external link should work
</button>
</template>
</NuxtLink>
</div>
</div>
</template>