Unobserve prefetched elements

[lfre]

Depending on the number of links on a page, or if new links are added programmatically after the initial call. Both the query selector in the timeout method https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L70, and/or the entries loop in the observer callback https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L23, could be problematic for subsequent calls.

I think it would be worth to unobserve elements that are already prefetched.

Additionally, instead of requiring additional calls to quicklink({ urls: [..]}), which I think it is still needed to override the intersection observer; a mutationobserver.mjs could be enabled through an option, or to keep the lib size down, expose it in dist and allow its default export to be passed as an option for runtime.

Let me know what you think and I can tackle these. 👍
Thanks!

[addyosmani]

Thanks for suggesting this idea. The two paths for observing elements we have at the moment are:

(default) Observe the document

For static sites, I don't imagine the content of the document to heavily change during the session (outside of things like ads). I can see a number of edge cases where pages do have dynamic content embedded that could create new links. With this in mind, changing the default behavior to stop observing document is something I'd personally say might have to be an option vs default initially.

(option) Observe one or more DOM elements

When a page author is observing specific parts of the page, they probably have a clearer level of insight into what is going to be relatively static vs. dynamic. As a call to quicklink() currently only allows you to specify one DOM element at a time, perhaps it makes sense to provide an unobserveOnPrefetch mechanism like this:

quicklink({ el: someElement, unobserveOnPrefetch: true });

or something to that effect. It could hook into the onload handler for rel=prefetch. Interested in others thoughts on how this could be exposed. I personally want to try avoiding going down the Mutation Observers path if possible.

[lfre]

Thanks for the feedback @addyosmani! I was thinking of an infinite scroll scenario where items are appended to a container. In https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L90, el is acting as a container rather than a element(s), so assuming my first call is:

quicklink({ unobserveOnPrefetch: true });

where el is document, my subsequent calls when appending new content would be:

quicklink({ el: [items-container], unobserveOnPrefetch: true });

The same elements from the first batch would be there as well and seem wasteful to observe them again, especially if they were already prefetched and unobserved. In this case, they would be because of the nature of an infinite scroll.

What I'm getting at is the ability to override the query options.el.querySelectorAll('a'), with an already filtered collection.

Maybe something like this?

const els = options.el instanceof window.NodeList || options.el instanceof Array ? options.el : options.el.querySelectorAll('a');

Another way, could be returning the link function, so it can be called on demand.

const linkObserver = link => {
   observer.observe(link);
   // filters...
};

Array.from(options.el.querySelectorAll('a'), linkObserver);

return linkObserver;

const ql = quicklink({ unobserveOnPrefetch: true });

// fetch renders new items...

newItems.forEach(ql);

Conceptually I prefer the latter since it reads as an extension of the initial call/options.

Thanks for your time!

[lukeed]

Edit: I had a moment 😇Was speaking on the basis of an alternate version & not actual quicklink code.

I still think we can add this by default. I don't see any harm in removing links from the Observer – it saves us repetitive work here: https://github.com/GoogleChromeLabs/quicklink/blob/master/src/index.mjs#L23-L28

The toPrefetch set won't have those links anymore (as they've already been processed), so it ends there. But we can make the entries loop shorter for less work 👍

[lukeed]

Perhaps we should rename this issue to reflect infinite scroll support? Now that quicklink is now technically unobserving processed elements, this issue has sparked a larger feature.

[lfre]

Thanks, @lukeed @addyosmani! I split the specific issue here. Apologies for lumping them together, infinite scroll is where the issue was most noticeable.

Closing this.