Skip to content

Commit

Permalink
LinkPrefetchObserver: listen for complementary events
Browse files Browse the repository at this point in the history
Prior to this commit, the `LinkPrefetchObserver` only listened for
`mouseleave` events to clear the `PrefetchCache` instance. Not only were
`mouseenter` events excluded, but the `mouseleave` event listeners were
attached directly to the `<a>` element with a `{ once: true }` option.

While unlikely, its was for those event listeners to never be removed if
a `mouseleave` were to not fire. Similarly, during `touchstart` events
the event listener were added, but never removed since there wasn't a
complementary `touchend` or `touchcancel` event firing to remove it.

This commit makes two changes to the event listeners:

1. extract the `addEventListener` calls to a loop, looping over
   `mouseenter` and `touchstart` event names
2. define complementary events for both `mouseenter` (`mouseleave`) and
   `touchstart` (`touchend` and `touchcancel`)

By moving the cancellation logic out of individual event listeners and
into an `this.eventTarget`-wide scope, we limit the risk of leaking
listeners. Similarly, we only ever instantiate one per event-pairing.

To track the `<a>` element reference, define both a
`this.#tryToCancelPrefetchRequest` method and a `this.#linkToPrefetch`
property to hold the reference to the `<a>` element in question.
  • Loading branch information
seanpdoyle committed Feb 6, 2024
1 parent f4bbb77 commit 7d338b6
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 39 deletions.
55 changes: 37 additions & 18 deletions src/observers/link_prefetch_observer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ import { StreamMessage } from "../core/streams/stream_message"
import { FetchMethod, FetchRequest } from "../http/fetch_request"
import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache"

const observedEvents = {
"mouseenter": ["mouseleave"],
"touchstart": ["touchend", "touchcancel"]
}

export class LinkPrefetchObserver {
started = false
hoverTriggerEvent = "mouseenter"
touchTriggerEvent = "touchstart"
#linkToPrefetch = null

constructor(delegate, eventTarget) {
this.delegate = delegate
Expand All @@ -32,26 +36,34 @@ export class LinkPrefetchObserver {
stop() {
if (!this.started) return

this.eventTarget.removeEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
this.eventTarget.removeEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, {
capture: true,
passive: true
Object.entries(observedEvents).forEach(([startEventName, stopEventNames]) => {
this.eventTarget.removeEventListener(startEventName, this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
stopEventNames.forEach((stopEventName) => {
this.eventTarget.removeEventListener(stopEventName, this.#tryToCancelPrefetchRequest, {
capture: true,
passive: true
})
})
})
this.eventTarget.removeEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true)
this.started = false
}

#enable = () => {
this.eventTarget.addEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
this.eventTarget.addEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, {
capture: true,
passive: true
Object.entries(observedEvents).forEach(([startEventName, stopEventNames]) => {
this.eventTarget.addEventListener(startEventName, this.#tryToPrefetchRequest, {
capture: true,
passive: true
})
stopEventNames.forEach((stopEventName) => {
this.eventTarget.addEventListener(stopEventName, this.#tryToCancelPrefetchRequest, {
capture: true,
passive: true
})
})
})
this.eventTarget.addEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true)
this.started = true
Expand All @@ -68,6 +80,7 @@ export class LinkPrefetchObserver {
const location = getLocationForLink(link)

if (this.delegate.canPrefetchRequestToLocation(link, location)) {
this.#linkToPrefetch = link
const fetchRequest = new FetchRequest(
this,
FetchMethod.get,
Expand All @@ -77,12 +90,18 @@ export class LinkPrefetchObserver {
)

prefetchCache.setLater(location.toString(), fetchRequest, this.#cacheTtl)

link.addEventListener("mouseleave", () => prefetchCache.clear(), { once: true })
}
}
}

#tryToCancelPrefetchRequest = (event) => {
if (event.target === this.#linkToPrefetch) {
prefetchCache.clear()
}

this.#linkToPrefetch = null
}

#tryToUsePrefetchedRequest = (event) => {
if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") {
const cached = prefetchCache.get(event.detail.url.toString())
Expand Down
30 changes: 9 additions & 21 deletions src/tests/functional/link_prefetch_observer_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,15 @@ test("it resets the cache when a link is hovered", async ({ page }) => {

test("it prefetches page on touchstart", async ({ page }) => {
await goTo({ page, path: "/hover_to_prefetch.html" })
await assertPrefetchedOnTouchstart({ page, selector: "#anchor_for_prefetch" })

let requestMade = false
const link = page.locator("#anchor_for_prefetch")
page.on("request", (request) => (requestMade = true))

await link.tap()
await sleep(100)

assertRequestMade(requestMade)
})

test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => {
Expand All @@ -248,26 +256,6 @@ test("it follows the link using the cached response when clicking on a link that
assert.equal(await page.title(), "Prefetched Page")
})

const assertPrefetchedOnTouchstart = async ({ page, selector, callback }) => {
let requestMade = false

page.on("request", (request) => {
callback && callback(request)
requestMade = true
})

const selectorXY = await page.$eval(selector, (el) => {
const { x, y } = el.getBoundingClientRect()
return { x, y }
})

await page.touchscreen.tap(selectorXY.x, selectorXY.y)

await sleep(100)

assertRequestMade(requestMade)
}

const assertPrefetchedOnHover = async ({ page, selector, callback }) => {
let requestMade = false

Expand Down

0 comments on commit 7d338b6

Please sign in to comment.