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

Prefetch Relevant Pages for Faster Navigation #3575

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

joshistoast
Copy link

@joshistoast joshistoast commented Aug 1, 2024

PR Summary:

Prefetch link urls for faster page navigation.
For desktop, hovered links will be prefetched. For mobile, links within view will be prefetched.

Why not just use any other pre-made js page prefetching solutions?

Because they consistently break the cart. Also, who doesn't want a native implementation?

Why are these changes introduced?

Faster page loads = better customer experience = better sales

What approach did you take?

Created 3 functions: addPrefetchLink, initPagePrefetching and getPrefetchMethod.

  1. addPrefetchLink takes arguments href and priority, and appends a <link> element to the document head. If an href is already present in document head <link> then skip.
  2. initPagePrefetching takes a single method argument which can be either mouseover or intersection. The function is responsible for indexing all relevant links and- dependent on the method argument, runs addPrefetchLink on hover or intersection.
  3. getprefetchMethod will return mouseover or intersection dependent on browser size.

initPagePrefetching will run on dom content loaded using the method provided by getPrefetchMethod. When browser is resized, the init is run again w/ debounce so the correct prefetch method is always correct.

Other considerations

Decision log

# Decision Alternatives Rationale Downsides
1 Add support for speculation api rely entirely on link prefetches It's a better and newer approach Not very widespread browser support
2 Move speculation api usage to prefetching function Better consolidation and better for mobile prefetching None apparent

Visual impact on existing themes

Faster page load, less content flash.

Testing steps/scenarios

  • Within Application -> Speculative Loads -> Speculations, links successfully prefetch to the best of the browser's ability.
  • Desktop hovering internal links adds to head and present in network requests.
  • Mobile link visibility in intersection adds to head and present in network requests.
  • Resizing browser changes link prefetch method. Try hovering/scrolling between screen sizes and analyze network requests for behaviors mentioned above.

Demo links

  • Store
    Password: prefetch

Checklist

@octipus
Copy link

octipus commented Aug 1, 2024

This is a great addition to Dawn and i think it can benefit many merchants. I would love to check the demo but the url's are not working.

Anyway, there is a native JS API for this, that serves the same purpose, called Speculation Rules . More here
We can argue that it does not have full browser compatibility (yet) but it is functional on any modern browser, yet.

@joshistoast
Copy link
Author

This is a great addition to Dawn and i think it can benefit many merchants. I would love to check the demo but the url's are not working.

Anyway, there is a native JS API for this, that serves the same purpose, called Speculation Rules . More here We can argue that it does not have full browser compatibility (yet) but it is functional on any modern browser, yet.

Updated the PR description with a development store link :)

I'll definitely need to look into the speculation api, maybe could update this pr with both methods dependent on browser 👀

@joshistoast
Copy link
Author

Latest commit adds speculation api support for supported browsers, and falls back to previous implementation otherwise. I've found no method to catch speculation api failures to fallback, so if prerendering is disabled by the browser or an ad-blocking extension- then there's no prefetch.

@bakura10
Copy link

bakura10 commented Aug 2, 2024

I recommend that you use instantpage library for that. It is a lightweight library and basically uses your approach (we're using it on our themes for several years and when used currently it does not cause cart issues).

There are a lot of edge case and you're basically re-creating instantpage. Especially as you're using a pretty aggressive preloading approach on mobile which can load a lot of unused data, things like taking into account power saving mode, slow connection... is important.

My opinion here though is that you should wait for @krzksz opinion here. A few months ago I could try an experimental feature on Shopify that would take advantage of native speculation API, managed by Shopify. It was a quite interesting approach and I think that is the road to go at this stage.

Browser support is an issue but, having said that, prefetching does not work on Safari and Firefox neither (while those browsers support the prefetch, the pages are never served from the prefetch cache so you don't get the "instant" boost). The prefetch approach is therefore a Chrome-only improvement, as is the speculation rules API.

@krzksz
Copy link
Contributor

krzksz commented Aug 2, 2024

Hi @joshistoast, thank you for the contribution! Unfortunately, I'm aligned with what @bakura10 mentioned above.

I'm exploring a platform-wide solution that would output Speculation Rules as a part of content_for_header. This will allow us to potentially cover all of the stores, no matter if they're based on the new version of Dawn or not.

We'll also be able to enable/adjust/disable the configuration any time, without having to deal with all of the logic that's already spread across merchants theme code. We need this ability because I've hit a lot of corner cases and browser bugs in beta testing already.

I don't have an ETA yet, but it's on top of my priority list right now.

@joshistoast
Copy link
Author

I like @krzksz platform-wide solution idea, however depending on if that eta is a few weeks/months or even years, I see no reason to delay having an initial implementation until then. Much like how we initially used structured data and removed lots of it in favor of the new structured_data filter.

Also, shouldn't speculation rules go at the end of the body instead of the head?

To address @bakura10's suggestions:

  1. I've tried several prefetching libraries already including instantpage, all of them reliably cause cart issues within dawn specifically... without configuring what routes to disallow/allow. Ushering merchants to a non-native solution and having them configure the js to get it working sounds less intuitive.
  2. Reading the instantpage source, my approach is just as aggressive, but pre-configured for only relevant resources and natively implemented.
  3. Power saving can be accommodated for with a commit, slow connections sounds like an interesting case- since prefetching would probably help them the most.

@krzksz
Copy link
Contributor

krzksz commented Aug 2, 2024

I like @krzksz platform-wide solution idea, however depending on if that eta is a few weeks/months or even years, I see no reason to delay having an initial implementation until then. Much like how we initially used structured data and removed lots of it in favor of the new structured_data filter.

This is a different case. If we release something in, let's say v16.0.0, that later turns out to break some pages, there is no way for us to address this. For example, this can be something that triggers a severe browser bug, or an incorrect configuration that doesn't exclude all of the routes it should. We would need to hope that everyone will update quickly, which is unrealistic.

Speculation rules have a huge potential, but they also introduce a lot of additional risk. That's why we need to move slowly, while maintaining control and testing as much as we can.

@joshistoast
Copy link
Author

If the concern is in speculation api, I can simply remove it in favor of the tried and tested <link> method.

@joshistoast joshistoast changed the title feat: ✨ prefetch relevant pages Prefetch Pages for Near-Instant Page Loading Aug 2, 2024
@joshistoast joshistoast changed the title Prefetch Pages for Near-Instant Page Loading Prefetch Relevant Pages for Faster Navigation Aug 2, 2024
@joshistoast
Copy link
Author

@krzksz

@krzksz
Copy link
Contributor

krzksz commented Aug 7, 2024

I don't have any additional feedback and I can't approve this.

In my opinion, global prefetching is too dangerous to be a part of Dawn theme code. If something turns up to break pages, there will be no way for us to disable the logic and/or release a fix quickly.

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.

4 participants