Skip to content

Commit

Permalink
Extract FrameVisit to drive FrameController
Browse files Browse the repository at this point in the history 
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, when the `FrameRedirector` class listens for page-wide
`click` and `submit` events, it 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 life cycle 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 for `FrameElementDelegate`
implementations 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
  • Loading branch information
@seanpdoyle @shiftyp
seanpdoyle authored and shiftyp committed Sep 10, 2023
1 parent 1f3416f commit 44212ed
Show file tree
Hide file tree
Showing 8 changed files with 432 additions and 191 deletions.
231 changes: 70 additions & 161 deletions src/core/frames/frame_controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,10 @@ import {
FrameLoadingStyle,
FrameElementObservedAttribute,
} from "../../elements/frame_element"
import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetch_request"
import { FetchRequest, TurboFetchRequestErrorEvent } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
import {
clearBusyState,
dispatch,
getAttribute,
parseHTMLDocument,
markAsBusy,
uuid,
getHistoryMethodForAction,
getVisitAction,
} from "../../util"
import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
import { dispatch, getAttribute, uuid, getHistoryMethodForAction } from "../../util"
import { Snapshot } from "../snapshot"
import { ViewDelegate, ViewRenderOptions } from "../view"
import { Locatable, getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
Expand All @@ -28,9 +18,9 @@ import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../obse
import { FrameRenderer } from "./frame_renderer"
import { session } from "../index"
import { Action } from "../types"
import { FrameVisit, FrameVisitDelegate, FrameVisitOptions } from "./frame_visit"
import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent } from "../session"
import { StreamMessage } from "../streams/stream_message"
import { PageSnapshot } from "../drive/page_snapshot"
import { TurboFrameMissingError } from "../errors"

Expand All @@ -40,11 +30,10 @@ export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: Vi
export class FrameController
implements
AppearanceObserverDelegate<FrameElement>,
FetchRequestDelegate,
FormSubmitObserverDelegate,
FormSubmissionDelegate,
FrameElementDelegate,
FormLinkClickObserverDelegate,
FrameVisitDelegate,
LinkInterceptorDelegate,
ViewDelegate<FrameElement, Snapshot<FrameElement>>
{
Expand All @@ -54,17 +43,12 @@ export class FrameController
readonly formLinkClickObserver: FormLinkClickObserver
readonly linkInterceptor: LinkInterceptor
readonly formSubmitObserver: FormSubmitObserver
formSubmission?: FormSubmission
fetchResponseLoaded = (_fetchResponse: FetchResponse) => {}
private currentFetchRequest: FetchRequest | null = null
private resolveVisitPromise = () => {}
frameVisit?: FrameVisit
private connected = false
private hasBeenLoaded = false
private ignoredAttributes: Set<FrameElementObservedAttribute> = new Set()
private action: Action | null = null
readonly restorationIdentifier: string
private previousFrameElement?: FrameElement
private currentNavigationElement?: Element

constructor(element: FrameElement) {
this.element = element
Expand Down Expand Up @@ -100,6 +84,11 @@ export class FrameController
}
}

visit(options: FrameVisitOptions): Promise<void> {
const frameVisit = new FrameVisit(this, this.element, options)
return frameVisit.start()
}

