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
`Visit` delegate. It also pairs calls to visit with an option 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` class's delegate
and option structures. 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 `FrameElement` delegate
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
seanpdoyle committed Sep 14, 2023
1 parent 32cadc0 commit 845459d
Show file tree
Hide file tree
Showing 7 changed files with 349 additions and 159 deletions.
220 changes: 69 additions & 151 deletions src/core/frames/frame_controller.js
View file
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
import { FrameElement, FrameLoadingStyle } from "../../elements/frame_element"
import { FetchMethod, FetchRequest } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { AppearanceObserver } from "../../observers/appearance_observer"
import {
clearBusyState,
dispatch,
getAttribute,
parseHTMLDocument,
markAsBusy,
uuid,
getHistoryMethodForAction,
getVisitAction
getHistoryMethodForAction
} from "../../util"
import { FormSubmission } from "../drive/form_submission"
import { Snapshot } from "../snapshot"
import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
import { FormSubmitObserver } from "../../observers/form_submit_observer"
Expand All @@ -21,18 +15,15 @@ import { LinkInterceptor } from "./link_interceptor"
import { FormLinkClickObserver } from "../../observers/form_link_click_observer"
import { FrameRenderer } from "./frame_renderer"
import { session } from "../index"
import { StreamMessage } from "../streams/stream_message"
import { PageSnapshot } from "../drive/page_snapshot"
import { TurboFrameMissingError } from "../errors"
import { FrameVisit } from "./frame_visit"

export class FrameController {
fetchResponseLoaded = (_fetchResponse) => Promise.resolve()
#currentFetchRequest = null
#resolveVisitPromise = () => {}
#connected = false
#hasBeenLoaded = false
#ignoredAttributes = new Set()
action = null
#frameVisit = null

constructor(element) {
this.element = element
Expand Down Expand Up @@ -70,6 +61,11 @@ export class FrameController {
}
}

visit(options) {
const frameVisit = new FrameVisit(this, this.element, options)
return frameVisit.start()
}

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

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) {
async loadResponse(fetchResponse, 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 = () => Promise.resolve()
}
}

// Appearance observer delegate

elementAppearedInViewport(element) {
this.proposeVisitIfNavigatedWithAction(element, element)
this.#loadSourceURL()
}

Expand Down Expand Up @@ -179,81 +166,48 @@ export class FrameController {
}

formSubmitted(element, submitter) {
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) {
request.headers["Turbo-Frame"] = this.id

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

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

requestPreventedHandlingResponse(_request, _response) {
this.#resolveVisitPromise()
}

async requestSucceededWithResponse(request, response) {
await this.loadResponse(response)
this.#resolveVisitPromise()
}

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

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

requestFinished(_request) {
clearBusyState(this.element)
shouldVisitFrame(_frameVisit) {
return this.enabled && this.isActive
}

// Form submission delegate

formSubmissionStarted({ formElement }) {
markAsBusy(formElement, this.#findFrameElement(formElement))
frameVisitStarted(frameVisit) {
this.#ignoringChangesToAttribute("complete", () => {
this.#frameVisit?.stop()
this.#frameVisit = frameVisit
this.element.removeAttribute("complete")
})
}

formSubmissionSucceededWithResponse(formSubmission, response) {
const frame = this.#findFrameElement(formSubmission.formElement, formSubmission.submitter)
async frameVisitSucceededWithResponse(frameVisit, response) {
await this.loadResponse(response, frameVisit)

frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
frame.delegate.loadResponse(response)

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

formSubmissionFailedWithResponse(formSubmission, fetchResponse) {
this.element.delegate.loadResponse(fetchResponse)
async frameVisitFailedWithResponse(frameVisit, response) {
await this.loadResponse(response, frameVisit)

session.clearCache()
}

formSubmissionErrored(formSubmission, error) {
frameVisitErrored(frameVisit, fetchRequest, error) {
console.error(error)
dispatch("turbo:fetch-request-error", {
target: this.element,
detail: { request: fetchRequest, error }
})
}

formSubmissionFinished({ formElement }) {
clearBusyState(formElement, this.#findFrameElement(formElement))
frameVisitCompleted(_frameVisit) {
this.hasBeenLoaded = true
}

// View delegate
Expand Down Expand Up @@ -302,83 +256,53 @@ export class FrameController {

// Private

async #loadFrameResponse(fetchResponse, document) {
const newFrameElement = await this.extractForeignFrameElement(document.body)
async #loadFrameResponse(fetchResponse, pageSnapshot, 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)
await this.fetchResponseLoaded(fetchResponse)
this.#proposeVisitIfNavigatedWithAction(frameVisit, fetchResponse)
} else if (this.#willHandleFrameMissingFromResponse(fetchResponse)) {
this.#handleFrameMissingFromResponse(fetchResponse)
}
}

async #visit(url) {
const request = new FetchRequest(this, FetchMethod.get, url, new URLSearchParams(), this.element)

this.#currentFetchRequest?.cancel()
this.#currentFetchRequest = request

return new Promise((resolve) => {
this.#resolveVisitPromise = () => {
this.#resolveVisitPromise = () => {}
this.#currentFetchRequest = null
resolve()
}
request.perform()
})
}

#navigateFrame(element, url, submitter) {
const frame = this.#findFrameElement(element, submitter)

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

this.#withCurrentNavigationElement(element, () => {
frame.src = url
})
#navigateFrame(element, url) {
const frame = this.#findFrameElement(element)
frame.delegate.visit(FrameVisit.optionsForClick(element, expandURL(url)))
}

proposeVisitIfNavigatedWithAction(frame, element, submitter) {
this.action = getVisitAction(submitter, element, frame)

if (this.action) {
const pageSnapshot = PageSnapshot.fromElement(frame).clone()
const { visitCachedSnapshot } = frame.delegate
#proposeVisitIfNavigatedWithAction(frameVisit, fetchResponse) {
const { action, frameElement, snapshot } = frameVisit

frame.delegate.fetchResponseLoaded = async (fetchResponse) => {
if (frame.src) {
const { statusCode, redirected } = fetchResponse
const responseHTML = await fetchResponse.responseHTML
const response = { statusCode, redirected, responseHTML }
const options = {
response,
visitCachedSnapshot,
willRender: false,
updateHistory: false,
restorationIdentifier: this.restorationIdentifier,
snapshot: pageSnapshot
}

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

session.visit(frame.src, options)
}
if (frameElement.src && action) {
const { statusCode, redirected } = fetchResponse
const responseHTML = frameElement.ownerDocument.documentElement.outerHTML
const options = {
action,
snapshot,
response: { statusCode, redirected, responseHTML },
restorationIdentifier: this.restorationIdentifier,
updateHistory: false,
visitCachedSnapshot: this.visitCachedSnapshot,
willRender: false
}

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

changeHistory() {
if (this.action) {
const method = getHistoryMethodForAction(this.action)
changeHistory(visitAction) {
if (visitAction) {
const method = getHistoryMethodForAction(visitAction)
session.history.update(method, expandURL(this.element.src || ""), this.restorationIdentifier)
}
}
Expand All @@ -392,7 +316,7 @@ export class FrameController {
}

#willHandleFrameMissingFromResponse(fetchResponse) {
this.element.setAttribute("complete", "")
this.complete = true

const response = fetchResponse.response
const visit = async (url, options) => {
Expand Down Expand Up @@ -520,7 +444,7 @@ export class FrameController {
}

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

get complete() {
Expand Down Expand Up @@ -556,12 +480,6 @@ export class FrameController {
callback()
this.#ignoredAttributes.delete(attributeName)
}

#withCurrentNavigationElement(element, callback) {
this.currentNavigationElement = element
callback()
delete this.currentNavigationElement
}
}

function getFrameElementById(id) {
Expand Down
Loading

0 comments on commit 845459d

Please sign in to comment.