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

@@ -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
 }

src/tests/fixtures/hover_to_prefetch.html

@@ -29,6 +29,10 @@
       >Won't prefetch when hovering me</a>
     <a href="/src/tests/fixtures/prefetched.html" id="anchor_with_remote_true" data-remote="true"
       >Won't prefetch when hovering me</a>
+    <a href="/src/tests/fixtures/prefetched.html" id="anchor_with_turbo_stream" data-turbo-stream="true"
+      >Won't prefetch when hovering me</a>
+    <a href="/src/tests/fixtures/prefetched.html" id="anchor_with_turbo_confirm" data-turbo-confirm="Are you sure?"
+      >Won't prefetch when hovering me</a>
     <a href="/src/tests/fixtures/hover_to_prefetch.html" id="anchor_for_same_location"
       >Won't prefetch when hovering me</a>
     <a href="/src/tests/fixtures/prefetched.html?foo=bar" id="anchor_for_same_location_with_query"

src/tests/functional/link_prefetch_observer_tests.js

@@ -65,7 +65,7 @@ test("it doesn't prefetch the page when link has data-turbo=false", async ({ pag
 test("allows to cancel prefetch requests with custom logic", async ({ page }) => {
   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" })