disabledChanged() {
if (this.loadingStyle == FrameLoadingStyle.eager) {
this.loadSourceURL()
Expand Down Expand Up @@ -145,39 +134,30 @@ export class FrameController

private async loadSourceURL() {
if (this.enabled && this.isActive && !this.complete && this.sourceURL) {
this.element.loaded = this.visit(expandURL(this.sourceURL))
this.appearanceObserver.stop()
await this.element.loaded
this.hasBeenLoaded = true
await this.visit({ url: this.sourceURL })
}
}

async loadResponse(fetchResponse: FetchResponse) {
async loadResponse(fetchResponse: FetchResponse, frameVisit: FrameVisit) {
if (fetchResponse.redirected || (fetchResponse.succeeded && fetchResponse.isHTML)) {
this.sourceURL = fetchResponse.response.url
}

try {
const html = await fetchResponse.responseHTML
if (html) {
const document = parseHTMLDocument(html)
const pageSnapshot = PageSnapshot.fromDocument(document)

if (pageSnapshot.isVisitable) {
await this.loadFrameResponse(fetchResponse, document)
} else {
await this.handleUnvisitableFrameResponse(fetchResponse)
}
const html = await fetchResponse.responseHTML
if (html) {
const pageSnapshot = PageSnapshot.fromHTMLString(html)

if (pageSnapshot.isVisitable) {
await this.loadFrameResponse(fetchResponse, pageSnapshot, frameVisit)
} else {
await this.handleUnvisitableFrameResponse(fetchResponse)
}
} finally {
this.fetchResponseLoaded = () => {}
}
}

// Appearance observer delegate

elementAppearedInViewport(element: FrameElement) {
this.proposeVisitIfNavigatedWithAction(element, element)
elementAppearedInViewport(_element: FrameElement) {
this.loadSourceURL()
}

Expand Down Expand Up @@ -209,81 +189,48 @@ export class FrameController
}

formSubmitted(element: HTMLFormElement, submitter?: HTMLElement) {
if (this.formSubmission) {
this.formSubmission.stop()
}

this.formSubmission = new FormSubmission(this, element, submitter)
const { fetchRequest } = this.formSubmission
this.prepareRequest(fetchRequest)
this.formSubmission.start()
}

// Fetch request delegate

prepareRequest(request: FetchRequest) {
request.headers["Turbo-Frame"] = this.id

if (this.currentNavigationElement?.hasAttribute("data-turbo-stream")) {
request.acceptResponseType(StreamMessage.contentType)
}
}

requestStarted(_request: FetchRequest) {
markAsBusy(this.element)
}

requestPreventedHandlingResponse(_request: FetchRequest, _response: FetchResponse) {
this.resolveVisitPromise()
}

async requestSucceededWithResponse(request: FetchRequest, response: FetchResponse) {
await this.loadResponse(response)
this.resolveVisitPromise()
}

async requestFailedWithResponse(request: FetchRequest, response: FetchResponse) {
await this.loadResponse(response)
this.resolveVisitPromise()
const frame = this.findFrameElement(element, submitter)
frame.delegate.visit(FrameVisit.optionsForSubmit(element, submitter))
}

requestErrored(request: FetchRequest, error: Error) {
console.error(error)
this.resolveVisitPromise()
}
// Frame visit delegate

requestFinished(_request: FetchRequest) {
clearBusyState(this.element)
shouldVisit(_frameVisit: FrameVisit) {
return this.enabled && this.isActive
}

// Form submission delegate

formSubmissionStarted({ formElement }: FormSubmission) {
markAsBusy(formElement, this.findFrameElement(formElement))
visitStarted(frameVisit: FrameVisit) {
this.ignoringChangesToAttribute("complete", () => {
this.frameVisit?.stop()
this.frameVisit = frameVisit
this.element.removeAttribute("complete")
})
}

formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) {
const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter)

frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
frame.delegate.loadResponse(response)
async visitSucceededWithResponse(frameVisit: FrameVisit, response: FetchResponse) {
await this.loadResponse(response, frameVisit)

if (!formSubmission.isSafe) {
if (!frameVisit.isSafe) {
session.clearCache()
}
}

formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
this.element.delegate.loadResponse(fetchResponse)
async visitFailedWithResponse(frameVisit: FrameVisit, response: FetchResponse) {
await this.loadResponse(response, frameVisit)

session.clearCache()
}

formSubmissionErrored(formSubmission: FormSubmission, error: Error) {
visitErrored(frameVisit: FrameVisit, request: FetchRequest, error: Error) {
console.error(error)
dispatch<TurboFetchRequestErrorEvent>("turbo:fetch-request-error", {
target: this.element,
detail: { request, error },
})
}

formSubmissionFinished({ formElement }: FormSubmission) {
clearBusyState(formElement, this.findFrameElement(formElement))
visitCompleted(_frameVisit: FrameVisit) {
this.hasBeenLoaded = true
}

// View delegate
Expand Down Expand Up @@ -335,83 +282,51 @@ export class FrameController

// Private

private async loadFrameResponse(fetchResponse: FetchResponse, document: Document) {
const newFrameElement = await this.extractForeignFrameElement(document.body)
private async loadFrameResponse(fetchResponse: FetchResponse, pageSnapshot: PageSnapshot, frameVisit: FrameVisit) {
const newFrameElement = await this.extractForeignFrameElement(pageSnapshot.element)

if (newFrameElement) {
const snapshot = new Snapshot(newFrameElement)
const renderer = new FrameRenderer(this, this.view.snapshot, snapshot, FrameRenderer.renderElement, false, false)
if (this.view.renderPromise) await this.view.renderPromise
this.changeHistory()
this.changeHistory(frameVisit.action)

await this.view.render(renderer)
this.complete = true
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
this.fetchResponseLoaded(fetchResponse)
this.proposeVisitIfNavigatedWithAction(frameVisit, fetchResponse)
} else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
this.handleFrameMissingFromResponse(fetchResponse)
}
}

private async visit(url: URL) {
const request = new FetchRequest(this, FetchMethod.get, 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) {
const frame = this.findFrameElement(element)
frame.delegate.visit(FrameVisit.optionsForClick(element, expandURL(url)))
}

private navigateFrame(element: Element, url: string, submitter?: HTMLElement) {
const frame = this.findFrameElement(element, submitter)

frame.delegate.proposeVisitIfNavigatedWithAction(frame, element, submitter)

this.withCurrentNavigationElement(element, () => {
frame.src = url
})
}

proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
this.action = getVisitAction(submitter, element, frame)

if (this.action) {
const pageSnapshot = PageSnapshot.fromElement(frame).clone()
const { visitCachedSnapshot } = frame.delegate

frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
if (frame.src) {
const { statusCode, redirected } = fetchResponse
const responseHTML = frame.ownerDocument.documentElement.outerHTML
const response = { statusCode, redirected, responseHTML }
const options: Partial<VisitOptions> = {
response,
visitCachedSnapshot,
willRender: false,
updateHistory: false,
restorationIdentifier: this.restorationIdentifier,
snapshot: pageSnapshot,
}

if (this.action) options.action = this.action

session.visit(frame.src, options)
}
private proposeVisitIfNavigatedWithAction({ action, element, snapshot }: FrameVisit, fetchResponse: FetchResponse) {
if (element.src && action) {
const { statusCode, redirected } = fetchResponse
const responseHTML = element.ownerDocument.documentElement.outerHTML
const options: Partial<VisitOptions> = {
action,
snapshot,
response: { statusCode, redirected, responseHTML },
restorationIdentifier: this.restorationIdentifier,
updateHistory: false,
visitCachedSnapshot: this.visitCachedSnapshot,
willRender: false,
}

session.visit(element.src, options)
}
}

changeHistory() {
if (this.action) {
const method = getHistoryMethodForAction(this.action)
changeHistory(action: Action | null) {
if (action) {
const method = getHistoryMethodForAction(action)
session.history.update(method, expandURL(this.element.src || ""), this.restorationIdentifier)
}
}
Expand Down Expand Up @@ -553,7 +468,7 @@ export class FrameController
}

get isLoading() {
return this.formSubmission !== undefined || this.resolveVisitPromise() !== undefined
return this.frameVisit !== undefined
}

get complete() {
Expand Down Expand Up @@ -589,12 +504,6 @@ 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) {
Expand Down
Loading

0 comments on commit 44212ed

Please sign in to comment.