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

feat: improved Link prefetching behavior, API, and docs; make it similar to Next.js #4678

Closed
wants to merge 12 commits into from

Conversation

jordanw66
Copy link
Contributor

@jordanw66 jordanw66 commented Jul 1, 2023

Overview

This PR makes prefetch on Link similar to prefetch on Next.js:

  • Prefetching occurs by default using on:qvisible.
  • Opt-out completely with prefetch={false}.
  • Or, optionally, only prefetch javascript bundles (and not data) to render the page with prefetch="js".
  • In dev mode, prefetch will occur with mouseover or focus instead of qvisible.
  • If a user has their data saver setting set, prefetching is skipped.
  • Optimized to not prefetch nor attach handlers on same-page Link. (hashes, navbar, etc)
  • Added detailed jsdoc/API explanation of what prefetch does and how it works, along with options.
  • Removed aria-pressed because this is for toggle buttons, not <a> tags. Screen readers already handle links, in fact adding and removing aria-pressed will cause confusion for these users as it's not normal link behavior.
  • Improved the reliability of pathname comparison by normalizing path trailing slashes first, added unit tests for this.
  • Added and modified unit tests for verifying decisions from prefetch checks.

Prefetching symbols (and/or data) to render a page before SPA navigation is a huge boost to user experience, especially on slow/mobile connections, preventing a burst of network activity during navigation.

If you think defaults or API should be different let me know.

References:
https://nextjs.org/docs/pages/api-reference/components/link#prefetch
https://nextjs.org/docs/messages/prefetch-true-deprecated
https://www.builder.io/blog/prefetching-nextjs-visual-guide
https://www.accessibility-developer-guide.com/examples/sensible-aria-usage/pressed/#intended-use

What is it?

  • Feature / enhancement
  • Bug
  • Docs / tests / types / typos

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Use cases and why

    1. One use case
    1. Another use case

Checklist:

  • My code follows the developer guidelines of this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • Added new tests to cover the fix / functionality

@stackblitz
Copy link

stackblitz bot commented Jul 1, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@netlify
Copy link

netlify bot commented Jul 1, 2023

👷 Deploy request for qwik-insights pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 7376b03

@jordanw66 jordanw66 changed the title feat: Link prefetches symbols by default feat: Link prefetches symbols (by default) Jul 1, 2023
@jordanw66 jordanw66 changed the title feat: Link prefetches symbols (by default) feat: improved Link prefetching behavior, API, and docs; make it similar to Next.js Jul 8, 2023
@zanettin zanettin added the WAITING FOR: team Waiting for one of the core team members to review and reply label Aug 22, 2023
@wmertens
Copy link
Member

@jordanw66 this is great! Could I implore you to rebase?

An e2e test would be very nice too, just not quite sure how to test prefetching.

@wmertens
Copy link
Member

@jordanw66 wow, I'm glad you're back now! Totally understand, if you could do a quick rebase that would be fine.

@wmertens
Copy link
Member

Closing in favor of #5480

@wmertens wmertens closed this Jan 17, 2024
@jordanw66 jordanw66 deleted the feat/link-prefetch-symbols branch March 14, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
COMP: qwik-city WAITING FOR: team Waiting for one of the core team members to review and reply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants