From b71f3fec98ab4e3f34234bba3350c42c57e1be78 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Wed, 31 Jan 2024 22:11:16 +1100 Subject: [PATCH] refactor(nuxt): normalize `external` link behavior --- packages/nuxt/src/app/components/nuxt-link.ts | 204 ++++++++---------- test/basic.test.ts | 4 + .../basic/pages/nuxt-link/trailing-slash.vue | 8 +- 3 files changed, 106 insertions(+), 110 deletions(-) diff --git a/packages/nuxt/src/app/components/nuxt-link.ts b/packages/nuxt/src/app/components/nuxt-link.ts index 88a0081bc1..afdb27f900 100644 --- a/packages/nuxt/src/app/components/nuxt-link.ts +++ b/packages/nuxt/src/app/components/nuxt-link.ts @@ -1,12 +1,19 @@ -import type { ComputedRef, DefineComponent, InjectionKey, PropType } from 'vue' +import type { + AllowedComponentProps, AnchorHTMLAttributes, + DefineComponent, + InjectionKey, + PropType, + VNodeProps +} from 'vue' import { computed, defineComponent, h, inject, onBeforeUnmount, onMounted, provide, ref, resolveComponent } from 'vue' -import type { RouteLocation, RouteLocationRaw } from '#vue-router' -import { hasProtocol, joinURL, parseQuery, parseURL, withTrailingSlash, withoutTrailingSlash } from 'ufo' +import type { RouteLocationRaw } from '#vue-router' +import { hasProtocol, parseQuery, parseURL, withTrailingSlash, withoutTrailingSlash } from 'ufo' +import type { RouterLinkProps } from 'vue-router' import { preloadRouteComponents } from '../composables/preload' import { onNuxtReady } from '../composables/ready' import { navigateTo, useRouter } from '../composables/router' -import { useNuxtApp, useRuntimeConfig } from '../nuxt' +import { useNuxtApp } from '../nuxt' import { cancelIdleCallback, requestIdleCallback } from '../compat/idle-callback' // @ts-expect-error virtual file @@ -59,27 +66,8 @@ export function defineNuxtLink (options: NuxtLinkOptions) { console.warn(`[${componentName}] \`${main}\` and \`${sub}\` cannot be used together. \`${sub}\` will be ignored.`) } } - const resolveTrailingSlashBehavior = ( - to: RouteLocationRaw, - resolve: (to: RouteLocationRaw) => RouteLocation & { href?: string } - ): RouteLocationRaw | RouteLocation => { - if (!to || (options.trailingSlash !== 'append' && options.trailingSlash !== 'remove')) { - return to - } - - if (typeof to === 'string') { - return applyTrailingSlashBehavior(to, options.trailingSlash) - } - - const path = 'path' in to ? to.path : resolve(to).path - - return { - ...to, - name: undefined, // named routes would otherwise always override trailing slash behavior - path: applyTrailingSlashBehavior(path, options.trailingSlash) - } - } + // TODO migrate to TypeScript props return defineComponent({ name: componentName, props: { @@ -153,7 +141,8 @@ export function defineNuxtLink (options: NuxtLinkOptions) { required: false }, - // Edge cases handling + // TODO deprecate, prefer more explicit prop name + // Should the link be rendered using an `a` tag instead of a `RouterLink` external: { type: Boolean as PropType, default: undefined, @@ -169,46 +158,70 @@ export function defineNuxtLink (options: NuxtLinkOptions) { }, setup (props, { slots }) { const router = useRouter() - const config = useRuntimeConfig() - // Resolving `to` value from `to` and `href` props - const to: ComputedRef = 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) - }) - - // Lazily check whether to.value has a protocol - const isProtocolURL = computed(() => typeof to.value === 'string' && hasProtocol(to.value, { acceptRelative: true })) - - // Resolving link type - const isExternal = computed(() => { - // External prop is explicitly set - if (props.external) { - return true - } - - // When `target` prop is set, link is external - if (props.target && props.target !== '_self') { - return true - } - - // When `to` is a route object then it's an internal link - if (typeof to.value === 'object') { - return false - } - - return to.value === '' || isProtocolURL.value - }) - - // Prefetching const prefetched = ref(false) const el = import.meta.server ? undefined : ref(null) const elRef = import.meta.server ? undefined : (ref: any) => { el!.value = props.custom ? ref?.$el?.nextElementSibling : ref?.$el } - if (import.meta.client) { + const link = computed(() => { + checkPropConflicts(props, 'to', 'href') + return props.to || props.href || '' // Defaults to empty string (won't render any `href` attribute) + }) + const href = computed(() => { + return typeof link.value === 'string' ? link.value : router.resolve(link.value).path + }) + const as = computed(() => { + const isExternalLink = hasProtocol(href.value, { acceptRelative: true }) + const forceAnchorTag = props.external + return !forceAnchorTag && !isExternalLink ? 'RouterLink' : 'a' + }) + + const routerLinkProps = computed(() => { + const path = applyTrailingSlashBehavior(href.value, options.trailingSlash) + // we need to provide a unresolved to prop to router-link otherwise it will strip arguments + const to = typeof link.value === 'string' ? path : { + ...link.value, + name: undefined, + path, + } + const attrs: AllowedComponentProps & VNodeProps & RouterLinkProps & AnchorHTMLAttributes = { + ref: elRef, + to, + activeClass: props.activeClass || options.activeClass, + exactActiveClass: props.exactActiveClass || options.exactActiveClass, + replace: props.replace, + ariaCurrentValue: props.ariaCurrentValue, + custom: props.custom + } + + // `custom` API cannot support fallthrough attributes as the slot + // may render fragment or text root nodes (#14897, #19375) + if (!props.custom) { + if (prefetched.value) { + attrs.class = props.prefetchedClass || options.prefetchedClass + } + attrs.rel = props.rel + } + return attrs + }) + + const anchorProps = computed(() => { + const to = link.value + // Resolves `target` value + const target = props.target || null + + // Resolves `rel` + checkPropConflicts(props, 'noRel', 'rel') + const rel = (props.noRel) + ? null + // converts `""` to `null` to prevent the attribute from being added as empty (`rel=""`) + : firstNonUndefined(props.rel, options.externalRelAttribute, to ? DEFAULT_EXTERNAL_REL_ATTRIBUTE : '') || null + + return { ref: el, href: to, rel, target } + }) + + // we can only prefetch valid vue-router links + if (import.meta.client && as.value === 'RouterLink') { checkPropConflicts(props, 'prefetch', 'noPrefetch') const shouldPrefetch = props.prefetch !== false && props.noPrefetch !== true && props.target !== '_blank' && !isSlowConnection() if (shouldPrefetch) { @@ -224,10 +237,9 @@ export function defineNuxtLink (options: NuxtLinkOptions) { unobserve?.() unobserve = null - const path = typeof to.value === 'string' ? 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(() => {}) + nuxtApp.hooks.callHook('link:prefetch', href.value).catch(() => {}), + preloadRouteComponents(link.value, router).catch(() => {}) ]) prefetched.value = true }) @@ -253,53 +265,24 @@ export function defineNuxtLink (options: NuxtLinkOptions) { } return () => { - if (!isExternal.value) { - const routerLinkProps: Record = { - ref: elRef, - to: to.value, - activeClass: props.activeClass || options.activeClass, - exactActiveClass: props.exactActiveClass || options.exactActiveClass, - replace: props.replace, - ariaCurrentValue: props.ariaCurrentValue, - custom: props.custom - } - - // `custom` API cannot support fallthrough attributes as the slot - // may render fragment or text root nodes (#14897, #19375) - if (!props.custom) { - if (prefetched.value) { - routerLinkProps.class = props.prefetchedClass || options.prefetchedClass - } - routerLinkProps.rel = props.rel - } - + if (as.value === 'RouterLink') { // Internal link return h( - resolveComponent('RouterLink'), - routerLinkProps, + resolveComponent(as.value), + routerLinkProps.value, slots.default ) } - // Resolves `to` value if it's a route location object - // converts `""` to `null` to prevent the attribute from being added as empty (`href=""`) - const href = typeof to.value === 'object' - ? router.resolve(to.value)?.href ?? null - : (to.value && !props.external && !isProtocolURL.value) - ? resolveTrailingSlashBehavior(joinURL(config.app.baseURL, to.value), router.resolve) as string - : to.value || null + if (typeof link.value === 'object') { + console.log('[nuxt] [NuxtLink] Providing `to` as a vue-router route is not supported with external links.', link.value) + return null + } - // Resolves `target` value - const target = props.target || null - - // Resolves `rel` - checkPropConflicts(props, 'noRel', 'rel') - const rel = (props.noRel) - ? null - // converts `""` to `null` to prevent the attribute from being added as empty (`rel=""`) - : firstNonUndefined(props.rel, options.externalRelAttribute, href ? DEFAULT_EXTERNAL_REL_ATTRIBUTE : '') || null - - const navigate = () => navigateTo(href, { replace: props.replace }) + const navigate = () => navigateTo(anchorProps.value.href, { + replace: props.replace, + external: props.external, // must explicitly opt-in + }) // https://router.vuejs.org/api/#custom if (props.custom) { @@ -308,9 +291,10 @@ export function defineNuxtLink (options: NuxtLinkOptions) { } return slots.default({ - href, + ...anchorProps.value, navigate, get route () { + const href = anchorProps.value.href if (!href) { return undefined } const url = parseURL(href) @@ -328,15 +312,14 @@ export function defineNuxtLink (options: NuxtLinkOptions) { href } }, - rel, - target, - isExternal: isExternal.value, + isExternal: true, isActive: false, isExactActive: false }) } - return h('a', { ref: el, href, rel, target }, slots.default?.()) + // "a" tag + return h(as.value, anchorProps.value, slots.default?.()) } } }) as unknown as DefineComponent @@ -345,7 +328,10 @@ export function defineNuxtLink (options: NuxtLinkOptions) { export default defineNuxtLink(nuxtLinkDefaults) // -- NuxtLink utils -- -function applyTrailingSlashBehavior (to: string, trailingSlash: NuxtLinkOptions['trailingSlash']): string { +function applyTrailingSlashBehavior (to: string, trailingSlash?: NuxtLinkOptions['trailingSlash']): string { + if (!trailingSlash || !to) { + return to + } const normalizeFn = trailingSlash === 'append' ? withTrailingSlash : withoutTrailingSlash // Until https://github.com/unjs/ufo/issues/189 is resolved const hasProtocolDifferentFromHttp = hasProtocol(to) && !to.startsWith('http') diff --git a/test/basic.test.ts b/test/basic.test.ts index 6a3803ce02..56dd3b641e 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -541,6 +541,7 @@ describe('nuxt links', () => { "/nuxt-link/trailing-slash/", "/nuxt-link/trailing-slash/?with-state=true", "/nuxt-link/trailing-slash/?without-state=true", + "https://example.com/page.html", ], "link-without-trailing-slash": [ "/", @@ -551,6 +552,7 @@ describe('nuxt links', () => { "/nuxt-link/trailing-slash", "/nuxt-link/trailing-slash?with-state=true", "/nuxt-link/trailing-slash?without-state=true", + "https://example.com/page.html", ], "nuxt-link": [ "/", @@ -561,6 +563,7 @@ describe('nuxt links', () => { "/nuxt-link/trailing-slash", "/nuxt-link/trailing-slash?with-state=true", "/nuxt-link/trailing-slash?without-state=true", + "https://example.com/page.html", ], "router-link": [ "/", @@ -571,6 +574,7 @@ describe('nuxt links', () => { "/nuxt-link/trailing-slash", "/nuxt-link/trailing-slash?with-state=true", "/nuxt-link/trailing-slash?without-state=true", + "/nuxt-link/https://example.com/page.html", ], } `) diff --git a/test/fixtures/basic/pages/nuxt-link/trailing-slash.vue b/test/fixtures/basic/pages/nuxt-link/trailing-slash.vue index 1b063cde42..526cdca842 100644 --- a/test/fixtures/basic/pages/nuxt-link/trailing-slash.vue +++ b/test/fixtures/basic/pages/nuxt-link/trailing-slash.vue @@ -13,7 +13,8 @@ const links = [ '/nuxt-link/trailing-slash/?test=true&thing=other/thing#thing-other', { name: 'nuxt-link-trailing-slash' }, { query: { 'with-state': 'true' }, state: { foo: 'bar' } }, - { query: { 'without-state': 'true' } } + { query: { 'without-state': 'true' } }, + 'https://example.com/page.html' ] as const const route = useRoute() @@ -28,11 +29,13 @@ const windowState = computed(() => {