Don't convert data-turbo-stream links to forms (#647)

The initial implementation of the `data-turbo-stream` attribute worked
by treating requests with that attribute as form submissions (which
would usually be `GET` form submissions, unless a different method type
was indicated).

This allowed us to include the `data-turbo-stream` logic in
`FormSubmission`, which was responsible for properly setting the
`Accept` header.

However, there are some downsides to submitting the requests as form
submissions. For example, a `GET` form submission does not include the
search portion of the URL. Additionally, servers are free to respond to
`data-turbo-stream` requests with plain HTML responses, and in that case
we don't want Turbo's handling of the response to differ from what it
would have been if the `data-turbo-stream` attribute wasn't present.

This commit takes a different approach. In this version, elements that
have `data-turbo-stream` continue to be processed by the same object
that they would otherwise, whether that's a `FormSubmission`, a `Visit`
or a `FrameController`. However each of these objects is now aware of
the `data-turbo-stream` attribute, and each will include the Turbo
Stream MIME type when appropriate.

This minimizes the impact of `data-turbo-stream` so that the only effect
it has is on the inclusion of that MIME type.

src/core/drive/form_submission.ts

@@ -161,7 +161,7 @@ export class FormSubmission {
     }
 
     if (this.requestAcceptsTurboStreamResponse(request)) {
-      headers["Accept"] = [StreamMessage.contentType, headers["Accept"]].join(", ")
+      request.acceptResponseType(StreamMessage.contentType)
     }
   }
 

src/core/drive/visit.ts

@@ -1,5 +1,5 @@
 import { Adapter } from "../native/adapter"
-import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request"
+import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
 import { FetchResponse } from "../../http/fetch_response"
 import { History } from "./history"
 import { getAnchor } from "../url"
@@ -8,6 +8,7 @@ import { PageSnapshot } from "./page_snapshot"
 import { Action, ResolvingFunctions } from "../types"
 import { getHistoryMethodForAction, uuid } from "../../util"
 import { PageView } from "./page_view"
