Extract FrameVisit to drive FrameController

The problem
---

Programmatically driving a `<turbo-frame>` element when its `[src]`
attribute changes is a suitable end-user experience in consumer
applications. It's a fitting black-box interface for the outside world:
change the value of the attribute and let Turbo handle the rest.

However, internally, it's a lossy abstraction.

For example, the `FrameRedirector` class listens for page-wide events
`click` and `submit` events, determines if their targets are meant to
drive a `<turbo-frame>` element by:

1. finding an element that matches a clicked `<a>` element's `[data-turbo-frame]` attribute
2. finding an element that matches a submitted `<form>` element's `[data-turbo-frame]` attribute
3. finding an element that matches a submitted `<form>` element's
   _submitter's_ `[data-turbo-frame]` attribute
4. finding the closest `<turbo-frame>` ancestor to the `<a>` or `<form>`

Once it finds the matching frame element, it disposes of all that
additional context and navigates the `<turbo-frame>` by updating its
`[src]` attribute. This makes it impossible to control various aspects
of the frame navigation (like its "rendering" explored in
[#146][]) outside of its destination URL.

Similarly, since a `<form>` and submitter pairing have an impact on
which `<turbo-frame>` is navigated, the `FrameController` implementation
passes around a `HTMLFormElement` and `HTMLSubmitter?` data clump and
constantly re-fetches a matching `<turbo-frame>` instance.

Outside of frames, page-wide navigation is driven by a `Visit` instance
that manages the HTTP lifecycle and delegates along the way to a
`VisitDelegate`. It also pairs calls to visit with a `VisitOption`
object to capture additional context.

The proposal
---

This commit introduces the `FrameVisit` class. It serves as an
encapsulation of the `FetchRequest` and `FormSubmission` lifecycle
events involved in navigating a frame.

It's implementation draws inspiration from the `Visit`, `VisitDelegate`,
and `VisitOptions` pairing. Since the `FrameVisit` knows how to unify
both `FetchRequest` and `FormSubmission` hooks, the resulting callbacks
fired from within the `FrameController` are flat and consistent.

Extra benefits
---

The biggest benefit is the introduction of a DRY abstraction to
manage the behind the scenes HTTP calls necessary to drive a
`<turbo-frame>`.

With the introduction of the `FrameVisit` concept, we can also declare a
`visit()` and `submit()` method to the `FrameElementDelegate` in the
place of other implementation-specific methods like `loadResponse()` and
`formSubmissionIntercepted()`.

In addition, these changes have the potential to close
[#326][], since we can consistently invoke
`loadResponse()` across `<a>`-click-initiated and
`<form>`-submission-initiated visits. To ensure that's the case, this
commit adds test coverage for navigating a `<turbo-frame>` by making a
`GET` request to an endpoint that responds with a `500` status.

[#146]: #146
[#326]: #326

src/core/drive/navigator.ts

@@ -1,10 +1,9 @@
-import { Action, isAction } from "../types"
+import { Action } from "../types"
 import { FetchMethod } from "../../http/fetch_request"
 import { FetchResponse } from "../../http/fetch_response"
 import { FormSubmission } from "./form_submission"
 import { expandURL, getAnchor, getRequestURL, Locatable, locationIsVisitable } from "../url"
-import { getAttribute } from "../../util"
-import { Visit, VisitDelegate, VisitOptions } from "./visit"
+import { Visit, VisitDelegate, VisitOptions, getVisitAction } from "./visit"
 import { PageSnapshot } from "./page_snapshot"
 
 export type NavigatorDelegate = VisitDelegate & {
@@ -157,9 +156,7 @@ export class Navigator {
     return this.history.restorationIdentifier
   }
 
-  getActionForFormSubmission(formSubmission: FormSubmission): Action {
-    const { formElement, submitter } = formSubmission
-    const action = getAttribute("data-turbo-action", submitter, formElement)
-    return isAction(action) ? action : "advance"
+  getActionForFormSubmission({ formElement, submitter }: FormSubmission): Action {
+    return getVisitAction(submitter, formElement) || "advance"
   }
 }

src/core/drive/visit.ts

@@ -4,8 +4,8 @@ import { FetchResponse } from "../../http/fetch_response"
 import { History } from "./history"
 import { getAnchor } from "../url"
 import { PageSnapshot } from "./page_snapshot"
-import { Action } from "../types"
-import { uuid } from "../../util"
+import { Action, isAction } from "../types"
+import { getAttribute, uuid } from "../../util"
 import { PageView } from "./page_view"
 
 export interface VisitDelegate {
@@ -413,6 +413,16 @@ export class Visit implements FetchRequestDelegate {
   }
 }
 
+export function getVisitAction(...elements: (Element|undefined)[]): Action | null {
+  const action = getAttribute("data-turbo-action", ...elements)
+
+  if (isAction(action)) {
+    return action
+  } else {
+    return null
+  }
+}
+
 function isSuccessful(statusCode: number) {
   return statusCode >= 200 && statusCode < 300
 }

src/core/frames/frame_controller.ts

@@ -1,29 +1,26 @@
 import { FrameElement, FrameElementDelegate, FrameLoadingStyle } from "../../elements/frame_element"
-import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
+import { FrameVisit, FrameVisitDelegate, FrameVisitOptions } from "./frame_visit"
 import { FetchResponse } from "../../http/fetch_response"
 import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
 import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
-import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
-import { getAction, expandURL, urlsAreEqual, locationIsVisitable, Locatable } from "../url"
+import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
 import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor"
 import { FrameView } from "./frame_view"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
 import { FrameRenderer } from "./frame_renderer"
 import { session } from "../index"
-import { isAction } from "../types"
+import { Action } from "../types"
 
-export class FrameController implements AppearanceObserverDelegate, FetchRequestDelegate, FormInterceptorDelegate, FormSubmissionDelegate, FrameElementDelegate, LinkInterceptorDelegate, ViewDelegate<Snapshot<FrameElement>> {
+export class FrameController implements AppearanceObserverDelegate, FormInterceptorDelegate, FrameElementDelegate, FrameVisitDelegate, LinkInterceptorDelegate, ViewDelegate<Snapshot<FrameElement>> {
   readonly element: FrameElement
   readonly view: FrameView
   readonly appearanceObserver: AppearanceObserver
   readonly linkInterceptor: LinkInterceptor
   readonly formInterceptor: FormInterceptor
   currentURL?: string | null
-  formSubmission?: FormSubmission
-  private currentFetchRequest: FetchRequest | null = null
-  private resolveVisitPromise = () => {}
+  frameVisit?: FrameVisit
   private connected = false
   private hasBeenLoaded = false
   private settingSourceURL = false
@@ -60,13 +57,13 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
 
   disabledChanged() {
     if (this.loadingStyle == FrameLoadingStyle.eager) {
-      this.loadSourceURL()
+      this.visit()
     }
   }
 
   sourceURLChanged() {
     if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) {
-      this.loadSourceURL()
+      this.visit()
     }
   }
 
@@ -75,30 +72,74 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
       this.appearanceObserver.start()
     } else {
       this.appearanceObserver.stop()
-      this.loadSourceURL()
+      this.visit()
     }
   }
 
-  async loadSourceURL() {
-    if (!this.settingSourceURL && this.enabled && this.isActive && (this.reloadable || this.sourceURL != this.currentURL)) {
-      const previousURL = this.currentURL
-      this.currentURL = this.sourceURL
-      if (this.sourceURL) {
-        try {
-          this.element.loaded = this.visit(this.sourceURL)
-          this.appearanceObserver.stop()
-          await this.element.loaded
-          this.hasBeenLoaded = true
-        } catch (error) {
-          this.currentURL = previousURL
-          throw error
-        }
-      }
+  visit(options: Partial<FrameVisitOptions> = {}) {
+    const { url } = options
+
+    if (url) this.sourceURL = url
+
+    if (this.sourceURL) {
+      const frameVisit = new FrameVisit(this, this.element, { url: this.sourceURL, ...options })
+      frameVisit.start()
+    }
+  }
+
+  submit(options: Partial<FrameVisitOptions> = {}) {
+    const { submit } = options
+
+    if (submit) {
+      const frameVisit = new FrameVisit(this, this.element, options)
+      frameVisit.start()
     }
   }
 
-  async loadResponse(fetchResponse: FetchResponse) {
-    if (fetchResponse.redirected) {
+  // Frame visit delegate
+
+  shouldVisit({ isFormSubmission }: FrameVisit) {
+    return !this.settingSourceURL && this.enabled && this.isActive && (this.reloadable || (this.sourceURL != this.currentURL || isFormSubmission))
+  }
+
+  visitStarted(frameVisit: FrameVisit) {
+    this.frameVisit?.stop()
+    this.frameVisit = frameVisit
+
+    const busyElements = [ this.element, document.documentElement ]
+    busyElements.forEach(markAsBusy)
+
+    if (frameVisit.options.url) {
+      this.currentURL = frameVisit.options.url
+    }
+
+    this.appearanceObserver.stop()
+  }
+
+  async visitSucceeded({ action }: FrameVisit, response: FetchResponse) {
+    await this.loadResponse(response, action)
+  }
+
+  async visitFailed({ action }: FrameVisit, response: FetchResponse) {
+    await this.loadResponse(response, action)
+  }
+
+  visitErrored(frameVisit: FrameVisit, error: Error) {
+    console.error(error)
+    this.currentURL = frameVisit.previousURL
+    this.view.invalidate()
+    throw error
+  }
+
+  visitCompleted(frameVisit: FrameVisit) {
+    const busyElements = [ this.element, document.documentElement ]
+    busyElements.forEach(clearBusyState)
+    this.hasBeenLoaded = true
+  }
+
+  async loadResponse(fetchResponse: FetchResponse, action: Action | null) {
+    const { location, redirected, statusCode } = fetchResponse
+    if (redirected) {
       this.sourceURL = fetchResponse.response.url
     }
 
@@ -112,6 +153,11 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
         await this.view.render(renderer)
         session.frameRendered(fetchResponse, this.element)
         session.frameLoaded(this.element)
+
+        if (action) {
+          const response = { responseHTML: html, redirected, statusCode }
+          session.visit(location, { action, response, willRender: false })
+        }
       }
     } catch (error) {
       console.error(error)
@@ -122,7 +168,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   // Appearance observer delegate
 
   elementAppearedInViewport(element: Element) {
-    this.loadSourceURL()
+    this.visit()
   }
 
   // Link interceptor delegate
@@ -147,74 +193,9 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   }
 
   formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement) {
-    if (this.formSubmission) {
-      this.formSubmission.stop()
-    }
-
-    this.reloadable = false
-    this.formSubmission = new FormSubmission(this, element, submitter)
-    const { fetchRequest } = this.formSubmission
-    this.prepareHeadersForRequest(fetchRequest.headers, fetchRequest)
-    this.formSubmission.start()
-  }
-
-  // Fetch request delegate
-
-  prepareHeadersForRequest(headers: FetchRequestHeaders, request: FetchRequest) {
-    headers["Turbo-Frame"] = this.id
-  }
-
-  requestStarted(request: FetchRequest) {
-    [ this.element, document.documentElement ].forEach(markAsBusy)
-  }
-
-  requestPreventedHandlingResponse(request: FetchRequest, response: FetchResponse) {
-    this.resolveVisitPromise()
-  }
-
-  async requestSucceededWithResponse(request: FetchRequest, response: FetchResponse) {
-    await this.loadResponse(response)
-    this.resolveVisitPromise()
-  }
-
-  requestFailedWithResponse(request: FetchRequest, response: FetchResponse) {
-    console.error(response)
-    this.resolveVisitPromise()
-  }
-
-  requestErrored(request: FetchRequest, error: Error) {
-    console.error(error)
-    this.resolveVisitPromise()
-  }
-
-  requestFinished(request: FetchRequest) {
-    [ this.element, document.documentElement ].forEach(clearBusyState)
-  }
-
-  // Form submission delegate
-
-  formSubmissionStarted({ formElement }: FormSubmission) {
-    [ formElement, this.findFrameElement(formElement), document.documentElement ].forEach(markAsBusy)
-  }
-
-  formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) {
-    const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter)
-
-    this.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
-
-    frame.delegate.loadResponse(response)
-  }
-
-  formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
-    this.element.delegate.loadResponse(fetchResponse)
-  }
-
-  formSubmissionErrored(formSubmission: FormSubmission, error: Error) {
-    console.error(error)
-  }
-
-  formSubmissionFinished({ formElement }: FormSubmission) {
-    [ formElement, this.findFrameElement(formElement), document.documentElement ].forEach(clearBusyState)
+    const frame = this.findFrameElement(element, submitter)
+    frame.removeAttribute("reloadable")
+    frame.delegate.submit(FrameVisit.optionsForSubmit(element, submitter))
   }
 
   // View delegate
@@ -231,44 +212,11 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
 
   // Private
 
-  private async visit(url: Locatable) {
-    const request = new FetchRequest(this, FetchMethod.get, expandURL(url), new URLSearchParams, this.element)
-
-    this.currentFetchRequest?.cancel()
-    this.currentFetchRequest = request
-
-    return new Promise<void>(resolve => {
-      this.resolveVisitPromise = () => {
-        this.resolveVisitPromise = () => {}
-        this.currentFetchRequest = null
-        resolve()
-      }
-      request.perform()
-    })
-  }
-
   private navigateFrame(element: Element, url: string, submitter?: HTMLElement) {
     const frame = this.findFrameElement(element, submitter)
 
-    this.proposeVisitIfNavigatedWithAction(frame, element, submitter)
-
     frame.setAttribute("reloadable", "")
-    frame.src = url
-  }
-
-  private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
-    const action = getAttribute("data-turbo-action", submitter, element, frame)
-
-    if (isAction(action)) {
-      const proposeVisit = async (event: Event) => {
-        const { detail: { fetchResponse: { location, redirected, statusCode } } } = event as CustomEvent
-        const responseHTML = document.documentElement.outerHTML
-
-        session.visit(location, { willRender: false, action, response: { redirected, responseHTML, statusCode } })
-      }
-
-      frame.addEventListener("turbo:frame-render", proposeVisit , { once: true })
-    }
+    frame.delegate.visit(FrameVisit.optionsForClick(element, url))
   }
 
   private findFrameElement(element: Element, submitter?: HTMLElement) {
@@ -375,7 +323,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   }
 
   get isLoading() {
-    return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined
+    return this.frameVisit !== undefined
   }
 
   get isActive() {

src/core/frames/frame_redirector.ts

@@ -2,6 +2,7 @@ import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor"
 import { FrameElement } from "../../elements/frame_element"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
 import { expandURL, getAction, locationIsVisitable } from "../url"
+import { FrameVisit } from "./frame_visit"
 
 export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptorDelegate {
   readonly element: Element
@@ -31,7 +32,8 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor
   linkClickIntercepted(element: Element, url: string) {
     const frame = this.findFrameElement(element)
     if (frame) {
-      frame.delegate.linkClickIntercepted(element, url)
+      frame.setAttribute("reloadable", "")
+      frame.delegate.visit(FrameVisit.optionsForClick(element, url))
     }
   }
 
@@ -43,7 +45,7 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor
     const frame = this.findFrameElement(element, submitter)
     if (frame) {
       frame.removeAttribute("reloadable")
-      frame.delegate.formSubmissionIntercepted(element, submitter)
+      frame.delegate.submit(FrameVisit.optionsForSubmit(element, submitter))
     }
   }