From 273456ad345ca10577551e591798aff6cde14b5a Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Mon, 4 Dec 2023 21:42:28 -0700 Subject: [PATCH 01/42] Move doesNotTargetIFrame to util.js --- src/observers/link_click_observer.js | 16 ++-------------- src/util.js | 12 ++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js index e6bd2fcaf..d02b73810 100644 --- a/src/observers/link_click_observer.js +++ b/src/observers/link_click_observer.js @@ -1,5 +1,5 @@ import { expandURL } from "../core/url" -import { findClosestRecursively } from "../util" +import { findClosestRecursively, doesNotTargetIFrame } from "../util" export class LinkClickObserver { started = false @@ -61,16 +61,4 @@ export class LinkClickObserver { getLocationForLink(link) { return expandURL(link.getAttribute("href") || "") } -} - -function doesNotTargetIFrame(anchor) { - if (anchor.hasAttribute("target")) { - for (const element of document.getElementsByName(anchor.target)) { - if (element instanceof HTMLIFrameElement) return false - } - - return true - } else { - return true - } -} +} \ No newline at end of file diff --git a/src/util.js b/src/util.js index 1dcf943fb..b1bfb1220 100644 --- a/src/util.js +++ b/src/util.js @@ -215,3 +215,15 @@ export async function around(callback, reader) { return [before, after] } + +export function doesNotTargetIFrame(anchor) { + if (anchor.hasAttribute("target")) { + for (const element of document.getElementsByName(anchor.target)) { + if (element instanceof HTMLIFrameElement) return false + } + + return true + } else { + return true + } +} \ No newline at end of file From 1df1428686722079c2326d49a6a8d9b5b8ed07f4 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 09:09:03 -0700 Subject: [PATCH 02/42] Move findLinkFromClickTarget to util.js --- src/observers/link_click_observer.js | 8 ++------ src/util.js | 8 +++++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js index d02b73810..6a11ebf69 100644 --- a/src/observers/link_click_observer.js +++ b/src/observers/link_click_observer.js @@ -1,5 +1,5 @@ import { expandURL } from "../core/url" -import { findClosestRecursively, doesNotTargetIFrame } from "../util" +import { findLinkFromClickTarget, doesNotTargetIFrame } from "../util" export class LinkClickObserver { started = false @@ -31,7 +31,7 @@ export class LinkClickObserver { clickBubbled = (event) => { if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) { const target = (event.composedPath && event.composedPath()[0]) || event.target - const link = this.findLinkFromClickTarget(target) + const link = findLinkFromClickTarget(target) if (link && doesNotTargetIFrame(link)) { const location = this.getLocationForLink(link) if (this.delegate.willFollowLinkToLocation(link, location, event)) { @@ -54,10 +54,6 @@ export class LinkClickObserver { ) } - findLinkFromClickTarget(target) { - return findClosestRecursively(target, "a[href]:not([target^=_]):not([download])") - } - getLocationForLink(link) { return expandURL(link.getAttribute("href") || "") } diff --git a/src/util.js b/src/util.js index b1bfb1220..41758a5ab 100644 --- a/src/util.js +++ b/src/util.js @@ -1,3 +1,5 @@ +import { expandURL } from "./core/url" + export function activateScriptElement(element) { if (element.getAttribute("data-turbo-eval") == "false") { return element @@ -226,4 +228,8 @@ export function doesNotTargetIFrame(anchor) { } else { return true } -} \ No newline at end of file +} + +export function findLinkFromClickTarget(target) { + return findClosestRecursively(target, "a[href]:not([target^=_]):not([download])") +} From 2f8c272214a599fe560d598d404ea7f896eef200 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 09:09:28 -0700 Subject: [PATCH 03/42] Move getLocationForLink to util.js --- src/observers/link_click_observer.js | 9 ++------- src/util.js | 4 ++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js index 6a11ebf69..29148ad3d 100644 --- a/src/observers/link_click_observer.js +++ b/src/observers/link_click_observer.js @@ -1,5 +1,4 @@ -import { expandURL } from "../core/url" -import { findLinkFromClickTarget, doesNotTargetIFrame } from "../util" +import { doesNotTargetIFrame, findLinkFromClickTarget, getLocationForLink } from "../util" export class LinkClickObserver { started = false @@ -33,7 +32,7 @@ export class LinkClickObserver { const target = (event.composedPath && event.composedPath()[0]) || event.target const link = findLinkFromClickTarget(target) if (link && doesNotTargetIFrame(link)) { - const location = this.getLocationForLink(link) + const location = getLocationForLink(link) if (this.delegate.willFollowLinkToLocation(link, location, event)) { event.preventDefault() this.delegate.followedLinkToLocation(link, location) @@ -53,8 +52,4 @@ export class LinkClickObserver { event.shiftKey ) } - - getLocationForLink(link) { - return expandURL(link.getAttribute("href") || "") - } } \ No newline at end of file diff --git a/src/util.js b/src/util.js index 41758a5ab..68d828f67 100644 --- a/src/util.js +++ b/src/util.js @@ -233,3 +233,7 @@ export function doesNotTargetIFrame(anchor) { export function findLinkFromClickTarget(target) { return findClosestRecursively(target, "a[href]:not([target^=_]):not([download])") } + +export function getLocationForLink(link) { + return expandURL(link.getAttribute("href") || "") +} \ No newline at end of file From de3ffcfc4d263b5e841ed0c34caf5ae419d2ab98 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 3 Dec 2023 15:07:17 -0600 Subject: [PATCH 04/42] Allow request to be intercepted and overriden on turbo:before-fetch-request --- src/http/fetch_request.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/http/fetch_request.js b/src/http/fetch_request.js index 3d79f24ae..6f5080c68 100644 --- a/src/http/fetch_request.js +++ b/src/http/fetch_request.js @@ -121,10 +121,10 @@ export class FetchRequest { async perform() { const { fetchOptions } = this this.delegate.prepareRequest(this) - await this.#allowRequestToBeIntercepted(fetchOptions) + const event = await this.#allowRequestToBeIntercepted(fetchOptions) try { this.delegate.requestStarted(this) - const response = await fetch(this.url.href, fetchOptions) + const response = await (event.detail.response || fetch(this.url.href, fetchOptions)) return await this.receive(response) } catch (error) { if (error.name !== "AbortError") { @@ -186,6 +186,8 @@ export class FetchRequest { }) this.url = event.detail.url if (event.defaultPrevented) await requestInterception + + return event } #willDelegateErrorHandling(error) { From 94ff0e1562213d63bfa5b577e367da3904c112d9 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Mon, 4 Dec 2023 21:42:51 -0700 Subject: [PATCH 05/42] Add instantclick behavior --- src/core/drive/prefetch_cache.js | 3 + src/core/session.js | 49 ++++++ src/observers/form_link_click_observer.js | 10 ++ .../link_prefetch_on_mouseover_observer.js | 111 +++++++++++++ src/tests/fixtures/hover_to_prefetch.html | 42 +++++ .../fixtures/hover_to_prefetch_disabled.html | 13 ++ .../fixtures/hover_to_prefetch_iframe.html | 13 ++ src/tests/fixtures/prefetched.html | 12 ++ ...nk_prefetch_on_mouseover_observer_tests.js | 153 ++++++++++++++++++ 9 files changed, 406 insertions(+) create mode 100644 src/core/drive/prefetch_cache.js create mode 100644 src/observers/link_prefetch_on_mouseover_observer.js create mode 100644 src/tests/fixtures/hover_to_prefetch.html create mode 100644 src/tests/fixtures/hover_to_prefetch_disabled.html create mode 100644 src/tests/fixtures/hover_to_prefetch_iframe.html create mode 100644 src/tests/fixtures/prefetched.html create mode 100644 src/tests/functional/link_prefetch_on_mouseover_observer_tests.js diff --git a/src/core/drive/prefetch_cache.js b/src/core/drive/prefetch_cache.js new file mode 100644 index 000000000..eaf33d4a9 --- /dev/null +++ b/src/core/drive/prefetch_cache.js @@ -0,0 +1,3 @@ +export const prefetchCache = new Map() + +export const cacheTtl = 10 * 1000 diff --git a/src/core/session.js b/src/core/session.js index be28a8287..620cbad4b 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -3,6 +3,7 @@ import { CacheObserver } from "../observers/cache_observer" import { FormSubmitObserver } from "../observers/form_submit_observer" import { FrameRedirector } from "./frames/frame_redirector" import { History } from "./drive/history" +import { LinkPrefetchOnMouseoverObserver } from "../observers/link_prefetch_on_mouseover_observer" import { LinkClickObserver } from "../observers/link_click_observer" import { FormLinkClickObserver } from "../observers/form_link_click_observer" import { getAction, expandURL, locationIsVisitable } from "./url" @@ -17,6 +18,7 @@ import { PageView } from "./drive/page_view" import { FrameElement } from "../elements/frame_element" import { Preloader } from "./drive/preloader" import { Cache } from "./cache" +import { prefetchCache, cacheTtl } from "./drive/prefetch_cache" export class Session { navigator = new Navigator(this) @@ -27,6 +29,7 @@ export class Session { pageObserver = new PageObserver(this) cacheObserver = new CacheObserver() + linkPrefetchOnMouseoverObserver = new LinkPrefetchOnMouseoverObserver(this, document) linkClickObserver = new LinkClickObserver(this, window) formSubmitObserver = new FormSubmitObserver(this, document) scrollObserver = new ScrollObserver(this) @@ -50,6 +53,7 @@ export class Session { if (!this.started) { this.pageObserver.start() this.cacheObserver.start() + this.linkPrefetchOnMouseoverObserver.start() this.formLinkClickObserver.start() this.linkClickObserver.start() this.formSubmitObserver.start() @@ -71,6 +75,7 @@ export class Session { if (this.started) { this.pageObserver.stop() this.cacheObserver.stop() + this.linkPrefetchOnMouseoverObserver.stop() this.formLinkClickObserver.stop() this.linkClickObserver.stop() this.formSubmitObserver.stop() @@ -166,6 +171,50 @@ export class Session { submittedFormLinkToLocation() {} + // Link hover observer delegate + + canPrefetchAndCacheRequestToLocation(link, location, event) { + const absoluteUrl = location.toString() + const cached = prefetchCache.get(absoluteUrl) + + return ( + this.elementIsNavigatable(link) && + locationIsVisitable(location, this.snapshot.rootLocation) && + (!cached || cached.ttl < new Date()) + ) + } + + prefetchAndCacheRequestToLocation(link, location) { + const requestOptions = { + credentials: "same-origin", + headers: { + "Sec-Purpose": "prefetch", + Accept: "text/html" + }, + redirect: "follow" + } + + if ( + link.dataset.turboFrame && + link.dataset.turboFrame !== "_top" + ) { + requestOptions.headers["Turbo-Frame"] = link.dataset.turboFrame + } else if (link.dataset.turboFrame !== "_top") { + const turboFrame = link.closest("turbo-frame") + + if (turboFrame) { + requestOptions.headers["Turbo-Frame"] = turboFrame.id + } + } + + const absoluteUrl = location.toString() + + prefetchCache.set(absoluteUrl, { + request: fetch(absoluteUrl, requestOptions), + ttl: new Date(new Date().getTime() + cacheTtl) + }) + } + // Link click observer delegate willFollowLinkToLocation(link, location, event) { diff --git a/src/observers/form_link_click_observer.js b/src/observers/form_link_click_observer.js index 55b94bb64..bf8d22e68 100644 --- a/src/observers/form_link_click_observer.js +++ b/src/observers/form_link_click_observer.js @@ -15,6 +15,16 @@ export class FormLinkClickObserver { this.linkInterceptor.stop() } + // Link hover observer delegate + + canPrefetchAndCacheRequestToLocation(link, location, event) { + return false + } + + prefetchAndCacheRequestToLocation(link, location) { + return + } + // Link click observer delegate willFollowLinkToLocation(link, location, originalEvent) { diff --git a/src/observers/link_prefetch_on_mouseover_observer.js b/src/observers/link_prefetch_on_mouseover_observer.js new file mode 100644 index 000000000..6bae75fb5 --- /dev/null +++ b/src/observers/link_prefetch_on_mouseover_observer.js @@ -0,0 +1,111 @@ +import { doesNotTargetIFrame, findLinkFromClickTarget, getLocationForLink, getMetaContent, findClosestRecursively } from "../util" +import { prefetchCache } from "../core/drive/prefetch_cache" + +export class LinkPrefetchOnMouseoverObserver { + started = false + + constructor(delegate, eventTarget) { + this.delegate = delegate + this.eventTarget = eventTarget + } + + start() { + if (this.started) return + + if (this.eventTarget.readyState === "loading") { + this.eventTarget.addEventListener("DOMContentLoaded", this._enable, { once: true }) + } else { + this._enable() + } + } + + stop() { + if (!this.started) return + + this.eventTarget.removeEventListener("mouseover", this.tryToPrefetchRequest, { + capture: true, + passive: true + }) + this.eventTarget.removeEventListener("turbo:before-fetch-request", this.tryToUsePrefetchedRequest, true) + this.started = false + } + + _enable = () => { + if (getMetaContent("turbo-prefetch") !== "true") return + + this.eventTarget.addEventListener("mouseover", this.tryToPrefetchRequest, { + capture: true, + passive: true + }) + this.eventTarget.addEventListener("turbo:before-fetch-request", this.tryToUsePrefetchedRequest, true) + this.started = true + } + + tryToPrefetchRequest = (event) => { + const target = event.target + const link = findLinkFromClickTarget(target) + + if (link && this.isPrefetchable(link)) { + const location = getLocationForLink(link) + if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { + this.delegate.prefetchAndCacheRequestToLocation(link, location) + } + } + } + + tryToUsePrefetchedRequest = (event) => { + if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") { + const cached = prefetchCache.get(event.detail.url.toString()) + + if (cached && cached.ttl > new Date()) { + event.detail.response = cached.request + } + } + + prefetchCache.clear() + } + + isPrefetchable(link) { + const href = link.getAttribute("href") + + if (!href || href === "#" || link.dataset.turbo === "false" || link.dataset.turboPrefetch === "false") { + return false + } + + if (link.origin !== document.location.origin) { + return false + } + + if (!["http:", "https:"].includes(link.protocol)) { + return false + } + + if (link.pathname + link.search === document.location.pathname + document.location.search) { + return false + } + + if (link.dataset.turboMethod && link.dataset.turboMethod !== "get") { + return false + } + + if (targetsIframe(link)) { + return false + } + + if (link.pathname + link.search === document.location.pathname + document.location.search) { + return false + } + + const turboPrefetchParent = findClosestRecursively(link, "[data-turbo-prefetch]") + + if (turboPrefetchParent && turboPrefetchParent.dataset.turboPrefetch === "false") { + return false + } + + return true + } +} + +const targetsIframe = (link) => { + return !doesNotTargetIFrame(link) +} diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html new file mode 100644 index 000000000..56f7e1009 --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -0,0 +1,42 @@ + + + + + Hover to Prefetch + + + + + + Hover to prefetch me +
+ Won't prefetch when hovering me +
+
+ Won't prefetch when hovering me +
+ Won't prefetch when hovering me + Won't prefetch when hovering me + Won't prefetch when hovering me + Hover to prefetch me + Won't prefetch when hovering me + Hover to prefetch me + Won't prefetch when hovering me + Won't prefetch when hovering me + Won't prefetch when hovering me + Won't prefetch when hovering me + + + diff --git a/src/tests/fixtures/hover_to_prefetch_disabled.html b/src/tests/fixtures/hover_to_prefetch_disabled.html new file mode 100644 index 000000000..3827f69e4 --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_disabled.html @@ -0,0 +1,13 @@ + + + + + Hover to Prefetch + + + + + + Hover to prefetch me + + diff --git a/src/tests/fixtures/hover_to_prefetch_iframe.html b/src/tests/fixtures/hover_to_prefetch_iframe.html new file mode 100644 index 000000000..046fefbd8 --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_iframe.html @@ -0,0 +1,13 @@ + + + + + Hover to Prefetch + + + + + + Hover to prefetch me + + diff --git a/src/tests/fixtures/prefetched.html b/src/tests/fixtures/prefetched.html new file mode 100644 index 000000000..492cd9bc8 --- /dev/null +++ b/src/tests/fixtures/prefetched.html @@ -0,0 +1,12 @@ + + + + + Prefetched Page + + + + + Prefetched Page Content + + diff --git a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js new file mode 100644 index 000000000..a30cdc73b --- /dev/null +++ b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js @@ -0,0 +1,153 @@ +import { test } from "@playwright/test" +import { assert } from "chai" +import { nextBeat } from "../helpers/page" + +test.describe("when hovering over a link", () => { + test.beforeEach(async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + }) + + test("it prefetches the page", async ({ page }) => { + await assertPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + }) + + test("it doesn't follow the link", async ({ page }) => { + await hoverSelector({ page, selector: "#prefetch_anchor" }) + + assert.equal(await page.title(), "Hover to Prefetch") + }) + + test.describe("when link has a whole valid url as a href", () => { + test("it prefetches the page", async ({ page }) => { + await assertPrefetchedOnMouseover({ page, selector: "#anchor_with_whole_url" }) + }) + }) + + test.describe("when link has the same location but with a query string", () => { + test("it prefetches the page", async ({ page }) => { + await assertPrefetchedOnMouseover({ page, selector: "#same_location_anchor_with_query" }) + }) + }) + + test.describe("when link is inside an element with data-turbo=false", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_false_parent_anchor" }) + }) + }) + + test.describe("when link is inside an element with data-turbo-prefetch=false", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_prefetch_false_parent_anchor" }) + }) + }) + + test.describe("when link has data-turbo-prefetch=false", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_prefetch_false_anchor" }) + }) + }) + + test.describe("when link has data-turbo=false", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_false_anchor" }) + }) + }) + + test.describe("when link has the same location as the current page", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#same_location_anchor" }) + }) + }) + + test.describe("when link has a different origin", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#different_origin_anchor" }) + }) + }) + + test.describe("when link has an hash as a href", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_hash" }) + }) + }) + + test.describe("when link has a ftp protocol", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_ftp_protocol" }) + }) + }) + + test.describe("when link is valid but it's inside an iframe", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_iframe_target" }) + }) + }) + + test.describe("when link has a POST data-turbo-method", () => { + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_post_method" }) + }) + }) + + test.describe("when turbo-prefetch meta tag is set to false", () => { + test.beforeEach(async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_disabled.html" }) + }) + + test("it doesn't prefetch the page", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + }) + }) +}) + +test.describe("when clicking on a link that has been prefetched", () => { + test.beforeEach(async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await hoverSelector({ page, selector: "#prefetch_anchor" }) + }) + + test("it does not make a network request", async ({ page }) => { + await assertNotPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + }) + + test("it follows the link using the cached response", async ({ page }) => { + await clickSelector({ page, selector: "#prefetch_anchor" }) + + assert.equal(await page.title(), "Prefetched Page") + }) +}) + +const assertPrefetchedOnMouseover = async ({ page, selector }) => { + let requestMade = false + + page.on("request", (request) => (requestMade = true)) + + await hoverSelector({ page, selector }) + + assert.equal(requestMade, true, "Network request wasn't made when it should have been.") +} + +const assertNotPrefetchedOnMouseover = async ({ page, selector }) => { + let requestMade = false + + page.on("request", (request) => (requestMade = true)) + + await hoverSelector({ page, selector }) + + assert.equal(requestMade, false, "Network request was made when it should not have been.") +} + +const goTo = async ({ page, path }) => { + await page.goto(`/src/tests/fixtures${path}`) + await nextBeat() +} + +const hoverSelector = async ({ page, selector }) => { + await page.hover(selector) + await nextBeat() +} + +const clickSelector = async ({ page, selector }) => { + await page.click(selector) + await nextBeat() +} From e662690cfde68ddc8300e9e59af0e1a7edbe71fd Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 17:28:02 -0700 Subject: [PATCH 06/42] Allow customizing the event that triggers prefetching --- .../link_prefetch_on_mouseover_observer.js | 20 ++++++- .../fixtures/hover_to_prefetch_mousedown.html | 14 +++++ ...nk_prefetch_on_mouseover_observer_tests.js | 60 +++++++++++++------ 3 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 src/tests/fixtures/hover_to_prefetch_mousedown.html diff --git a/src/observers/link_prefetch_on_mouseover_observer.js b/src/observers/link_prefetch_on_mouseover_observer.js index 6bae75fb5..a9c1e65de 100644 --- a/src/observers/link_prefetch_on_mouseover_observer.js +++ b/src/observers/link_prefetch_on_mouseover_observer.js @@ -1,7 +1,17 @@ -import { doesNotTargetIFrame, findLinkFromClickTarget, getLocationForLink, getMetaContent, findClosestRecursively } from "../util" +import { + doesNotTargetIFrame, + findLinkFromClickTarget, + getLocationForLink, + getMetaContent, + findClosestRecursively +} from "../util" import { prefetchCache } from "../core/drive/prefetch_cache" export class LinkPrefetchOnMouseoverObserver { + triggerEvents = { + mouseover: "mouseover", + mousedown: "mousedown" + } started = false constructor(delegate, eventTarget) { @@ -22,7 +32,7 @@ export class LinkPrefetchOnMouseoverObserver { stop() { if (!this.started) return - this.eventTarget.removeEventListener("mouseover", this.tryToPrefetchRequest, { + this.eventTarget.removeEventListener(this.triggerEvent, this.tryToPrefetchRequest, { capture: true, passive: true }) @@ -30,10 +40,14 @@ export class LinkPrefetchOnMouseoverObserver { this.started = false } + get triggerEvent() { + return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover + } + _enable = () => { if (getMetaContent("turbo-prefetch") !== "true") return - this.eventTarget.addEventListener("mouseover", this.tryToPrefetchRequest, { + this.eventTarget.addEventListener(this.triggerEvent, this.tryToPrefetchRequest, { capture: true, passive: true }) diff --git a/src/tests/fixtures/hover_to_prefetch_mousedown.html b/src/tests/fixtures/hover_to_prefetch_mousedown.html new file mode 100644 index 000000000..0a4c3a7dd --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_mousedown.html @@ -0,0 +1,14 @@ + + + + + Hover to Prefetch + + + + + + + Hover to prefetch me + + diff --git a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js index a30cdc73b..ac21b11db 100644 --- a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js +++ b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js @@ -8,7 +8,7 @@ test.describe("when hovering over a link", () => { }) test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) }) test("it doesn't follow the link", async ({ page }) => { @@ -19,73 +19,73 @@ test.describe("when hovering over a link", () => { test.describe("when link has a whole valid url as a href", () => { test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnMouseover({ page, selector: "#anchor_with_whole_url" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_with_whole_url" }) }) }) test.describe("when link has the same location but with a query string", () => { test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnMouseover({ page, selector: "#same_location_anchor_with_query" }) + await assertPrefetchedOnHover({ page, selector: "#same_location_anchor_with_query" }) }) }) test.describe("when link is inside an element with data-turbo=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_false_parent_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#turbo_false_parent_anchor" }) }) }) test.describe("when link is inside an element with data-turbo-prefetch=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_prefetch_false_parent_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#turbo_prefetch_false_parent_anchor" }) }) }) test.describe("when link has data-turbo-prefetch=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_prefetch_false_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#turbo_prefetch_false_anchor" }) }) }) test.describe("when link has data-turbo=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#turbo_false_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#turbo_false_anchor" }) }) }) test.describe("when link has the same location as the current page", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#same_location_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#same_location_anchor" }) }) }) test.describe("when link has a different origin", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#different_origin_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#different_origin_anchor" }) }) }) test.describe("when link has an hash as a href", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_hash" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_hash" }) }) }) test.describe("when link has a ftp protocol", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_ftp_protocol" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_ftp_protocol" }) }) }) test.describe("when link is valid but it's inside an iframe", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_iframe_target" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_iframe_target" }) }) }) test.describe("when link has a POST data-turbo-method", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#anchor_with_post_method" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_post_method" }) }) }) @@ -95,7 +95,21 @@ test.describe("when hovering over a link", () => { }) test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + }) + }) + + test.describe("when turbo-prefetch-trigger-event is set to mousedown", () => { + test.beforeEach(async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) + }) + + test("it prefetches the page on mousedown", async ({ page }) => { + await assertPrefetchedOnMouseDown({ page, selector: "#prefetch_anchor" }) + }) + + test("it doesn't prefetch the page on mouseover", async ({ page }) => { + await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) }) }) }) @@ -107,7 +121,7 @@ test.describe("when clicking on a link that has been prefetched", () => { }) test("it does not make a network request", async ({ page }) => { - await assertNotPrefetchedOnMouseover({ page, selector: "#prefetch_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) }) test("it follows the link using the cached response", async ({ page }) => { @@ -117,7 +131,7 @@ test.describe("when clicking on a link that has been prefetched", () => { }) }) -const assertPrefetchedOnMouseover = async ({ page, selector }) => { +const assertPrefetchedOnHover = async ({ page, selector }) => { let requestMade = false page.on("request", (request) => (requestMade = true)) @@ -127,7 +141,7 @@ const assertPrefetchedOnMouseover = async ({ page, selector }) => { assert.equal(requestMade, true, "Network request wasn't made when it should have been.") } -const assertNotPrefetchedOnMouseover = async ({ page, selector }) => { +const assertNotPrefetchedOnHover = async ({ page, selector }) => { let requestMade = false page.on("request", (request) => (requestMade = true)) @@ -137,6 +151,18 @@ const assertNotPrefetchedOnMouseover = async ({ page, selector }) => { assert.equal(requestMade, false, "Network request was made when it should not have been.") } +const assertPrefetchedOnMouseDown = async ({ page, selector }) => { + let requestMade = false + + page.on("request", (request) => (requestMade = true)) + + await page.hover(selector) + await page.mouse.down() + await nextBeat() + + assert.equal(requestMade, true, "Network request wasn't made when it should have been.") +} + const goTo = async ({ page, path }) => { await page.goto(`/src/tests/fixtures${path}`) await nextBeat() From 15517d4045ee26151207410b8f13244b36288e00 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 19:10:18 -0700 Subject: [PATCH 07/42] Allow customizing the cache time for prefetching --- src/core/session.js | 4 ++-- .../link_prefetch_on_mouseover_observer.js | 8 +++++-- .../hover_to_prefetch_custom_cache_time.html | 14 +++++++++++++ ...nk_prefetch_on_mouseover_observer_tests.js | 21 ++++++++++++++++++- 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 src/tests/fixtures/hover_to_prefetch_custom_cache_time.html diff --git a/src/core/session.js b/src/core/session.js index 620cbad4b..0b33f79be 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -18,7 +18,7 @@ import { PageView } from "./drive/page_view" import { FrameElement } from "../elements/frame_element" import { Preloader } from "./drive/preloader" import { Cache } from "./cache" -import { prefetchCache, cacheTtl } from "./drive/prefetch_cache" +import { prefetchCache } from "./drive/prefetch_cache" export class Session { navigator = new Navigator(this) @@ -184,7 +184,7 @@ export class Session { ) } - prefetchAndCacheRequestToLocation(link, location) { + prefetchAndCacheRequestToLocation(link, location, cacheTtl) { const requestOptions = { credentials: "same-origin", headers: { diff --git a/src/observers/link_prefetch_on_mouseover_observer.js b/src/observers/link_prefetch_on_mouseover_observer.js index a9c1e65de..63efb298c 100644 --- a/src/observers/link_prefetch_on_mouseover_observer.js +++ b/src/observers/link_prefetch_on_mouseover_observer.js @@ -5,7 +5,7 @@ import { getMetaContent, findClosestRecursively } from "../util" -import { prefetchCache } from "../core/drive/prefetch_cache" +import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" export class LinkPrefetchOnMouseoverObserver { triggerEvents = { @@ -44,6 +44,10 @@ export class LinkPrefetchOnMouseoverObserver { return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover } + get cacheTtl() { + return Number(getMetaContent("turbo-prefetch-cache-time")) || cacheTtl + } + _enable = () => { if (getMetaContent("turbo-prefetch") !== "true") return @@ -62,7 +66,7 @@ export class LinkPrefetchOnMouseoverObserver { if (link && this.isPrefetchable(link)) { const location = getLocationForLink(link) if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { - this.delegate.prefetchAndCacheRequestToLocation(link, location) + this.delegate.prefetchAndCacheRequestToLocation(link, location, this.cacheTtl) } } } diff --git a/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html b/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html new file mode 100644 index 000000000..b17178eac --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html @@ -0,0 +1,14 @@ + + + + + Hover to Prefetch + + + + + + + Hover to prefetch me + + diff --git a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js index ac21b11db..b17b39066 100644 --- a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js +++ b/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js @@ -1,6 +1,6 @@ import { test } from "@playwright/test" import { assert } from "chai" -import { nextBeat } from "../helpers/page" +import { nextBeat, sleep } from "../helpers/page" test.describe("when hovering over a link", () => { test.beforeEach(async ({ page }) => { @@ -112,6 +112,25 @@ test.describe("when hovering over a link", () => { await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) }) }) + + test.describe("when turbo-prefetch-cache-time is set to 1", () => { + test.beforeEach(async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) + }) + + test("it prefetches the page", async ({ page }) => { + await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + }) + + test("it caches the request for 1 second", async ({ page }) => { + await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + + await sleep(1100) + await page.mouse.move(0, 0) + + await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + }) + }) }) test.describe("when clicking on a link that has been prefetched", () => { From ab0e5389aad7f5cddb49ad99205eef5aa044d02a Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 19:12:07 -0700 Subject: [PATCH 08/42] Rename LinkPrefetchOnMouseoverObserver to LinkPrefetchObserver Because it is not only triggered on mouseover, but could also be on mousedown, or eventually touchstart. --- src/core/session.js | 8 ++++---- ...on_mouseover_observer.js => link_prefetch_observer.js} | 2 +- ..._observer_tests.js => link_prefetch_observer_tests.js} | 0 3 files changed, 5 insertions(+), 5 deletions(-) rename src/observers/{link_prefetch_on_mouseover_observer.js => link_prefetch_observer.js} (98%) rename src/tests/functional/{link_prefetch_on_mouseover_observer_tests.js => link_prefetch_observer_tests.js} (100%) diff --git a/src/core/session.js b/src/core/session.js index 0b33f79be..c068333d9 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -3,7 +3,7 @@ import { CacheObserver } from "../observers/cache_observer" import { FormSubmitObserver } from "../observers/form_submit_observer" import { FrameRedirector } from "./frames/frame_redirector" import { History } from "./drive/history" -import { LinkPrefetchOnMouseoverObserver } from "../observers/link_prefetch_on_mouseover_observer" +import { LinkPrefetchObserver } from "../observers/link_prefetch_observer" import { LinkClickObserver } from "../observers/link_click_observer" import { FormLinkClickObserver } from "../observers/form_link_click_observer" import { getAction, expandURL, locationIsVisitable } from "./url" @@ -29,7 +29,7 @@ export class Session { pageObserver = new PageObserver(this) cacheObserver = new CacheObserver() - linkPrefetchOnMouseoverObserver = new LinkPrefetchOnMouseoverObserver(this, document) + linkPrefetchObserver = new LinkPrefetchObserver(this, document) linkClickObserver = new LinkClickObserver(this, window) formSubmitObserver = new FormSubmitObserver(this, document) scrollObserver = new ScrollObserver(this) @@ -53,7 +53,7 @@ export class Session { if (!this.started) { this.pageObserver.start() this.cacheObserver.start() - this.linkPrefetchOnMouseoverObserver.start() + this.linkPrefetchObserver.start() this.formLinkClickObserver.start() this.linkClickObserver.start() this.formSubmitObserver.start() @@ -75,7 +75,7 @@ export class Session { if (this.started) { this.pageObserver.stop() this.cacheObserver.stop() - this.linkPrefetchOnMouseoverObserver.stop() + this.linkPrefetchObserver.stop() this.formLinkClickObserver.stop() this.linkClickObserver.stop() this.formSubmitObserver.stop() diff --git a/src/observers/link_prefetch_on_mouseover_observer.js b/src/observers/link_prefetch_observer.js similarity index 98% rename from src/observers/link_prefetch_on_mouseover_observer.js rename to src/observers/link_prefetch_observer.js index 63efb298c..d712e8e25 100644 --- a/src/observers/link_prefetch_on_mouseover_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -7,7 +7,7 @@ import { } from "../util" import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" -export class LinkPrefetchOnMouseoverObserver { +export class LinkPrefetchObserver { triggerEvents = { mouseover: "mouseover", mousedown: "mousedown" diff --git a/src/tests/functional/link_prefetch_on_mouseover_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js similarity index 100% rename from src/tests/functional/link_prefetch_on_mouseover_observer_tests.js rename to src/tests/functional/link_prefetch_observer_tests.js From fca147e5e9145f429a02f9bd03e444c08d9da42b Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 19:15:34 -0700 Subject: [PATCH 09/42] Use private methods in LinkPrefetchObserver --- src/observers/link_prefetch_observer.js | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index d712e8e25..1054d0185 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -23,55 +23,55 @@ export class LinkPrefetchObserver { if (this.started) return if (this.eventTarget.readyState === "loading") { - this.eventTarget.addEventListener("DOMContentLoaded", this._enable, { once: true }) + this.eventTarget.addEventListener("DOMContentLoaded", this.#enable, { once: true }) } else { - this._enable() + this.#enable() } } stop() { if (!this.started) return - this.eventTarget.removeEventListener(this.triggerEvent, this.tryToPrefetchRequest, { + this.eventTarget.removeEventListener(this.#triggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) - this.eventTarget.removeEventListener("turbo:before-fetch-request", this.tryToUsePrefetchedRequest, true) + this.eventTarget.removeEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true) this.started = false } - get triggerEvent() { + get #triggerEvent() { return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover } - get cacheTtl() { + get #cacheTtl() { return Number(getMetaContent("turbo-prefetch-cache-time")) || cacheTtl } - _enable = () => { + #enable = () => { if (getMetaContent("turbo-prefetch") !== "true") return - this.eventTarget.addEventListener(this.triggerEvent, this.tryToPrefetchRequest, { + this.eventTarget.addEventListener(this.#triggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) - this.eventTarget.addEventListener("turbo:before-fetch-request", this.tryToUsePrefetchedRequest, true) + this.eventTarget.addEventListener("turbo:before-fetch-request", this.#tryToUsePrefetchedRequest, true) this.started = true } - tryToPrefetchRequest = (event) => { + #tryToPrefetchRequest = (event) => { const target = event.target const link = findLinkFromClickTarget(target) - if (link && this.isPrefetchable(link)) { + if (link && this.#isPrefetchable(link)) { const location = getLocationForLink(link) if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { - this.delegate.prefetchAndCacheRequestToLocation(link, location, this.cacheTtl) + this.delegate.prefetchAndCacheRequestToLocation(link, location, this.#cacheTtl) } } } - tryToUsePrefetchedRequest = (event) => { + #tryToUsePrefetchedRequest = (event) => { if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") { const cached = prefetchCache.get(event.detail.url.toString()) @@ -83,7 +83,7 @@ export class LinkPrefetchObserver { prefetchCache.clear() } - isPrefetchable(link) { + #isPrefetchable(link) { const href = link.getAttribute("href") if (!href || href === "#" || link.dataset.turbo === "false" || link.dataset.turboPrefetch === "false") { From 3baf81cbd9cc05039e6083ea055f86cc07ed70f1 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 19:17:12 -0700 Subject: [PATCH 10/42] Reorganize methods on LinkPrefetchObserver --- src/observers/link_prefetch_observer.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 1054d0185..261b36d1d 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -40,14 +40,6 @@ export class LinkPrefetchObserver { this.started = false } - get #triggerEvent() { - return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover - } - - get #cacheTtl() { - return Number(getMetaContent("turbo-prefetch-cache-time")) || cacheTtl - } - #enable = () => { if (getMetaContent("turbo-prefetch") !== "true") return @@ -83,6 +75,14 @@ export class LinkPrefetchObserver { prefetchCache.clear() } + get #triggerEvent() { + return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover + } + + get #cacheTtl() { + return Number(getMetaContent("turbo-prefetch-cache-time")) || cacheTtl + } + #isPrefetchable(link) { const href = link.getAttribute("href") From d2ec0211274106180996a0dd2e1ed210001a268a Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 19:41:37 -0700 Subject: [PATCH 11/42] Require a shorter sleep time in the test Since turbo-prefetch-cache-time is set to 1 millisecond in the html fixture --- src/tests/functional/link_prefetch_observer_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index b17b39066..c27221a7f 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -125,7 +125,7 @@ test.describe("when hovering over a link", () => { test("it caches the request for 1 second", async ({ page }) => { await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) - await sleep(1100) + await sleep(10) await page.mouse.move(0, 0) await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) From 7c7ded9b0ef111bf789ede518c60900656055e91 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 5 Dec 2023 22:46:18 -0700 Subject: [PATCH 12/42] Standardize anchor IDs in link_prefetch_observer_tests anchor_ prefix is used for all anchors in the tests --- src/tests/fixtures/hover_to_prefetch.html | 22 +++++++----- .../hover_to_prefetch_custom_cache_time.html | 2 +- .../fixtures/hover_to_prefetch_disabled.html | 2 +- .../fixtures/hover_to_prefetch_mousedown.html | 2 +- .../link_prefetch_observer_tests.js | 36 +++++++++---------- 5 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index 56f7e1009..990e96a1b 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -8,24 +8,30 @@ - Hover to prefetch me + Hover to prefetch me
- Won't prefetch when hovering me + Won't prefetch when hovering me
- Won't prefetch when hovering me
- Won't prefetch when hovering me + Won't prefetch when hovering me - Won't prefetch when hovering me - Won't prefetch when hovering me - Hover to prefetch me - Won't prefetch when hovering me + Hover to prefetch me + Won't prefetch when hovering me Hover to prefetch me diff --git a/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html b/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html index b17178eac..00ed35fd2 100644 --- a/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html +++ b/src/tests/fixtures/hover_to_prefetch_custom_cache_time.html @@ -9,6 +9,6 @@ - Hover to prefetch me + Hover to prefetch me diff --git a/src/tests/fixtures/hover_to_prefetch_disabled.html b/src/tests/fixtures/hover_to_prefetch_disabled.html index 3827f69e4..689041d39 100644 --- a/src/tests/fixtures/hover_to_prefetch_disabled.html +++ b/src/tests/fixtures/hover_to_prefetch_disabled.html @@ -8,6 +8,6 @@ - Hover to prefetch me + Hover to prefetch me diff --git a/src/tests/fixtures/hover_to_prefetch_mousedown.html b/src/tests/fixtures/hover_to_prefetch_mousedown.html index 0a4c3a7dd..0dd5668d5 100644 --- a/src/tests/fixtures/hover_to_prefetch_mousedown.html +++ b/src/tests/fixtures/hover_to_prefetch_mousedown.html @@ -9,6 +9,6 @@ - Hover to prefetch me + Hover to prefetch me diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index c27221a7f..5caf329d4 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -8,11 +8,11 @@ test.describe("when hovering over a link", () => { }) test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) test("it doesn't follow the link", async ({ page }) => { - await hoverSelector({ page, selector: "#prefetch_anchor" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) assert.equal(await page.title(), "Hover to Prefetch") }) @@ -25,43 +25,43 @@ test.describe("when hovering over a link", () => { test.describe("when link has the same location but with a query string", () => { test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#same_location_anchor_with_query" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_same_location_with_query" }) }) }) test.describe("when link is inside an element with data-turbo=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#turbo_false_parent_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false_parent" }) }) }) test.describe("when link is inside an element with data-turbo-prefetch=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#turbo_prefetch_false_parent_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false_parent" }) }) }) test.describe("when link has data-turbo-prefetch=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#turbo_prefetch_false_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false" }) }) }) test.describe("when link has data-turbo=false", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#turbo_false_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false" }) }) }) test.describe("when link has the same location as the current page", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#same_location_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_same_location" }) }) }) test.describe("when link has a different origin", () => { test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#different_origin_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_different_origin" }) }) }) @@ -95,7 +95,7 @@ test.describe("when hovering over a link", () => { }) test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) }) @@ -105,11 +105,11 @@ test.describe("when hovering over a link", () => { }) test("it prefetches the page on mousedown", async ({ page }) => { - await assertPrefetchedOnMouseDown({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) }) test("it doesn't prefetch the page on mouseover", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) }) @@ -119,16 +119,16 @@ test.describe("when hovering over a link", () => { }) test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) test("it caches the request for 1 second", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) await sleep(10) await page.mouse.move(0, 0) - await assertPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) }) }) @@ -136,15 +136,15 @@ test.describe("when hovering over a link", () => { test.describe("when clicking on a link that has been prefetched", () => { test.beforeEach(async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) - await hoverSelector({ page, selector: "#prefetch_anchor" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) }) test("it does not make a network request", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#prefetch_anchor" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) test("it follows the link using the cached response", async ({ page }) => { - await clickSelector({ page, selector: "#prefetch_anchor" }) + await clickSelector({ page, selector: "#anchor_for_prefetch" }) assert.equal(await page.title(), "Prefetched Page") }) From 806aa9d07585ddaf713b8a1f5642a5cadac17bac Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Fri, 8 Dec 2023 21:43:56 -0700 Subject: [PATCH 13/42] Don't try traverse DOM to determine if the target is a link This is not necessary, since we can just check if the target is an anchor element with an href attribute. We were just using findLinkFromClickTarget because it had the selector we needed, but we can just use the selector directly. --- src/observers/link_prefetch_observer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 261b36d1d..473d3c95d 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -1,6 +1,5 @@ import { doesNotTargetIFrame, - findLinkFromClickTarget, getLocationForLink, getMetaContent, findClosestRecursively @@ -53,9 +52,10 @@ export class LinkPrefetchObserver { #tryToPrefetchRequest = (event) => { const target = event.target - const link = findLinkFromClickTarget(target) + const isLink = target.matches("a[href]:not([target^=_]):not([download])") + const link = target - if (link && this.#isPrefetchable(link)) { + if (isLink && this.#isPrefetchable(link)) { const location = getLocationForLink(link) if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { this.delegate.prefetchAndCacheRequestToLocation(link, location, this.#cacheTtl) From 39e3ddad49f16adc7cc6097f8ba20e1ec8af6163 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Fri, 8 Dec 2023 21:47:58 -0700 Subject: [PATCH 14/42] Keep the closing tag on the same line as the rest of the tag --- src/tests/fixtures/hover_to_prefetch.html | 29 +++++++---------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index 990e96a1b..a95e8a7d0 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -10,39 +10,28 @@ Hover to prefetch me
- Won't prefetch when hovering me + Won't prefetch when hovering me
- Won't prefetch when hovering me + Won't prefetch when hovering me
Won't prefetch when hovering me + >Won't prefetch when hovering me Won't prefetch when hovering me + >Won't prefetch when hovering me Won't prefetch when hovering me + >Won't prefetch when hovering me Hover to prefetch me + >Hover to prefetch me Won't prefetch when hovering me Hover to prefetch me + >Hover to prefetch me Won't prefetch when hovering me Won't prefetch when hovering me Won't prefetch when hovering me + >Won't prefetch when hovering me Won't prefetch when hovering me + >Won't prefetch when hovering me From 9cdff6de913bedd254ed2cd23a958baaba699159 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Fri, 8 Dec 2023 21:56:33 -0700 Subject: [PATCH 15/42] Remove unnecessary nesting in tests --- .../link_prefetch_observer_tests.js | 265 ++++++++---------- 1 file changed, 119 insertions(+), 146 deletions(-) diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 5caf329d4..35fb29d92 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -2,152 +2,125 @@ import { test } from "@playwright/test" import { assert } from "chai" import { nextBeat, sleep } from "../helpers/page" -test.describe("when hovering over a link", () => { - test.beforeEach(async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch.html" }) - }) - - test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - - test("it doesn't follow the link", async ({ page }) => { - await hoverSelector({ page, selector: "#anchor_for_prefetch" }) - - assert.equal(await page.title(), "Hover to Prefetch") - }) - - test.describe("when link has a whole valid url as a href", () => { - test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#anchor_with_whole_url" }) - }) - }) - - test.describe("when link has the same location but with a query string", () => { - test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#anchor_for_same_location_with_query" }) - }) - }) - - test.describe("when link is inside an element with data-turbo=false", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false_parent" }) - }) - }) - - test.describe("when link is inside an element with data-turbo-prefetch=false", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false_parent" }) - }) - }) - - test.describe("when link has data-turbo-prefetch=false", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false" }) - }) - }) - - test.describe("when link has data-turbo=false", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false" }) - }) - }) - - test.describe("when link has the same location as the current page", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_same_location" }) - }) - }) - - test.describe("when link has a different origin", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_different_origin" }) - }) - }) - - test.describe("when link has an hash as a href", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_hash" }) - }) - }) - - test.describe("when link has a ftp protocol", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_ftp_protocol" }) - }) - }) - - test.describe("when link is valid but it's inside an iframe", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_iframe_target" }) - }) - }) - - test.describe("when link has a POST data-turbo-method", () => { - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_post_method" }) - }) - }) - - test.describe("when turbo-prefetch meta tag is set to false", () => { - test.beforeEach(async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_disabled.html" }) - }) - - test("it doesn't prefetch the page", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - }) - - test.describe("when turbo-prefetch-trigger-event is set to mousedown", () => { - test.beforeEach(async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) - }) - - test("it prefetches the page on mousedown", async ({ page }) => { - await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) - }) - - test("it doesn't prefetch the page on mouseover", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - }) - - test.describe("when turbo-prefetch-cache-time is set to 1", () => { - test.beforeEach(async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) - }) - - test("it prefetches the page", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - - test("it caches the request for 1 second", async ({ page }) => { - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - - await sleep(10) - await page.mouse.move(0, 0) - - await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - }) -}) - -test.describe("when clicking on a link that has been prefetched", () => { - test.beforeEach(async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch.html" }) - await hoverSelector({ page, selector: "#anchor_for_prefetch" }) - }) - - test("it does not make a network request", async ({ page }) => { - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) - }) - - test("it follows the link using the cached response", async ({ page }) => { - await clickSelector({ page, selector: "#anchor_for_prefetch" }) - - assert.equal(await page.title(), "Prefetched Page") - }) +test("it prefetches the page", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it doesn't follow the link", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + + assert.equal(await page.title(), "Hover to Prefetch") +}) + +test("prefetches the page when link has a whole valid url as a href", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_with_whole_url" }) +}) + +test("it prefetches the page when link has the same location but with a query string", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_same_location_with_query" }) +}) + +test("it doesn't prefetch the page when link is inside an element with data-turbo=false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false_parent" }) +}) + +test("it doesn't prefetch the page when link is inside an element with data-turbo-prefetch=false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false_parent" }) +}) + +test("it doesn't prefetch the page when link has data-turbo-prefetch=false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false" }) +}) + +test("it doesn't prefetch the page when link has data-turbo=false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_false" }) +}) + +test("it doesn't prefetch the page when link has the same location", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_same_location" }) +}) + +test("it doesn't prefetch the page when link has a different origin", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_different_origin" }) +}) + +test("it doesn't prefetch the page when link has a hash as a href", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_hash" }) +}) + +test("it doesn't prefetch the page when link has a ftp protocol", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_ftp_protocol" }) +}) + +test("it doesn't prefetch the page when links is valid but it's inside an iframe", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_iframe_target" }) +}) + +test("it doesn't prefetch the page when link has a POST data-turbo-method", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_post_method" }) +}) + +test("it doesn't prefetch the page when turbo-prefetch meta tag is set to false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_disabled.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it prefetches the page on mousedown when turbo-prefetch-trigger-event is set to mousedown", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) + await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it doesn't prefetch the page on mouseover when turbo-prefetch-trigger-event is set to mousedown", async ({ + page +}) => { + await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it prefetches the page when turbo-prefetch-cache-time is set to 1", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it caches the request for 1 second when turbo-prefetch-cache-time is set to 1", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + + await sleep(10) + await page.mouse.move(0, 0) + + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it follows the link using the cached response when clicking on a link that has been prefetched", async ({ + page +}) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + + await clickSelector({ page, selector: "#anchor_for_prefetch" }) + assert.equal(await page.title(), "Prefetched Page") }) const assertPrefetchedOnHover = async ({ page, selector }) => { From 368225a20ab6ba206a8064acaa7d6385477fc09f Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Fri, 8 Dec 2023 21:57:06 -0700 Subject: [PATCH 16/42] Add missing newline at end of file --- src/observers/link_click_observer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/observers/link_click_observer.js b/src/observers/link_click_observer.js index 29148ad3d..24c6aa235 100644 --- a/src/observers/link_click_observer.js +++ b/src/observers/link_click_observer.js @@ -52,4 +52,4 @@ export class LinkClickObserver { event.shiftKey ) } -} \ No newline at end of file +} From 3e75161107b1e18e0353931216f6e614b1fb894e Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Fri, 8 Dec 2023 22:09:22 -0700 Subject: [PATCH 17/42] Check for prefetch meta tag before prefetching (on hover event) --- src/observers/link_prefetch_observer.js | 2 ++ .../functional/link_prefetch_observer_tests.js | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 473d3c95d..068c963a3 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -51,6 +51,8 @@ export class LinkPrefetchObserver { } #tryToPrefetchRequest = (event) => { + if (getMetaContent("turbo-prefetch") !== "true") return + const target = event.target const isLink = target.matches("a[href]:not([target^=_]):not([download])") const link = target diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 35fb29d92..2de059338 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -79,6 +79,23 @@ test("it doesn't prefetch the page when turbo-prefetch meta tag is set to false" await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) +test("it doesn't prefetch the page when turbo-prefetch meta tag is set to true, but is later set to false", async ({ + page +}) => { + await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) + + await page.evaluate(() => { + const meta = document.querySelector('meta[name="turbo-prefetch"]') + meta.setAttribute("content", "false") + }) + + await sleep(10) + await page.mouse.move(0, 0) + + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + test("it prefetches the page on mousedown when turbo-prefetch-trigger-event is set to mousedown", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) From b9e82f240b31381db6b8f526425aa9238f695a45 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 00:07:47 -0700 Subject: [PATCH 18/42] Use FetchRequest to build request for LinkPrefetchObserver * LinkPrefetchObserver implements the FetchRequest interface, so it can be used to build a request. * It also adds this.response to FetchRequest to store the non-awaited `fetch` response, because we need to FetchRequest#receive() a `fetch` response, not a FetchRequest. --- src/core/session.js | 34 ++++++++----------------- src/http/fetch_request.js | 9 ++++++- src/observers/link_prefetch_observer.js | 32 ++++++++++++++++++++++- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/core/session.js b/src/core/session.js index c068333d9..1dca062ef 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -19,6 +19,7 @@ import { FrameElement } from "../elements/frame_element" import { Preloader } from "./drive/preloader" import { Cache } from "./cache" import { prefetchCache } from "./drive/prefetch_cache" +import { FetchMethod, FetchRequest } from "../http/fetch_request" export class Session { navigator = new Navigator(this) @@ -185,32 +186,19 @@ export class Session { } prefetchAndCacheRequestToLocation(link, location, cacheTtl) { - const requestOptions = { - credentials: "same-origin", - headers: { - "Sec-Purpose": "prefetch", - Accept: "text/html" - }, - redirect: "follow" - } - - if ( - link.dataset.turboFrame && - link.dataset.turboFrame !== "_top" - ) { - requestOptions.headers["Turbo-Frame"] = link.dataset.turboFrame - } else if (link.dataset.turboFrame !== "_top") { - const turboFrame = link.closest("turbo-frame") - - if (turboFrame) { - requestOptions.headers["Turbo-Frame"] = turboFrame.id - } - } - const absoluteUrl = location.toString() + const fetchRequest = new FetchRequest( + this.linkPrefetchObserver, + FetchMethod.get, + location, + new URLSearchParams(), + link + ) + + fetchRequest.perform() prefetchCache.set(absoluteUrl, { - request: fetch(absoluteUrl, requestOptions), + fetchRequest, ttl: new Date(new Date().getTime() + cacheTtl) }) } diff --git a/src/http/fetch_request.js b/src/http/fetch_request.js index 6f5080c68..f4ff5adbe 100644 --- a/src/http/fetch_request.js +++ b/src/http/fetch_request.js @@ -124,7 +124,14 @@ export class FetchRequest { const event = await this.#allowRequestToBeIntercepted(fetchOptions) try { this.delegate.requestStarted(this) - const response = await (event.detail.response || fetch(this.url.href, fetchOptions)) + + if (event.detail.fetchRequest) { + this.response = event.detail.fetchRequest.response + } else { + this.response = fetch(this.url.href, fetchOptions) + } + + const response = await this.response return await this.receive(response) } catch (error) { if (error.name !== "AbortError") { diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 068c963a3..97800d9ed 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -70,13 +70,43 @@ export class LinkPrefetchObserver { const cached = prefetchCache.get(event.detail.url.toString()) if (cached && cached.ttl > new Date()) { - event.detail.response = cached.request + event.detail.fetchRequest = cached.fetchRequest } } prefetchCache.clear() } + prepareRequest(request) { + const link = request.target + + request.headers["Sec-Purpose"] = "prefetch" + + if (link.dataset.turboFrame && link.dataset.turboFrame !== "_top") { + request.headers["Turbo-Frame"] = link.dataset.turboFrame + } else if (link.dataset.turboFrame !== "_top") { + const turboFrame = link.closest("turbo-frame") + + if (turboFrame) { + request.headers["Turbo-Frame"] = turboFrame.id + } + } + } + + // Fetch request interface + + requestSucceededWithResponse() {} + + requestStarted(fetchRequest) {} + + requestErrored(fetchRequest) {} + + requestFinished(fetchRequest) {} + + requestPreventedHandlingResponse(fetchRequest, fetchResponse) {} + + requestFailedWithResponse(fetchRequest, fetchResponse) {} + get #triggerEvent() { return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover } From ab010993eed00275bb9c2d500b0cf7f05bf9f83d Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 00:25:40 -0700 Subject: [PATCH 19/42] Add Turbo Stream header to Accept header when link has data-turbo-stream --- src/observers/link_prefetch_observer.js | 6 +++++ src/tests/fixtures/hover_to_prefetch.html | 1 + .../link_prefetch_observer_tests.js | 25 ++++++++++++++++--- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 97800d9ed..972be9f86 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -91,6 +91,12 @@ export class LinkPrefetchObserver { request.headers["Turbo-Frame"] = turboFrame.id } } + + if (link.hasAttribute("data-turbo-stream")) { + // Add text/vnd.turbo-stream.html to Accept header, apart from the + // ones that the request already has. + request.acceptResponseType("text/vnd.turbo-stream.html") + } } // Fetch request interface diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index a95e8a7d0..ac045d6d0 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -9,6 +9,7 @@ Hover to prefetch me + Hover to prefetch me
Won't prefetch when hovering me
diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 2de059338..e2aeb9f29 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -123,6 +123,17 @@ test("it caches the request for 1 second when turbo-prefetch-cache-time is set t await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) +test("it adds text/vnd.turbo-stream.html header to the Accept header when link has data-turbo-stream", async ({ + page +}) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_with_turbo_stream", callback: (request) => { + const headers = request.headers()["accept"].split(",").map((header) => header.trim()) + + assert.includeMembers(headers, ["text/vnd.turbo-stream.html", "text/html", "application/xhtml+xml"]) + }}) +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) @@ -140,20 +151,26 @@ test("it follows the link using the cached response when clicking on a link that assert.equal(await page.title(), "Prefetched Page") }) -const assertPrefetchedOnHover = async ({ page, selector }) => { +const assertPrefetchedOnHover = async ({ page, selector, callback }) => { let requestMade = false - page.on("request", (request) => (requestMade = true)) + page.on("request", (request) => { + callback && callback(request) + requestMade = true + }) await hoverSelector({ page, selector }) assert.equal(requestMade, true, "Network request wasn't made when it should have been.") } -const assertNotPrefetchedOnHover = async ({ page, selector }) => { +const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { let requestMade = false - page.on("request", (request) => (requestMade = true)) + page.on("request", (request) => { + callback && callback(request) + requestMade = true + }) await hoverSelector({ page, selector }) From c735036118536b0ac676775968889724de73b7b1 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 01:06:40 -0700 Subject: [PATCH 20/42] Bring back prefetching links with inner elements --- src/observers/link_prefetch_observer.js | 6 +++--- src/tests/fixtures/hover_to_prefetch.html | 3 +++ src/tests/functional/link_prefetch_observer_tests.js | 5 +++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 972be9f86..3571308bd 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -2,6 +2,7 @@ import { doesNotTargetIFrame, getLocationForLink, getMetaContent, + findLinkFromClickTarget, findClosestRecursively } from "../util" import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" @@ -54,10 +55,9 @@ export class LinkPrefetchObserver { if (getMetaContent("turbo-prefetch") !== "true") return const target = event.target - const isLink = target.matches("a[href]:not([target^=_]):not([download])") - const link = target + const link = findLinkFromClickTarget(target) - if (isLink && this.#isPrefetchable(link)) { + if (link && this.#isPrefetchable(link)) { const location = getLocationForLink(link) if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { this.delegate.prefetchAndCacheRequestToLocation(link, location, this.#cacheTtl) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index ac045d6d0..d28498edd 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -9,6 +9,9 @@ Hover to prefetch me + + Hover to prefetch me + Hover to prefetch me
Won't prefetch when hovering me diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index e2aeb9f29..4cf572f19 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -134,6 +134,11 @@ test("it adds text/vnd.turbo-stream.html header to the Accept header when link h }}) }) +test("it prefetches links with inner elements", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_with_inner_elements" }) +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) From b27bc42a062898fd8eee976ab38c746a4c351d36 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 01:50:27 -0700 Subject: [PATCH 21/42] Add cancelable delay to prefetching links on hover --- src/observers/link_prefetch_observer.js | 29 ++++++++-- src/tests/fixtures/hover_to_prefetch.html | 1 + .../link_prefetch_observer_tests.js | 53 +++++++++++++++++-- 3 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 3571308bd..c4e21d8ae 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -58,13 +58,36 @@ export class LinkPrefetchObserver { const link = findLinkFromClickTarget(target) if (link && this.#isPrefetchable(link)) { - const location = getLocationForLink(link) - if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { - this.delegate.prefetchAndCacheRequestToLocation(link, location, this.#cacheTtl) + const delay = target.dataset.turboPrefetchDelay || getMetaContent("turbo-prefetch-delay") + + if (delay) { + this.prefetchTimeout = setTimeout(() => { + this.#startPrefetch(event, link) + } , Number(delay)) + + link.addEventListener('mouseout', this.#cancelPrefetchTimeoutIfAny, { + capture: true, + passive: true + }) + } else { + this.#startPrefetch(event, link) } } } + #startPrefetch = (event, link) => { + const location = getLocationForLink(link) + if (this.delegate.canPrefetchAndCacheRequestToLocation(link, location, event)) { + this.delegate.prefetchAndCacheRequestToLocation(link, location, this.#cacheTtl) + } + } + + #cancelPrefetchTimeoutIfAny = () => { + if (this.prefetchTimeout) { + clearTimeout(this.prefetchTimeout) + } + } + #tryToUsePrefetchedRequest = (event) => { if (event.target.tagName !== "FORM" && event.detail.fetchOptions.method === "get") { const cached = prefetchCache.get(event.detail.url.toString()) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index d28498edd..0cf9e2300 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -13,6 +13,7 @@ Hover to prefetch me Hover to prefetch me + Hover to prefetch me
Won't prefetch when hovering me
diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 4cf572f19..d5f9eb53a 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -113,7 +113,7 @@ test("it prefetches the page when turbo-prefetch-cache-time is set to 1", async await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) -test("it caches the request for 1 second when turbo-prefetch-cache-time is set to 1", async ({ page }) => { +test("it caches the request for 1 millisecond when turbo-prefetch-cache-time is set to 1", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) @@ -139,6 +139,40 @@ test("it prefetches links with inner elements", async ({ page }) => { await assertPrefetchedOnHover({ page, selector: "#anchor_with_inner_elements" }) }) +test("it prefetches links with a delay if present on the element itself", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + + let requestMade = false + page.on("request", async (request) => (requestMade = true)) + + await page.hover("#anchor_with_delay") + await sleep(100) + + assertRequestNotMade(requestMade) + + await sleep(300) + + assertRequestMade(requestMade) +}) + +test("it cancels the prefetch request if the link is no longer hovered", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + + let requestMade = false + page.on("request", async (request) => (requestMade = true)) + + await page.hover("#anchor_with_delay") + await sleep(100) + + assertRequestNotMade(requestMade) + + await page.mouse.move(0, 0) + + await sleep(300) + + assertRequestNotMade(requestMade) +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) @@ -166,7 +200,7 @@ const assertPrefetchedOnHover = async ({ page, selector, callback }) => { await hoverSelector({ page, selector }) - assert.equal(requestMade, true, "Network request wasn't made when it should have been.") + assertRequestMade(requestMade) } const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { @@ -182,18 +216,29 @@ const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { assert.equal(requestMade, false, "Network request was made when it should not have been.") } -const assertPrefetchedOnMouseDown = async ({ page, selector }) => { +const assertPrefetchedOnMouseDown = async ({ page, selector, callback }) => { let requestMade = false - page.on("request", (request) => (requestMade = true)) + page.on("request", (request) => { + callback && callback(request) + requestMade = true + }) await page.hover(selector) await page.mouse.down() await nextBeat() + assertRequestMade(requestMade) +} + +const assertRequestMade = (requestMade) => { assert.equal(requestMade, true, "Network request wasn't made when it should have been.") } +const assertRequestNotMade = (requestMade) => { + assert.equal(requestMade, false, "Network request was made when it should not have been.") +} + const goTo = async ({ page, path }) => { await page.goto(`/src/tests/fixtures${path}`) await nextBeat() From 4c86295556587323f8f20a243eab96bdd152fb4c Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 02:26:13 -0700 Subject: [PATCH 22/42] Fix clearing cache on every prefetch after b9e82f2 --- src/observers/link_prefetch_observer.js | 4 ++-- src/tests/fixtures/hover_to_prefetch.html | 1 + .../functional/link_prefetch_observer_tests.js | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index c4e21d8ae..b8d204335 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -93,11 +93,11 @@ export class LinkPrefetchObserver { const cached = prefetchCache.get(event.detail.url.toString()) if (cached && cached.ttl > new Date()) { + // User clicked link, use cache response and clear cache. event.detail.fetchRequest = cached.fetchRequest + prefetchCache.clear() } } - - prefetchCache.clear() } prepareRequest(request) { diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index 0cf9e2300..fdeaa24f5 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -9,6 +9,7 @@ Hover to prefetch me + Hover to prefetch me Hover to prefetch me diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index d5f9eb53a..29ebd700a 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -180,6 +180,23 @@ test("it does not make a network request when clicking on a link that has been p await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) +test("it does not more than 2 network requests when hovering between 2 links", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + + let requestCount = 0 + + page.on("request", async (request) => { + requestCount++ + }) + + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch_other_href" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + await hoverSelector({ page, selector: "#anchor_for_prefetch_other_href" }) + + assert.equal(requestCount, 2) +}) + test("it follows the link using the cached response when clicking on a link that has been prefetched", async ({ page }) => { From 4170ddba97fc835b5b214ed33441a54beb549282 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 02:32:41 -0700 Subject: [PATCH 23/42] Add tests for the delay on the meta tag --- ...er_to_prefetch_with_delay_on_meta_tag.html | 14 ++++++++ .../link_prefetch_observer_tests.js | 36 ++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html diff --git a/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html b/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html new file mode 100644 index 000000000..2efc23882 --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html @@ -0,0 +1,14 @@ + + + + + Hover to Prefetch + + + + + + + Hover to prefetch me + + diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 29ebd700a..3d23329ec 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -155,7 +155,7 @@ test("it prefetches links with a delay if present on the element itself", async assertRequestMade(requestMade) }) -test("it cancels the prefetch request if the link is no longer hovered", async ({ page }) => { +test("it cancels the prefetch request if the link with delay present on itself is no longer hovered", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) let requestMade = false @@ -173,6 +173,40 @@ test("it cancels the prefetch request if the link is no longer hovered", async ( assertRequestNotMade(requestMade) }) +test("it prefetches links with a delay if present on the meta tag", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_with_delay_on_meta_tag.html" }) + + let requestMade = false + page.on("request", async (request) => (requestMade = true)) + + await page.hover("#anchor_for_prefetch") + await sleep(100) + + assertRequestNotMade(requestMade) + + await sleep(300) + + assertRequestMade(requestMade) +}) + +test("it cancels the prefetch request if the link with delay present on the meta tag is no longer hovered", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_with_delay_on_meta_tag.html" }) + + let requestMade = false + page.on("request", async (request) => (requestMade = true)) + + await page.hover("#anchor_for_prefetch") + await sleep(100) + + assertRequestNotMade(requestMade) + + await page.mouse.move(0, 0) + + await sleep(300) + + assertRequestNotMade(requestMade) +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) From 5078e0bc2237ae7369e2a4db4179a7a7c4b6321d Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 02:48:40 -0700 Subject: [PATCH 24/42] Use mouseenter and mouseleave instead of mouseover and mouseout To avoid having to traverse the DOM to find the link element --- src/observers/link_prefetch_observer.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index b8d204335..d49cab7d8 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -2,14 +2,13 @@ import { doesNotTargetIFrame, getLocationForLink, getMetaContent, - findLinkFromClickTarget, findClosestRecursively } from "../util" import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" export class LinkPrefetchObserver { triggerEvents = { - mouseover: "mouseover", + mouseover: "mouseenter", mousedown: "mousedown" } started = false @@ -55,9 +54,10 @@ export class LinkPrefetchObserver { if (getMetaContent("turbo-prefetch") !== "true") return const target = event.target - const link = findLinkFromClickTarget(target) + const isLink = target.matches && target.matches("a[href]:not([target^=_]):not([download])") + const link = target - if (link && this.#isPrefetchable(link)) { + if (isLink && this.#isPrefetchable(link)) { const delay = target.dataset.turboPrefetchDelay || getMetaContent("turbo-prefetch-delay") if (delay) { @@ -65,7 +65,7 @@ export class LinkPrefetchObserver { this.#startPrefetch(event, link) } , Number(delay)) - link.addEventListener('mouseout', this.#cancelPrefetchTimeoutIfAny, { + link.addEventListener('mouseleave', this.#cancelPrefetchTimeoutIfAny, { capture: true, passive: true }) From 3d2665a86afed9df5aefcb4fe889a4182d81b5ed Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sat, 9 Dec 2023 03:03:11 -0700 Subject: [PATCH 25/42] Remove unneeded comment --- src/observers/link_prefetch_observer.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index d49cab7d8..fcb7c6248 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -116,8 +116,6 @@ export class LinkPrefetchObserver { } if (link.hasAttribute("data-turbo-stream")) { - // Add text/vnd.turbo-stream.html to Accept header, apart from the - // ones that the request already has. request.acceptResponseType("text/vnd.turbo-stream.html") } } From efd58fbd22d93ab30f1cba8c9724cfa444d8f02d Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 11:40:25 -0700 Subject: [PATCH 26/42] Use double quotes instead of single quotes for consistency --- src/observers/link_prefetch_observer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index fcb7c6248..3bee621ba 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -65,7 +65,7 @@ export class LinkPrefetchObserver { this.#startPrefetch(event, link) } , Number(delay)) - link.addEventListener('mouseleave', this.#cancelPrefetchTimeoutIfAny, { + link.addEventListener("mouseleave", this.#cancelPrefetchTimeoutIfAny, { capture: true, passive: true }) From 10e1311864c50a886e0ec98e7362a2aa96a38736 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 11:42:34 -0700 Subject: [PATCH 27/42] Move link variable declaration inside if statement Since target is only a link if isLink is true --- src/observers/link_prefetch_observer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 3bee621ba..ac8106789 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -55,10 +55,10 @@ export class LinkPrefetchObserver { const target = event.target const isLink = target.matches && target.matches("a[href]:not([target^=_]):not([download])") - const link = target - if (isLink && this.#isPrefetchable(link)) { - const delay = target.dataset.turboPrefetchDelay || getMetaContent("turbo-prefetch-delay") + if (isLink && this.#isPrefetchable(target)) { + const link = target + const delay = link.dataset.turboPrefetchDelay || getMetaContent("turbo-prefetch-delay") if (delay) { this.prefetchTimeout = setTimeout(() => { From 5326a0f336fe54aa0020acbe80390c5e2cb63030 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 11:47:19 -0700 Subject: [PATCH 28/42] Use correct key name for mouseenter event on LinkPrefetchObserver.triggerEvents On 5078e0b we started using the `mouseenter` event instead of the `mouseover` event to trigger prefetching. However, we forgot to update the key name on the `LinkPrefetchObserver.triggerEvents` object. --- src/observers/link_prefetch_observer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index ac8106789..85133594d 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -8,7 +8,7 @@ import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" export class LinkPrefetchObserver { triggerEvents = { - mouseover: "mouseenter", + mouseenter: "mouseenter", mousedown: "mousedown" } started = false @@ -135,7 +135,7 @@ export class LinkPrefetchObserver { requestFailedWithResponse(fetchRequest, fetchResponse) {} get #triggerEvent() { - return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseover + return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseenter } get #cacheTtl() { From a551b1f256bb0376895c93d5688bd017c242c1e6 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 12:47:48 -0700 Subject: [PATCH 29/42] Allow prefetching when visiting page without meta, then visiting one with it --- src/observers/link_prefetch_observer.js | 2 -- ...t_meta_tag_with_link_to_with_meta_tag.html | 19 +++++++++++++++++++ .../link_prefetch_observer_tests.js | 8 ++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 src/tests/fixtures/hover_to_prefetch_without_meta_tag_with_link_to_with_meta_tag.html diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 85133594d..02d0780ff 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -40,8 +40,6 @@ export class LinkPrefetchObserver { } #enable = () => { - if (getMetaContent("turbo-prefetch") !== "true") return - this.eventTarget.addEventListener(this.#triggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true diff --git a/src/tests/fixtures/hover_to_prefetch_without_meta_tag_with_link_to_with_meta_tag.html b/src/tests/fixtures/hover_to_prefetch_without_meta_tag_with_link_to_with_meta_tag.html new file mode 100644 index 000000000..75a7265aa --- /dev/null +++ b/src/tests/fixtures/hover_to_prefetch_without_meta_tag_with_link_to_with_meta_tag.html @@ -0,0 +1,19 @@ + + + + + Hover to Prefetch Not Enabled + + + + + + + Click to go to page with prefetch meta tag + + + diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 3d23329ec..c652364cf 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -96,6 +96,14 @@ test("it doesn't prefetch the page when turbo-prefetch meta tag is set to true, await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) +test("it prefetches when visiting a page without the meta tag, then visiting a page with it", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch_without_meta_tag_with_link_to_with_meta_tag.html" }) + + await clickSelector({ page, selector: "#anchor_for_page_with_meta_tag" }) + + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + test("it prefetches the page on mousedown when turbo-prefetch-trigger-event is set to mousedown", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) From ebea9510613402e71eee0c25fcfd66cc14aa062e Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 15:39:38 -0700 Subject: [PATCH 30/42] Allow create and delete posts with comments on the test server --- package.json | 1 + .../fixtures/volatile_posts_database.json | 1 + src/tests/server.mjs | 100 ++++++++++++++++++ src/tests/templates/post.eta | 34 ++++++ src/tests/templates/posts.eta | 29 +++++ yarn.lock | 5 + 6 files changed, 170 insertions(+) create mode 100644 src/tests/fixtures/volatile_posts_database.json create mode 100644 src/tests/templates/post.eta create mode 100644 src/tests/templates/posts.eta diff --git a/package.json b/package.json index c74caab8d..6e67127e7 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "body-parser": "^1.20.1", "chai": "~4.3.4", "eslint": "^8.13.0", + "eta": "^3.2.0", "express": "^4.18.2", "idiomorph": "https://github.com/basecamp/idiomorph#rollout-build", "multer": "^1.4.2", diff --git a/src/tests/fixtures/volatile_posts_database.json b/src/tests/fixtures/volatile_posts_database.json new file mode 100644 index 000000000..0637a088a --- /dev/null +++ b/src/tests/fixtures/volatile_posts_database.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/src/tests/server.mjs b/src/tests/server.mjs index 10eab3abb..6fab98dc7 100644 --- a/src/tests/server.mjs +++ b/src/tests/server.mjs @@ -4,6 +4,7 @@ import multer from "multer" import path from "path" import url, { fileURLToPath } from "url" import fs from "fs" +import { Eta } from "eta" const __filename = fileURLToPath(import.meta.url) const __dirname = path.dirname(__filename) @@ -12,6 +13,7 @@ const { json, urlencoded } = bodyParser const router = Router() const streamResponses = new Set() +const templateRenderer = new Eta({ views: path.join(__dirname, "templates") }) router.use(multer().none()) @@ -151,6 +153,104 @@ router.put("/messages/:id", (request, response) => { } }) +router.get("/posts", async (request, response) => { + const posts = await getPosts() + + const res = templateRenderer.render("./posts", { posts }) + + response.type("html").status(200).send(res) +}) + +router.post("/posts", async (request, response) => { + const { title, body } = request.body + + await addPost(title, body) + + response.redirect(303, "/__turbo/posts") +}) + +router.get("/posts/:id/", async (request, response) => { + const posts = await getPosts() + const { id } = request.params + + const post = posts.find((post) => post.id == id) + + if (post) { + const res = templateRenderer.render("./post", { post }) + + response.type("html").status(200).send(res) + } else { + response.sendStatus(404) + } +}) + +router.post("/posts/:id", async (request, response) => { + if (request.body._method == "delete") { + const { id } = request.params + + await deletePost(id) + response.redirect(303, "/__turbo/posts") + } else { + response.sendStatus(422) + } +}) + +router.post("/posts/:post_id/comments", async (request, response) => { + const { post_id } = request.params + const { body } = request.body.comment + + const post = await getPost(post_id) + + if (post) { + await addComment(post_id, body) + + response.redirect(303, `/__turbo/posts/${post_id}`) + } else { + response.sendStatus(404) + } +}) + +const databaseName = "src/tests/fixtures/volatile_posts_database.json" + +const getPosts = async () => { + return JSON.parse(fs.readFileSync(databaseName).toString()) +} + +const addPost = async (title, body) => { + const posts = await getPosts() + + const id = posts.length + 1 + + posts.push({ id, title, body }) + + return fs.writeFileSync(databaseName, JSON.stringify(posts)) +} + +const getPost = async (id) => { + const posts = await getPosts() + + return posts.find((post) => post.id == id) +} + +const deletePost = async (id) => { + const posts = await getPosts() + const newPosts = posts.filter((post) => post.id != id) + + return fs.writeFileSync(databaseName, JSON.stringify(newPosts)) +} + +const addComment = async (postId, body) => { + const posts = await getPosts() + const post = posts.find((post) => post.id == postId) + const comments = post.comments || [] + const id = comments.length + 1 + + post.comments = comments + post.comments.push({ id, body }) + + return fs.writeFileSync(databaseName, JSON.stringify(posts)) +} + router.get("/messages", (request, response) => { response.set({ "Cache-Control": "no-cache", diff --git a/src/tests/templates/post.eta b/src/tests/templates/post.eta new file mode 100644 index 000000000..07e31ba17 --- /dev/null +++ b/src/tests/templates/post.eta @@ -0,0 +1,34 @@ + + + Post + + + + +

<%= it.post.title %>

+ +

<%= it.post.body %>

+ +

Comments

+ + + +

Add a comment

+ +
+ + +
+ +
+ + +
+ + Back to all posts + + \ No newline at end of file diff --git a/src/tests/templates/posts.eta b/src/tests/templates/posts.eta new file mode 100644 index 000000000..d6261309b --- /dev/null +++ b/src/tests/templates/posts.eta @@ -0,0 +1,29 @@ + + + Posts + + + + +

Posts

+ + +
+ + + + + + + +
+ + \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index b0e316fa0..e29c68c9a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1522,6 +1522,11 @@ esutils@^2.0.2: resolved "https://registry.yarnpkg.com/esutils/-/esutils-2.0.3.tgz#74d2eb4de0b8da1293711910d50775b9b710ef64" integrity sha512-kVscqXk4OCp68SZ0dkgEKVi6/8ij300KBWTJq32P/dYeWTSwK41WyTxalN1eRmA5Z9UU/LX9D7FWSmV9SAYx6g== +eta@^3.2.0: + version "3.2.0" + resolved "https://registry.yarnpkg.com/eta/-/eta-3.2.0.tgz#7a2af4c897f57ea9483f5d953dc35c1ab0e97080" + integrity sha512-Qzc3it7nLn49dbOb9+oHV9rwtt9qN8oShRztqkZ3gXPqQflF0VLin5qhWk0g/2ioibBwT4DU6OIMVft7tg/rVg== + etag@^1.8.1, etag@~1.8.1: version "1.8.1" resolved "https://registry.yarnpkg.com/etag/-/etag-1.8.1.tgz#41ae2eeb65efa62268aebfea83ac7d79299b0887" From aa6f0137c9a562592e6102c05d785ec00b75c0e3 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Sun, 7 Jan 2024 17:16:12 -0700 Subject: [PATCH 31/42] Clear prefetch cache after form submission --- src/core/drive/form_submission.js | 10 ++++- .../link_prefetch_observer_tests.js | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/core/drive/form_submission.js b/src/core/drive/form_submission.js index e2fa643b2..10624fa68 100644 --- a/src/core/drive/form_submission.js +++ b/src/core/drive/form_submission.js @@ -2,6 +2,7 @@ import { FetchRequest, FetchMethod, fetchMethodFromString, fetchEnctypeFromStrin import { expandURL } from "../url" import { dispatch, getAttribute, getMetaContent, hasAttribute } from "../../util" import { StreamMessage } from "../streams/stream_message" +import { prefetchCache } from "./prefetch_cache" export const FormSubmissionState = { initialized: "initialized", @@ -125,13 +126,20 @@ export class FormSubmission { } requestPreventedHandlingResponse(request, response) { + prefetchCache.clear() + this.result = { success: response.succeeded, fetchResponse: response } } requestSucceededWithResponse(request, response) { if (response.clientError || response.serverError) { this.delegate.formSubmissionFailedWithResponse(this, response) - } else if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) { + return + } + + prefetchCache.clear() + + if (this.requestMustRedirect(request) && responseSucceededWithoutRedirect(response)) { const error = new Error("Form responses must redirect to another location") this.delegate.formSubmissionErrored(this, error) } else { diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index c652364cf..439c6e741 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -1,6 +1,21 @@ import { test } from "@playwright/test" import { assert } from "chai" import { nextBeat, sleep } from "../helpers/page" +import fs from 'fs' +import path from 'path' + +// eslint-disable-next-line no-undef +const fixturesPath = path.join(process.cwd(), 'src', 'tests', 'fixtures', 'volatile_posts_database.json') + +test.beforeEach(() => { + // Clear the contents of the file before each test + fs.writeFileSync(fixturesPath, '[]') +}) + +test.afterEach(() => { + // Clear the contents of the file after each test + fs.writeFileSync(fixturesPath, '[]') +}) test("it prefetches the page", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) @@ -215,6 +230,30 @@ test("it cancels the prefetch request if the link with delay present on the meta assertRequestNotMade(requestMade) }) +test("it clears cache on form submission", async ({ page }) => { + await page.goto("/__turbo/posts") + + await page.click("#submit") + const post = await page.getByTestId("post_1") + const postText = await post.innerText() + assert.equal(postText, "A new post title") + + await post.click() + + await page.hover("#anchor_back_to_all_posts") + + await page.click("#comment_submit") + const comment = await page.getByTestId("comment_1") + const commentText = await comment.innerText() + assert.equal(commentText, "A new comment") + + await page.click("#anchor_back_to_all_posts") + + const postComments = await page.getByTestId("post_1_comments") + const postCommentsText = await postComments.innerText() + assert.equal(postCommentsText, "(1 comments)") +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) From 851dabba538543c28f8d2a9cca7f6a0d8a29a936 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 16 Jan 2024 05:09:57 +0100 Subject: [PATCH 32/42] Add test for nested data-turbo-prefetch=true within data-turbo-prefetch=false --- src/tests/fixtures/hover_to_prefetch.html | 3 +++ src/tests/functional/link_prefetch_observer_tests.js | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index fdeaa24f5..d762a30cc 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -20,6 +20,9 @@
Won't prefetch when hovering me +
+ Hover to prefetch me +
Won't prefetch when hovering me diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 439c6e741..84122c330 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -49,6 +49,11 @@ test("it doesn't prefetch the page when link is inside an element with data-turb await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false_parent" }) }) +test("it does prefech the page when link is inside a container with data-turbo-prefetch=true, that is within an element with data-turbo-prefetch=false", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_true_parent_within_turbo_prefetch_false_parent" }) +}) + test("it doesn't prefetch the page when link has data-turbo-prefetch=false", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_prefetch_false" }) From ccab6b25b0a18b1c158bd7d258dd1901adc4df6d Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 16 Jan 2024 05:14:05 +0100 Subject: [PATCH 33/42] No longer allow customizing the prefetch trigger event --- src/observers/link_prefetch_observer.js | 14 +++------- .../fixtures/hover_to_prefetch_mousedown.html | 14 ---------- .../link_prefetch_observer_tests.js | 27 ------------------- 3 files changed, 4 insertions(+), 51 deletions(-) delete mode 100644 src/tests/fixtures/hover_to_prefetch_mousedown.html diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 02d0780ff..71d66a2ec 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -7,11 +7,9 @@ import { import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" export class LinkPrefetchObserver { - triggerEvents = { - mouseenter: "mouseenter", - mousedown: "mousedown" - } started = false + delayBeforePrefetching = 100 + triggerEvent = "mouseenter" constructor(delegate, eventTarget) { this.delegate = delegate @@ -31,7 +29,7 @@ export class LinkPrefetchObserver { stop() { if (!this.started) return - this.eventTarget.removeEventListener(this.#triggerEvent, this.#tryToPrefetchRequest, { + this.eventTarget.removeEventListener(this.triggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) @@ -40,7 +38,7 @@ export class LinkPrefetchObserver { } #enable = () => { - this.eventTarget.addEventListener(this.#triggerEvent, this.#tryToPrefetchRequest, { + this.eventTarget.addEventListener(this.triggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) @@ -132,10 +130,6 @@ export class LinkPrefetchObserver { requestFailedWithResponse(fetchRequest, fetchResponse) {} - get #triggerEvent() { - return this.triggerEvents[getMetaContent("turbo-prefetch-trigger-event")] || this.triggerEvents.mouseenter - } - get #cacheTtl() { return Number(getMetaContent("turbo-prefetch-cache-time")) || cacheTtl } diff --git a/src/tests/fixtures/hover_to_prefetch_mousedown.html b/src/tests/fixtures/hover_to_prefetch_mousedown.html deleted file mode 100644 index 0dd5668d5..000000000 --- a/src/tests/fixtures/hover_to_prefetch_mousedown.html +++ /dev/null @@ -1,14 +0,0 @@ - - - - - Hover to Prefetch - - - - - - - Hover to prefetch me - - diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 84122c330..678eb3650 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -124,18 +124,6 @@ test("it prefetches when visiting a page without the meta tag, then visiting a p await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) -test("it prefetches the page on mousedown when turbo-prefetch-trigger-event is set to mousedown", async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) - await assertPrefetchedOnMouseDown({ page, selector: "#anchor_for_prefetch" }) -}) - -test("it doesn't prefetch the page on mouseover when turbo-prefetch-trigger-event is set to mousedown", async ({ - page -}) => { - await goTo({ page, path: "/hover_to_prefetch_mousedown.html" }) - await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) -}) - test("it prefetches the page when turbo-prefetch-cache-time is set to 1", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch_custom_cache_time.html" }) await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) @@ -319,21 +307,6 @@ const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { assert.equal(requestMade, false, "Network request was made when it should not have been.") } -const assertPrefetchedOnMouseDown = async ({ page, selector, callback }) => { - let requestMade = false - - page.on("request", (request) => { - callback && callback(request) - requestMade = true - }) - - await page.hover(selector) - await page.mouse.down() - await nextBeat() - - assertRequestMade(requestMade) -} - const assertRequestMade = (requestMade) => { assert.equal(requestMade, true, "Network request wasn't made when it should have been.") } From 06181be65780d3279d8b1a9504d2751f3fd2df28 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 16 Jan 2024 05:43:51 +0100 Subject: [PATCH 34/42] No longer allow customizing the prefetch delay --- src/observers/link_prefetch_observer.js | 21 +++----- src/tests/fixtures/hover_to_prefetch.html | 1 - ...er_to_prefetch_with_delay_on_meta_tag.html | 14 ----- .../link_prefetch_observer_tests.js | 54 +++++-------------- 4 files changed, 21 insertions(+), 69 deletions(-) delete mode 100644 src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 71d66a2ec..51b88bcc2 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -54,20 +54,15 @@ export class LinkPrefetchObserver { if (isLink && this.#isPrefetchable(target)) { const link = target - const delay = link.dataset.turboPrefetchDelay || getMetaContent("turbo-prefetch-delay") - - if (delay) { - this.prefetchTimeout = setTimeout(() => { - this.#startPrefetch(event, link) - } , Number(delay)) - - link.addEventListener("mouseleave", this.#cancelPrefetchTimeoutIfAny, { - capture: true, - passive: true - }) - } else { + + this.prefetchTimeout = setTimeout(() => { this.#startPrefetch(event, link) - } + } , this.delayBeforePrefetching) + + link.addEventListener("mouseleave", this.#cancelPrefetchTimeoutIfAny, { + capture: true, + passive: true + }) } } diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index d762a30cc..e0748fe0e 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -14,7 +14,6 @@ Hover to prefetch me Hover to prefetch me - Hover to prefetch me
Won't prefetch when hovering me
diff --git a/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html b/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html deleted file mode 100644 index 2efc23882..000000000 --- a/src/tests/fixtures/hover_to_prefetch_with_delay_on_meta_tag.html +++ /dev/null @@ -1,14 +0,0 @@ - - - - - Hover to Prefetch - - - - - - - Hover to prefetch me - - diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 678eb3650..612333dba 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -155,70 +155,36 @@ test("it prefetches links with inner elements", async ({ page }) => { await assertPrefetchedOnHover({ page, selector: "#anchor_with_inner_elements" }) }) -test("it prefetches links with a delay if present on the element itself", async ({ page }) => { +test("it prefetches links with a delay", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) let requestMade = false page.on("request", async (request) => (requestMade = true)) - await page.hover("#anchor_with_delay") - await sleep(100) - - assertRequestNotMade(requestMade) - - await sleep(300) - - assertRequestMade(requestMade) -}) - -test("it cancels the prefetch request if the link with delay present on itself is no longer hovered", async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch.html" }) - - let requestMade = false - page.on("request", async (request) => (requestMade = true)) - - await page.hover("#anchor_with_delay") - await sleep(100) - - assertRequestNotMade(requestMade) - - await page.mouse.move(0, 0) - - await sleep(300) - - assertRequestNotMade(requestMade) -}) - -test("it prefetches links with a delay if present on the meta tag", async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_with_delay_on_meta_tag.html" }) - - let requestMade = false - page.on("request", async (request) => (requestMade = true)) - await page.hover("#anchor_for_prefetch") - await sleep(100) + await sleep(75) assertRequestNotMade(requestMade) - await sleep(300) + await sleep(100) assertRequestMade(requestMade) }) -test("it cancels the prefetch request if the link with delay present on the meta tag is no longer hovered", async ({ page }) => { - await goTo({ page, path: "/hover_to_prefetch_with_delay_on_meta_tag.html" }) +test("it cancels the prefetch request if the link is no longer hovered", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) let requestMade = false page.on("request", async (request) => (requestMade = true)) await page.hover("#anchor_for_prefetch") - await sleep(100) + await sleep(75) assertRequestNotMade(requestMade) await page.mouse.move(0, 0) - await sleep(300) + await sleep(100) assertRequestNotMade(requestMade) }) @@ -251,6 +217,8 @@ test("it does not make a network request when clicking on a link that has been p await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) + await sleep(100) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) }) @@ -291,6 +259,8 @@ const assertPrefetchedOnHover = async ({ page, selector, callback }) => { await hoverSelector({ page, selector }) + await sleep(100) + assertRequestMade(requestMade) } @@ -304,6 +274,8 @@ const assertNotPrefetchedOnHover = async ({ page, selector, callback }) => { await hoverSelector({ page, selector }) + await sleep(100) + assert.equal(requestMade, false, "Network request was made when it should not have been.") } From 0704cef8c654a6dc313537c676f30710425df9f9 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 16 Jan 2024 06:05:44 +0100 Subject: [PATCH 35/42] Add touchstart event to prefetch observer --- playwright.config.js | 6 +++-- src/observers/link_prefetch_observer.js | 15 ++++++++--- .../link_prefetch_observer_tests.js | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/playwright.config.js b/playwright.config.js index aae8335d2..4b4dfdc22 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -8,7 +8,8 @@ const config = { ...devices["Desktop Chrome"], contextOptions: { timeout: 60000 - } + }, + hasTouch: true } }, { @@ -17,7 +18,8 @@ const config = { ...devices["Desktop Firefox"], contextOptions: { timeout: 60000 - } + }, + hasTouch: true } } ], diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 51b88bcc2..deb33ac84 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -9,7 +9,8 @@ import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" export class LinkPrefetchObserver { started = false delayBeforePrefetching = 100 - triggerEvent = "mouseenter" + hoverTriggerEvent = "mouseenter" + touchTriggerEvent = "touchstart" constructor(delegate, eventTarget) { this.delegate = delegate @@ -29,7 +30,11 @@ export class LinkPrefetchObserver { stop() { if (!this.started) return - this.eventTarget.removeEventListener(this.triggerEvent, this.#tryToPrefetchRequest, { + this.eventTarget.removeEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, { + capture: true, + passive: true + }) + this.eventTarget.removeEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) @@ -38,7 +43,11 @@ export class LinkPrefetchObserver { } #enable = () => { - this.eventTarget.addEventListener(this.triggerEvent, this.#tryToPrefetchRequest, { + this.eventTarget.addEventListener(this.hoverTriggerEvent, this.#tryToPrefetchRequest, { + capture: true, + passive: true + }) + this.eventTarget.addEventListener(this.touchTriggerEvent, this.#tryToPrefetchRequest, { capture: true, passive: true }) diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 612333dba..8ccf00347 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -213,6 +213,11 @@ test("it clears cache on form submission", async ({ page }) => { assert.equal(postCommentsText, "(1 comments)") }) +test("it prefetches page on touchstart", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertPrefetchedOnTouchstart({ page, selector: "#anchor_for_prefetch" }) +}) + test("it does not make a network request when clicking on a link that has been prefetched", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await hoverSelector({ page, selector: "#anchor_for_prefetch" }) @@ -249,6 +254,26 @@ 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 From c975a7bdf2d2ef2b0701c2203bd7d69ae5dbcdb4 Mon Sep 17 00:00:00 2001 From: David Alejandro <15317732+davidalejandroaguilar@users.noreply.github.com> Date: Tue, 16 Jan 2024 07:01:41 +0100 Subject: [PATCH 36/42] Fix flaky tests This commit fixes the flaky tests by ensuring that each worker has its own database file. This is done by adding a `worker_id` query parameter to the URLs of the pages that are being tested. This `worker_id` is passed to the database functions, which then use it to determine the name of the database file. It's necessary because the tests are running in parallel, and the database file is shared between all the workers. This means that if one worker creates a post, the other workers will see that post, and the tests will fail. --- .../fixtures/volatile_posts_database.json | 1 - .../link_prefetch_observer_tests.js | 23 +++--- src/tests/server.mjs | 75 ++++++++++++------- src/tests/templates/post.eta | 4 +- src/tests/templates/posts.eta | 4 +- 5 files changed, 69 insertions(+), 38 deletions(-) delete mode 100644 src/tests/fixtures/volatile_posts_database.json diff --git a/src/tests/fixtures/volatile_posts_database.json b/src/tests/fixtures/volatile_posts_database.json deleted file mode 100644 index 0637a088a..000000000 --- a/src/tests/fixtures/volatile_posts_database.json +++ /dev/null @@ -1 +0,0 @@ -[] \ No newline at end of file diff --git a/src/tests/functional/link_prefetch_observer_tests.js b/src/tests/functional/link_prefetch_observer_tests.js index 8ccf00347..dd2ef6da3 100644 --- a/src/tests/functional/link_prefetch_observer_tests.js +++ b/src/tests/functional/link_prefetch_observer_tests.js @@ -5,16 +5,14 @@ import fs from 'fs' import path from 'path' // eslint-disable-next-line no-undef -const fixturesPath = path.join(process.cwd(), 'src', 'tests', 'fixtures', 'volatile_posts_database.json') - -test.beforeEach(() => { - // Clear the contents of the file before each test - fs.writeFileSync(fixturesPath, '[]') -}) +const fixturesDir = path.join(process.cwd(), 'src', 'tests', 'fixtures') test.afterEach(() => { - // Clear the contents of the file after each test - fs.writeFileSync(fixturesPath, '[]') + fs.readdirSync(fixturesDir).forEach(file => { + if (file.startsWith('volatile_posts_database')) { + fs.unlinkSync(path.join(fixturesDir, file)) + } + }) }) test("it prefetches the page", async ({ page }) => { @@ -190,24 +188,31 @@ test("it cancels the prefetch request if the link is no longer hovered", async ( }) test("it clears cache on form submission", async ({ page }) => { - await page.goto("/__turbo/posts") + // eslint-disable-next-line no-undef + await page.goto(`/__turbo/posts?worker_id=${process.env.TEST_PARALLEL_INDEX}`) + // Create a post await page.click("#submit") const post = await page.getByTestId("post_1") const postText = await post.innerText() assert.equal(postText, "A new post title") + // Visit the post await post.click() + // Hover over the link back to all posts, prefetching page showing 0 comments await page.hover("#anchor_back_to_all_posts") + // Create a comment, prefetch cache should be discarded await page.click("#comment_submit") const comment = await page.getByTestId("comment_1") const commentText = await comment.innerText() assert.equal(commentText, "A new comment") + // Go back to all posts await page.click("#anchor_back_to_all_posts") + // Assert that the comments count is 1, not 0 const postComments = await page.getByTestId("post_1_comments") const postCommentsText = await postComments.innerText() assert.equal(postCommentsText, "(1 comments)") diff --git a/src/tests/server.mjs b/src/tests/server.mjs index 6fab98dc7..6338da067 100644 --- a/src/tests/server.mjs +++ b/src/tests/server.mjs @@ -154,29 +154,32 @@ router.put("/messages/:id", (request, response) => { }) router.get("/posts", async (request, response) => { - const posts = await getPosts() + const { worker_id } = request.query - const res = templateRenderer.render("./posts", { posts }) + const posts = await getPosts(worker_id) + + const res = templateRenderer.render("./posts", { posts, worker_id }) response.type("html").status(200).send(res) }) router.post("/posts", async (request, response) => { - const { title, body } = request.body + const { title, body, worker_id } = request.body - await addPost(title, body) + await addPost(title, body, worker_id) - response.redirect(303, "/__turbo/posts") + response.redirect(303, `/__turbo/posts?worker_id=${worker_id}`) }) router.get("/posts/:id/", async (request, response) => { - const posts = await getPosts() + const { worker_id } = request.query const { id } = request.params + const posts = await getPosts(worker_id) const post = posts.find((post) => post.id == id) if (post) { - const res = templateRenderer.render("./post", { post }) + const res = templateRenderer.render("./post", { post, worker_id }) response.type("html").status(200).send(res) } else { @@ -186,10 +189,11 @@ router.get("/posts/:id/", async (request, response) => { router.post("/posts/:id", async (request, response) => { if (request.body._method == "delete") { + const { worker_id } = request.body const { id } = request.params - await deletePost(id) - response.redirect(303, "/__turbo/posts") + await deletePost(id, worker_id) + response.redirect(303, `/__turbo/posts?worker_id=${worker_id}`) } else { response.sendStatus(422) } @@ -197,50 +201,69 @@ router.post("/posts/:id", async (request, response) => { router.post("/posts/:post_id/comments", async (request, response) => { const { post_id } = request.params + const { worker_id } = request.body const { body } = request.body.comment - const post = await getPost(post_id) + const post = await getPost(post_id, worker_id) if (post) { - await addComment(post_id, body) + await addComment(post_id, body, worker_id) - response.redirect(303, `/__turbo/posts/${post_id}`) + response.redirect(303, `/__turbo/posts/${post_id}?worker_id=${worker_id}`) } else { response.sendStatus(404) } }) -const databaseName = "src/tests/fixtures/volatile_posts_database.json" +const postsDatabaseName = (worker_id) => { + return `src/tests/fixtures/volatile_posts_database_${worker_id}.json` +} + +const ensureDatabase = async (worker_id) => { + if (worker_id == null || worker_id == undefined) { + throw new Error("worker_id is required") + } + + if (!fs.existsSync(postsDatabaseName(worker_id))) { + fs.writeFileSync(postsDatabaseName(worker_id), "[]") + } +} + +const getPosts = async (worker_id) => { + await ensureDatabase(worker_id) -const getPosts = async () => { - return JSON.parse(fs.readFileSync(databaseName).toString()) + return JSON.parse(fs.readFileSync(postsDatabaseName(worker_id)).toString()) } -const addPost = async (title, body) => { - const posts = await getPosts() +const addPost = async (title, body, worker_id) => { + await ensureDatabase(worker_id) + const posts = await getPosts(worker_id) const id = posts.length + 1 posts.push({ id, title, body }) - return fs.writeFileSync(databaseName, JSON.stringify(posts)) + return fs.writeFileSync(postsDatabaseName(worker_id), JSON.stringify(posts)) } -const getPost = async (id) => { - const posts = await getPosts() +const getPost = async (id, worker_id) => { + await ensureDatabase(worker_id) + const posts = await getPosts(worker_id) return posts.find((post) => post.id == id) } -const deletePost = async (id) => { - const posts = await getPosts() +const deletePost = async (id, worker_id) => { + await ensureDatabase(worker_id) + const posts = await getPosts(worker_id) const newPosts = posts.filter((post) => post.id != id) - return fs.writeFileSync(databaseName, JSON.stringify(newPosts)) + return fs.writeFileSync(postsDatabaseName(worker_id), JSON.stringify(newPosts)) } -const addComment = async (postId, body) => { - const posts = await getPosts() +const addComment = async (postId, body, worker_id) => { + await ensureDatabase(worker_id) + const posts = await getPosts(worker_id) const post = posts.find((post) => post.id == postId) const comments = post.comments || [] const id = comments.length + 1 @@ -248,7 +271,7 @@ const addComment = async (postId, body) => { post.comments = comments post.comments.push({ id, body }) - return fs.writeFileSync(databaseName, JSON.stringify(posts)) + return fs.writeFileSync(postsDatabaseName(worker_id), JSON.stringify(posts)) } router.get("/messages", (request, response) => { diff --git a/src/tests/templates/post.eta b/src/tests/templates/post.eta index 07e31ba17..9969ed060 100644 --- a/src/tests/templates/post.eta +++ b/src/tests/templates/post.eta @@ -22,13 +22,15 @@
+
+
- Back to all posts + Back to all posts \ No newline at end of file diff --git a/src/tests/templates/posts.eta b/src/tests/templates/posts.eta index d6261309b..1d64b8a7d 100644 --- a/src/tests/templates/posts.eta +++ b/src/tests/templates/posts.eta @@ -9,7 +9,7 @@