+import { StreamMessage } from "../streams/stream_message"
 
 export interface VisitDelegate {
   readonly adapter: Adapter
@@ -49,6 +50,7 @@ export type VisitOptions = {
   restorationIdentifier?: string
   shouldCacheSnapshot: boolean
   frame?: string
+  acceptsStreamResponse: boolean
 }
 
 const defaultOptions: VisitOptions = {
@@ -58,6 +60,7 @@ const defaultOptions: VisitOptions = {
   willRender: true,
   updateHistory: true,
   shouldCacheSnapshot: true,
+  acceptsStreamResponse: false,
 }
 
 export type VisitResponse = {
@@ -96,6 +99,7 @@ export class Visit implements FetchRequestDelegate {
   response?: VisitResponse
   scrolled = false
   shouldCacheSnapshot = true
+  acceptsStreamResponse = false
   snapshotHTML?: string
   snapshotCached = false
   state = VisitState.initialized
@@ -121,6 +125,7 @@ export class Visit implements FetchRequestDelegate {
       willRender,
       updateHistory,
       shouldCacheSnapshot,
+      acceptsStreamResponse,
     } = {
       ...defaultOptions,
       ...options,
@@ -136,6 +141,7 @@ export class Visit implements FetchRequestDelegate {
     this.updateHistory = updateHistory
     this.scrolled = !willRender
     this.shouldCacheSnapshot = shouldCacheSnapshot
+    this.acceptsStreamResponse = acceptsStreamResponse
   }
 
   get adapter() {
@@ -335,6 +341,12 @@ export class Visit implements FetchRequestDelegate {
 
   // Fetch request delegate
 
+  prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
+    if (this.acceptsStreamResponse) {
+      request.acceptResponseType(StreamMessage.contentType)
+    }
+  }
+
   requestStarted() {
     this.startRequest()
   }

src/core/frames/frame_controller.ts

@@ -30,6 +30,7 @@ import { session } from "../index"
 import { isAction, Action } from "../types"
 import { VisitOptions } from "../drive/visit"
 import { TurboBeforeFrameRenderEvent } from "../session"
+import { StreamMessage } from "../streams/stream_message"
 
 export class FrameController
   implements
@@ -59,6 +60,7 @@ export class FrameController
   private frame?: FrameElement
   readonly restorationIdentifier: string
   private previousFrameElement?: FrameElement
+  private currentNavigationElement?: Element
 
   constructor(element: FrameElement) {
     this.element = element
@@ -217,8 +219,12 @@ export class FrameController
 
   // Fetch request delegate
 
-  prepareHeadersForRequest(headers: FetchRequestHeaders, _request: FetchRequest) {
+  prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
     headers["Turbo-Frame"] = this.id
+
+    if (this.currentNavigationElement?.hasAttribute("data-turbo-stream")) {
+      request.acceptResponseType(StreamMessage.contentType)
+    }
   }
 
   requestStarted(_request: FetchRequest) {
@@ -340,7 +346,9 @@ export class FrameController
 
     this.proposeVisitIfNavigatedWithAction(frame, element, submitter)
 
-    frame.src = url
+    this.withCurrentNavigationElement(element, () => {
+      frame.src = url
+    })
   }
 
   private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
@@ -505,6 +513,12 @@ export class FrameController
     callback()
     this.ignoredAttributes.delete(attributeName)
   }
+
+  private withCurrentNavigationElement(element: Element, callback: () => void) {
+    this.currentNavigationElement = element
+    callback()
+    delete this.currentNavigationElement
+  }
 }
 
 function getFrameElementById(id: string | null) {

src/core/session.ts

@@ -191,7 +191,9 @@ export class Session
 
   followedLinkToLocation(link: Element, location: URL) {
     const action = this.getActionForLink(link)
-    this.visit(location.href, { action })
+    const acceptsStreamResponse = link.hasAttribute("data-turbo-stream")
+
+    this.visit(location.href, { action, acceptsStreamResponse })
   }
 
   // Navigator delegate

src/http/fetch_request.ts

@@ -158,6 +158,10 @@ export class FetchRequest {
     return this.abortController.signal
   }
 
+  acceptResponseType(mimeType: string) {
+    this.headers["Accept"] = [mimeType, this.headers["Accept"]].join(", ")
+  }
+
   private async allowRequestToBeIntercepted(fetchOptions: RequestInit) {
     const requestInterception = new Promise((resolve) => (this.resolveRequestPromise = resolve))
     const event = dispatch<TurboBeforeFetchRequestEvent>("turbo:before-fetch-request", {

src/observers/form_link_click_observer.ts

@@ -25,7 +25,7 @@ export class FormLinkClickObserver implements LinkClickObserverDelegate {
   willFollowLinkToLocation(link: Element, location: URL, originalEvent: MouseEvent): boolean {
     return (
       this.delegate.willSubmitFormLinkToLocation(link, location, originalEvent) &&
-      (link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream"))
+      link.hasAttribute("data-turbo-method")
     )
   }
 

src/tests/fixtures/form.html

@@ -296,7 +296,6 @@ <h2>Frame: Form</h2>
     </turbo-frame>
     <a href="/src/tests/fixtures/frames/hello.html" data-turbo-method="get" id="link-method-outside-frame">Method link outside frame</a><br />
     <a href="/__turbo/messages?content=Link!&type=stream" data-turbo-method="post" id="stream-link-method-outside-frame">Stream link outside frame</a>
-    <a href="/__turbo/messages?content=Link!&type=stream" data-turbo-stream id="stream-link-outside-frame">Stream link (no method) outside frame</a>
     <form>
       <a href="/src/tests/fixtures/frames/hello.html" data-turbo-method="get" id="link-method-within-form-outside-frame">Method link within form outside frame</a><br />
       <a href="/__turbo/messages?content=Link!&type=stream" data-turbo-method="post" id="stream-link-method-within-form-outside-frame">Stream link within form outside frame</a>

src/tests/fixtures/visit.html

@@ -19,6 +19,7 @@ <h1>Visit</h1>
       <hr class="push-below-fold">
       <p><a id="below-the-fold-link" href="/src/tests/fixtures/one.html">one.html</a></p>
       <hr class="push-below-fold">
+      <p><a id="stream-link" href="/src/tests/fixtures/one.html?key=value" data-turbo-stream>Stream link with ?key=value</a></p>
     </section>
   </body>
 </html>

src/tests/functional/form_submission_tests.ts

@@ -886,17 +886,10 @@ test("test stream link GET method form submission inside frame", async ({ page }
 test("test stream link inside frame", async ({ page }) => {
   await page.click("#stream-link-inside-frame")
 
-  const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
-
-  assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
-})
-
-test("test stream link outside frame", async ({ page }) => {
-  await page.click("#stream-link-outside-frame")
-
-  const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
+  const { fetchOptions, url } = await nextEventNamed(page, "turbo:before-fetch-request")
 
   assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
+  assert.equal(getSearchParam(url, "content"), "Link!")
 })
 
 test("test link method form submission within form inside frame", async ({ page }) => {

src/tests/functional/visit_tests.ts

@@ -2,6 +2,7 @@ import { Page, test } from "@playwright/test"
 import { assert } from "chai"
 import { get } from "http"
 import {
+  getSearchParam,
   isScrolledToSelector,
   isScrolledToTop,
   nextBeat,
@@ -178,6 +179,14 @@ test("test turbo:before-fetch-response open new site", async ({ page }) => {
   assert.isTrue(fetchResponseResult.responseHTML.indexOf("An element with an ID") > -1)
 })
 
+test("test visits with data-turbo-stream include MIME type & search params", async ({ page }) => {
+  await page.click("#stream-link")
+  const { fetchOptions, url } = await nextEventNamed(page, "turbo:before-fetch-request")
+
+  assert.ok(fetchOptions.headers["Accept"].includes("text/vnd.turbo-stream.html"))
+  assert.equal(getSearchParam(url, "key"), "value")
+})
+
 test("test cache does not override response after redirect", async ({ page }) => {
   await page.evaluate(() => {
     const cachedElement = document.createElement("some-cached-element")