From 38e7bd27f26b50377faecf218eab862c35efffb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez-Capel?= Date: Fri, 9 Feb 2024 09:28:08 +0000 Subject: [PATCH] Add more prefetch conditions (#1178) * Clean up method to decide if a link is prefetchable * Add more conditions where a link is not considered safe to prefetch Exclude links with data-turbo-stream or associated with UJS behavior. * We no longer preload data-turbo-stream links * Update test * Add some more test cases --- src/observers/link_prefetch_observer.js | 76 +++++++++---------- src/tests/fixtures/hover_to_prefetch.html | 4 + .../link_prefetch_observer_tests.js | 28 ++++--- 3 files changed, 56 insertions(+), 52 deletions(-) diff --git a/src/observers/link_prefetch_observer.js b/src/observers/link_prefetch_observer.js index 957898644..6265f075a 100644 --- a/src/observers/link_prefetch_observer.js +++ b/src/observers/link_prefetch_observer.js @@ -1,12 +1,10 @@ import { dispatch, - doesNotTargetIFrame, getLocationForLink, getMetaContent, findClosestRecursively } from "../util" -import { StreamMessage } from "../core/streams/stream_message" import { FetchMethod, FetchRequest } from "../http/fetch_request" import { prefetchCache, cacheTtl } from "../core/drive/prefetch_cache" @@ -118,10 +116,6 @@ export class LinkPrefetchObserver { if (turboFrameTarget && turboFrameTarget !== "_top") { request.headers["Turbo-Frame"] = turboFrameTarget } - - if (link.hasAttribute("data-turbo-stream")) { - request.acceptResponseType(StreamMessage.contentType) - } } // Fetch request interface @@ -145,50 +139,52 @@ export class LinkPrefetchObserver { #isPrefetchable(link) { const href = link.getAttribute("href") - if (!href || href.startsWith("#") || link.getAttribute("data-turbo") === "false" || link.getAttribute("data-turbo-prefetch") === "false") { - return false - } + if (!href) return false - const event = dispatch("turbo:before-prefetch", { - target: link, - cancelable: true - }) + if (unfetchableLink(link)) return false + if (linkToTheSamePage(link)) return false + if (linkOptsOut(link)) return false + if (nonSafeLink(link)) return false + if (eventPrevented(link)) return false - if (event.defaultPrevented) { - return false - } + return true + } +} - if (link.origin !== document.location.origin) { - return false - } +const unfetchableLink = (link) => { + return link.origin !== document.location.origin || !["http:", "https:"].includes(link.protocol) || link.hasAttribute("target") +} - if (!["http:", "https:"].includes(link.protocol)) { - return false - } +const linkToTheSamePage = (link) => { + return (link.pathname + link.search === document.location.pathname + document.location.search) || link.href.startsWith("#") +} - if (link.pathname + link.search === document.location.pathname + document.location.search) { - return false - } +const linkOptsOut = (link) => { + if (link.getAttribute("data-turbo-prefetch") === "false") return true + if (link.getAttribute("data-turbo") === "false") return true - const turboMethod = link.getAttribute("data-turbo-method") - if (turboMethod && turboMethod !== "get") { - return false - } + const turboPrefetchParent = findClosestRecursively(link, "[data-turbo-prefetch]") + if (turboPrefetchParent && turboPrefetchParent.getAttribute("data-turbo-prefetch") === "false") return true - if (targetsIframe(link)) { - return false - } + return false +} - const turboPrefetchParent = findClosestRecursively(link, "[data-turbo-prefetch]") +const nonSafeLink = (link) => { + const turboMethod = link.getAttribute("data-turbo-method") + if (turboMethod && turboMethod.toLowerCase() !== "get") return true - if (turboPrefetchParent && turboPrefetchParent.getAttribute("data-turbo-prefetch") === "false") { - return false - } + if (isUJS(link)) return true + if (link.hasAttribute("data-turbo-confirm")) return true + if (link.hasAttribute("data-turbo-stream")) return true - return true - } + return false +} + +const isUJS = (link) => { + return link.hasAttribute("data-remote") || link.hasAttribute("data-behavior") || link.hasAttribute("data-confirm") || link.hasAttribute("data-method") } -const targetsIframe = (link) => { - return !doesNotTargetIFrame(link) +const eventPrevented = (link) => { + const event = dispatch("turbo:before-prefetch", { target: link, cancelable: true }) + return event.defaultPrevented } diff --git a/src/tests/fixtures/hover_to_prefetch.html b/src/tests/fixtures/hover_to_prefetch.html index 5ee33684d..d47be49db 100644 --- a/src/tests/fixtures/hover_to_prefetch.html +++ b/src/tests/fixtures/hover_to_prefetch.html @@ -29,6 +29,10 @@ >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 { await goTo({ page, path: "/hover_to_prefetch.html" }) - await assertPrefetchedOnHover({ page, selector: "#anchor_with_remote_true" }) + await assertPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) await page.evaluate(() => { document.body.addEventListener("turbo:before-prefetch", (event) => { @@ -75,9 +75,24 @@ test("allows to cancel prefetch requests with custom logic", async ({ page }) => }) }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_for_prefetch" }) +}) + +test("it doesn't prefetch UJS links", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_remote_true" }) }) +test("it doesn't prefetch data-turbo-stream links", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_stream" }) +}) + +test("it doesn't prefetch data-turbo-confirm links", async ({ page }) => { + await goTo({ page, path: "/hover_to_prefetch.html" }) + await assertNotPrefetchedOnHover({ page, selector: "#anchor_with_turbo_confirm" }) +}) + 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" }) @@ -153,17 +168,6 @@ test("it caches the request for 1 millisecond when turbo-prefetch-cache-time is 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 prefetches links with inner elements", async ({ page }) => { await goTo({ page, path: "/hover_to_prefetch.html" }) await assertPrefetchedOnHover({ page, selector: "#anchor_with_inner_elements" })