instantpage / instant.page

Make your site’s pages instant in 1 minute and improve your conversion rate by 1%
https://instant.page
MIT License
6.04k stars 205 forks source link

Prefetch (slightly) more conservatively? #24

Closed futtta closed 5 years ago

futtta commented 5 years ago

I'm looking into instant.page vs. quicklink for inclusion in a (somewhat popular) WordPress plugin of mine. Based on feedback from users I'm leaning slightly more towards instant.page, but the following remark from https://css-tricks.com/instant-page/ might be worth considering;

Quicklink does two interesting things in which to attempt to be more responsible with prefetching: 1) wait for requestIdleCallback and 2) respects info from navigator.connection, like a user enabling data-saver mode

Now from what I understood quicklink prefetches a whole lot more then instant.page, but still these 2 changes (that amount to being somewhat conservative when prefetching) might be worth looking into?

ahmed-sigmalux commented 5 years ago

I second the use of requestidlecallback to keep the current page jank-free.

dieulot commented 5 years ago

requestIdleCallback wouldn’t be as useful as it is in quicklink. quicklink adds an IntersectionObserver to every link on the page, instant.page just adds a <link> to the <head> and two event listeners on document and checks for two data- attributes in the <body>.

Checking for data-saver mode also isn’t as useful as it is in quicklink, because instant.page preloads only if there’s a good chance that the page is going to be visited, while quicklink preloads everything it sees. Also it’s mostly Chrome-specific.

That said, they would still be a bit useful, so I’m open to adding them.

kurtextrem commented 5 years ago

rIC could be used before adding the event listeners and the link tag to the dom.

The only use of rIC in quicklink is here: https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L85. And instant.page does not query the dom and prefetches X links at the "same time", but only ever once, on click. The handler itself is somewhat cheap and settings a single DOM attribute is probably not worth using rIC + a polyfill (as the preload happens on a different thread anyway).

dieulot commented 5 years ago

I’m backtracking on my statement that they would be welcome. I’ll have to think more about it. They seem like over-optimization.

requestIdleCallback’s use and non-use would need to be benchmarked; though I’m pretty sure it would only save a negligible amount.

Disabling prefetching in Chrome’s data saver might not be worth it either, the HTML of a page is 50kB on a big site while the average complete web page is over 60 times that, and 80% (and growing) of them won’t be optimized by Chrome’s data saver mode because they use HTTPS.

Chrome could also have disabled prefetch if they thought it was a good idea. I’m sure they thought about it (and ended up deciding against it) because they’re using prefetching on their proxy. I have asked the former lead on the Chrome data saver team its opinion though (he doesn’t work at Google anymore so I’m not sure if I’ll hear back.)

dieulot commented 5 years ago

instant.page’s initialization (from const prefetcher = to document.addEventListener('mouseover') takes on average 1.5 ms on a first gen Moto G, a cheap smartphone from 2013. So requestIdleCallback’ing the initialization is definitely not worth it.

I’m leaving this issue open while I ponder a bit more about Chrome’s data saving mode.

ahmed-sigmalux commented 5 years ago

instant.page’s initialization (from const prefetcher = to document.addEventListener('mouseover') takes on average 1.5 ms on a first gen Moto G, a cheap smartphone from 2013. So requestIdleCallback’ing the initialization is definitely not worth it.

I’m leaving this issue open while I ponder a bit more about Chrome’s data saving mode.

requestIdleCallback should be used not because instant.page's function(s) take a long time, but rather because prefetching a potential future navigation shouldn't interrupt anything running on the current page.

kurtextrem commented 5 years ago

Quicklink does not use rIC to preload. It uses it once to catch all links on a page, register the IntersectionObserver - see here: https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L85. Preloading in instant.page only happens if a user is very likely to click on a link, as it's triggered after 65ms.

The only maybe useful thing is to use requestIdleCallback(fn, { timeout: 65 }), but this might reduce the valuable time that is left until the click event to preload the page.

The <link rel="preload"> doesn't preload on main thread afaik. It is non-blocking.

dieulot commented 5 years ago

I doubt prefetching takes more than a couple of milliseconds, and even if it did it might be detrimental to delay them. Note also that event listeners are passive so they already don’t interrupt scrolling.

notasausage commented 5 years ago

Enjoying the discussion here, found it really interesting to read through both scripts. While I would love to see instant.page take advantage of the Network Information API like Google's Quicklink, it doesn't seem all that useful yet due to the lack of browser support (Chrome of course has implemented it though). Hopefully that changes in the future, at which point adding it into instant.page would be a nice touch.

dieulot commented 5 years ago

So in the end I just added it (in 1.2.2) because I saw enough people comparing instant.page and quicklink and bringing up that point.