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
[hotwired#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
[hotwired#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.

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

src/core/frames/frame_controller.ts

@@ -1,28 +1,25 @@
 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 { parseHTMLDocument } from "../../util"
-import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
+import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
-import { expandURL, urlsAreEqual, Locatable } from "../url"
+import { urlsAreEqual } 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"
 
-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
@@ -59,13 +56,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()
     }
   }
 
@@ -74,29 +71,69 @@ 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
-          session.frameLoaded(this.element)
-        } 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()
     }
   }
 
+  // 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
+
+    this.element.setAttribute("busy", "")
+
+    if (frameVisit.options.url) {
+      this.currentURL = frameVisit.options.url
+    }
+
+    this.appearanceObserver.stop()
+  }
+
+  async visitSucceeded(frameVisit: FrameVisit, response: FetchResponse) {
+    await this.loadResponse(response)
+  }
+
+  async visitFailed(frameVisit: FrameVisit, response: FetchResponse) {
+    await this.loadResponse(response)
+  }
+
+  visitErrored(frameVisit: FrameVisit, error: Error) {
+    console.error(error)
+    this.currentURL = frameVisit.previousURL
+    this.view.invalidate()
+    throw error
+  }
+
+  visitCompleted(frameVisit: FrameVisit) {
+    this.element.removeAttribute("busy")
+    this.hasBeenLoaded = true
+  }
+
   async loadResponse(fetchResponse: FetchResponse) {
     if (fetchResponse.redirected) {
       this.sourceURL = fetchResponse.response.url
@@ -111,6 +148,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
         if (this.view.renderPromise) await this.view.renderPromise
         await this.view.render(renderer)
         session.frameRendered(fetchResponse, this.element);
+        session.frameLoaded(this.element)
       }
     } catch (error) {
       console.error(error)
@@ -121,7 +159,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   // Appearance observer delegate
 
   elementAppearedInViewport(element: Element) {
-    this.loadSourceURL()
+    this.visit()
   }
 
   // Link interceptor delegate
@@ -146,73 +184,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.setAttribute("busy", "")
-  }
-
-  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.removeAttribute("busy")
-  }
-
-  // Form submission delegate
-
-  formSubmissionStarted(formSubmission: FormSubmission) {
-    const frame = this.findFrameElement(formSubmission.formElement)
-    frame.setAttribute("busy", "")
-  }
-
-  formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) {
-    const frame = this.findFrameElement(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(formSubmission: FormSubmission) {
-    const frame = this.findFrameElement(formSubmission.formElement)
-    frame.removeAttribute("busy")
+    const frame = this.findFrameElement(element, submitter)
+    frame.removeAttribute("reloadable")
+    frame.delegate.submit(FrameVisit.optionsForSubmit(element, submitter))
   }
 
   // View delegate
@@ -229,26 +203,10 @@ 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)
     frame.setAttribute("reloadable", "")
-    frame.src = url
+    frame.delegate.visit(FrameVisit.optionsForClick(element, url))
   }
 
   private findFrameElement(element: Element, submitter?: HTMLElement) {
@@ -345,7 +303,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

@@ -1,6 +1,7 @@
 import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor"
 import { FrameElement } from "../../elements/frame_element"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
+import { FrameVisit } from "./frame_visit"
 
 export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptorDelegate {
   readonly element: Element
@@ -31,7 +32,7 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor
     const frame = this.findFrameElement(element)
     if (frame) {
       frame.setAttribute("reloadable", "")
-      frame.src = url
+      frame.delegate.visit(FrameVisit.optionsForClick(element, url))
     }
   }
 
@@ -43,7 +44,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))
     }
   }