craftedbygc / taxi

Taxi is a 🤏 small and 🐊 snappy js library for adding slick PJAX navigation and beautiful transitions to your website.
https://taxi.js.org
565 stars 11 forks source link

Preloading assets too shortly in advance can lead to double fetch #13

Closed tobimori closed 1 year ago

tobimori commented 1 year ago

Describe the bug When preloading a page in advance, it will only be added to the cache as soon as the fetch is completed. This means, if the Navigation is started soon after, it'll try to fetch the page again instead of waiting for the preloaded entry to finish downloading.

Ideally, I want to be able to preload a page, and then let the Navigation use that preloaded page even if it starts before preloading is finished, so no content gets downloaded twice.

Why? Starting preloading pages as soon as the user hovers over a link can lead to a lower loading time, as most users need 100-200ms to actually click a link upon hovering. This pattern is something I'd like to implement yet can't because of this constraint.

To Reproduce Steps to reproduce the behavior:

  1. Preload an URL
  2. Navigate to the same page shortly after
  3. Taxi will fire two requests
jakewhiteley commented 1 year ago

Hi @tobimori

For a short while, hovering over links did actually prefetch just as your are, but this issue cropped up.

I could never recreate it however, and couldn't track down the specific browser condition causing it to happen so I abandoned the fix.

I believe the approach to solve would be to have a pool of requests, and upon navigation check if a pending request exists in the pool for the target URL. If it does, then we wait for that to resolve instead of creating a new fetch promise.

I can write this update, but would you be willing to help me test it?

tobimori commented 1 year ago

yes of course! would be glad to get this to work

jakewhiteley commented 1 year ago

@tobimori I have put the prefetch functionality back in, but this time using a promises stack to avoid duplicate requests.

The feature is enabled by default, but you can opt out by passing enablePrefetch: false into the initial parameters.

The feature will prefetch a url if a user mouses over a link, but will not attempt to actually load any of the images/js/videos etc from the new page - it simply caches the HTML response to make navigation much snappier.

Of course this also works with the prefetch function in case you want to prefetch, but not by the in-built mouseenter/focus mechanism.

Could you pull the master branch directly into your project and help me test for race conditions/duplicate network requests?

I think it's pretty solid, but would appreciate the extra testing!

tobimori commented 1 year ago

Just tested it, works great for me! Thanks! Types for the enablePrefetch option are still missing though.

jakewhiteley commented 1 year ago

Yeah I didn't rebuild the typings for this test. What IDE are you using? VS?

I'll push it to npm shortly as a new version (with typings of course).

tobimori commented 1 year ago

yeah vs code

jakewhiteley commented 1 year ago

Done - v1.3.0

I'm going to close this issue, but please feel free to reopen if you find any issues 💪