Skip to content
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

fix(nuxt): prefetch object-syntax routes with <NuxtLink> #19144

Merged
merged 4 commits into from Feb 20, 2023

Conversation

Mini-ghost
Copy link
Contributor

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Based on PR #19120, the preloadRouteComponents function now accepts a RouteLocationRaw type as a value. This update has also been made to <NuxtLink> to allow RouteLocationRaw as a valid value type.

This PR does not modify the type of hook link:prefetch, but obtains router.resolve(to.value).fullPath before calling the hook, because others still need string path, rash modification may cause Breaking change.

nuxtApp.hooks.hook('link:prefetch', (url) => {
if (hasProtocol(url)) { return }
const route = router.resolve(url)
if (!route) { return }
const layout = route?.meta?.layout
let middleware = Array.isArray(route?.meta?.middleware) ? route?.meta?.middleware : [route?.meta?.middleware]
middleware = middleware.filter(m => typeof m === 'string')
for (const name of middleware) {
if (typeof namedMiddleware[name] === 'function') {
namedMiddleware[name]()
}
}
if (layout && typeof layouts[layout] === 'function') {
layouts[layout]()
}
})

nuxtApp.hook('link:prefetch', (url) => {
const { protocol } = parseURL(url)
if (protocol && ['http:', 'https:'].includes(protocol)) {
externalURLs.value.add(url)
head?.patch({
script: [generateRules()]
})
}
})

nuxtApp.hooks.hook('link:prefetch', (url) => {
if (!parseURL(url).protocol) {
return loadPayload(url)
}
})

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@codesandbox
Copy link

codesandbox bot commented Feb 18, 2023

CodeSandbox logoCodeSandbox logoΒ  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. we currently support prefetching external links if experimental.crossOriginPrefetch is enabled, which the router won't be able to resolve. Maybe worth adding a check for whether a link is external?
  2. if we resolve in the link:prefetch hook we'll end up double-resolving
  3. possible solution to 1 and reduction in resolutions - we can skip if to.value is already a string?

@Mini-ghost
Copy link
Contributor Author

If it is an external link, to.value is a string, so I only judge whether to.value is a string, if yes, skip it, if not, execute router.resolve(to.value) .

@danielroe danielroe changed the title feat(nuxt): enable <NuxtLink> to accept RouteLocationRaw values fix(nuxt): prefetch object-syntax routes with <NuxtLink> Feb 20, 2023
@danielroe danielroe merged commit 268cded into nuxt:main Feb 20, 2023
@danielroe danielroe mentioned this pull request Feb 27, 2023
@nathanchase
Copy link
Contributor

nathanchase commented Mar 3, 2023

FYI: Something in this PR broke NuxtLink (or perhaps #19309), and am now getting flooded with
[Vue warn]: Extraneous non-props attributes (rel) were passed to component but could not be automatically inherited because component renders fragment or text root nodes. anywhere that uses a custom NuxtLink template.

Example:

<NuxtLink
  v-slot="{ href }"
  :custom="true"
  :to="`/user/${id}`"
>
    <a :href="href">
      <img src="avatar.png">
    </a>
</NuxtLink>

The above doesn't error in 3.2.2, but does in 3.2.3. - I will try to create a repro of the error in a nuxt.new StackBlitz.

@nathanchase
Copy link
Contributor

StackBlitz with the error:
https://stackblitz.com/edit/github-hmvahh?file=app.vue

@danielroe
Copy link
Member

That should be resolved in the edge channel, or in the next release with #19379

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants