Skip to content

Commit

Permalink
turbo:frame-missing: Dispatch for 4xx and 5xx (#693)
Browse files Browse the repository at this point in the history
Closes #670

Re-use the existing `2xx` and `3xx` behavior for a response to handle
error responses. When the frame is missing from the page (likely for
error pages like `404`), dispatch a `turbo:frame-missing` event.

Alongside that behavior, yield the [Response][] instance and a
`Turbo.visit`-like callback that can transform the `Response` instance
into a `Visit`. It also accepts all the arguments that the `Turbo.visit`
call normally does.

When the event is not canceled, treat the `Response` as a Visit. When
the event **is** canceled, let the listener handle it.

[Response]: https://developer.mozilla.org/en-US/docs/Web/API/Response
  • Loading branch information
seanpdoyle authored Sep 13, 2022
1 parent 8871817 commit ebf4471
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 30 deletions.
36 changes: 29 additions & 7 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
import { Snapshot } from "../snapshot"
import { ViewDelegate, ViewRenderOptions } from "../view"
import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
import { Locatable, getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/form_submit_observer"
import { FrameView } from "./frame_view"
import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer"
Expand All @@ -32,7 +32,8 @@ import { VisitOptions } from "../drive/visit"
import { TurboBeforeFrameRenderEvent, TurboFetchRequestErrorEvent } from "../session"
import { StreamMessage } from "../streams/stream_message"

export type TurboFrameMissingEvent = CustomEvent<{ fetchResponse: FetchResponse }>
type VisitFallback = (location: Response | Locatable, options: Partial<VisitOptions>) => Promise<void>
export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: VisitFallback }>

export class FrameController
implements
Expand Down Expand Up @@ -169,8 +170,11 @@ export class FrameController
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
this.fetchResponseLoaded(fetchResponse)
} else if (this.sessionWillHandleMissingFrame(fetchResponse)) {
await session.frameMissing(this.element, fetchResponse)
} else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
console.warn(
`A matching frame for #${this.element.id} was missing from the response, transforming into full-page Visit.`
)
this.visitResponse(fetchResponse.response)

This comment has been minimized.

Copy link
@fschwahn

fschwahn Oct 18, 2022

@seanpdoyle we disabled turbo-drive using

Turbo.session.drive = false;

but after updating Turbo to 7.2.2 clicks inside turbo-frames which don't target the frame result in turbo visits instead of full page reloads. Should the Turbo.session.drive also be respected in this case?

This comment has been minimized.

Copy link
@seanpdoyle

seanpdoyle Oct 18, 2022

Author Contributor

@fschwahn According to #648 and 567edf0, that behavior was deliberate.

I wasn't the author of that change. If you're interested in changing that behavior, would you be interested in opening a separate issue to discuss it further?

This comment has been minimized.

Copy link
@fschwahn

fschwahn Oct 18, 2022

Got it, thanks for pointing me in the right direction.

}
}
} catch (error) {
Expand Down Expand Up @@ -248,8 +252,9 @@ export class FrameController
this.resolveVisitPromise()
}

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

Expand Down Expand Up @@ -398,18 +403,35 @@ export class FrameController
}
}

private sessionWillHandleMissingFrame(fetchResponse: FetchResponse) {
private willHandleFrameMissingFromResponse(fetchResponse: FetchResponse): boolean {
this.element.setAttribute("complete", "")

const response = fetchResponse.response
const visit = async (url: Locatable | Response, options: Partial<VisitOptions> = {}) => {
if (url instanceof Response) {
this.visitResponse(url)
} else {
session.visit(url, options)
}
}

const event = dispatch<TurboFrameMissingEvent>("turbo:frame-missing", {
target: this.element,
detail: { fetchResponse },
detail: { response, visit },
cancelable: true,
})

return !event.defaultPrevented
}

private async visitResponse(response: Response): Promise<void> {
const wrapped = new FetchResponse(response)
const responseHTML = await wrapped.responseHTML
const { location, redirected, statusCode } = wrapped

return session.visit(location, { response: { redirected, statusCode, responseHTML } })
}

private findFrameElement(element: Element, submitter?: HTMLElement) {
const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target")
return getFrameElementById(id) ?? this.element
Expand Down
9 changes: 0 additions & 9 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,15 +316,6 @@ export class Session
this.notifyApplicationAfterFrameRender(fetchResponse, frame)
}

async frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise<void> {
console.warn(`A matching frame for #${frame.id} was missing from the response, transforming into full-page Visit.`)

const responseHTML = await fetchResponse.responseHTML
const { location, redirected, statusCode } = fetchResponse

return this.visit(location, { response: { redirected, statusCode, responseHTML } })
}

// Application events

applicationAllowsFollowingLinkToLocation(link: Element, location: URL, ev: MouseEvent) {
Expand Down
3 changes: 2 additions & 1 deletion src/tests/fixtures/frames.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ <h2>Frames: #nested-child</h2>
</turbo-frame>

<turbo-frame id="missing">
<a href="/src/tests/fixtures/frames/frame.html">Missing frame</a>
<a id="missing-frame-link" href="/src/tests/fixtures/frames/frame.html">Missing frame</a>
<a id="missing-page-link" href="/missing.html">404</a>
</turbo-frame>

<turbo-frame id="body-script" target="body-script">
Expand Down
72 changes: 59 additions & 13 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,16 @@ test("test following a link driving a frame toggles the [aria-busy=true] attribu
)
})

test("test following a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
test("test successfully following a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
page,
}) => {
await page.click("#missing a")
await noNextEventOnTarget(page, "missing", "turbo:frame-render")
await noNextEventOnTarget(page, "missing", "turbo:frame-load")
const { fetchResponse } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")
await page.click("#missing-frame-link")
await nextEventOnTarget(page, "missing", "turbo:before-fetch-request")
const { response } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")
await noNextEventNamed(page, "turbo:before-fetch-request")
await nextEventNamed(page, "turbo:load")

assert.ok(fetchResponse, "dispatchs turbo:frame-missing with event.detail.fetchResponse")
assert.ok(response, "dispatches turbo:frame-missing with event.detail.response")
assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html", "navigates the page")

await page.goBack()
Expand All @@ -133,28 +132,75 @@ test("test following a link to a page without a matching frame dispatches a turb
assert.ok(await innerHTMLForSelector(page, "#missing"))
})

test("test following a link to a page without a matching frame dispatches a turbo:frame-missing event that can be cancelled", async ({
test("test failing to follow a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({
page,
}) => {
await page.click("#missing-page-link")
await nextEventOnTarget(page, "missing", "turbo:before-fetch-request")
const { response } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")
await noNextEventNamed(page, "turbo:before-fetch-request")
await noNextEventNamed(page, "turbo:load")

assert.ok(response, "dispatches turbo:frame-missing with event.detail.response")
assert.equal(pathname(page.url()), "/missing.html", "navigates the page")

await page.goBack()
await nextEventNamed(page, "turbo:load")

assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
assert.ok(await innerHTMLForSelector(page, "#missing"))
})

test("test the turbo:frame-missing event following a link to a page without a matching frame can be handled", async ({
page,
}) => {
await page.locator("#missing").evaluate((frame) => {
frame.addEventListener(
"turbo:frame-missing",
(event) => {
event.preventDefault()

if (event.target instanceof HTMLElement) {
((event) => {
if (event.target instanceof Element) {
event.preventDefault()
event.target.textContent = "Overridden"
}
},
}) as EventListener,
{ once: true }
)
})
await page.click("#missing a")
await page.click("#missing-frame-link")
await nextEventOnTarget(page, "missing", "turbo:frame-missing")

assert.equal(await page.textContent("#missing"), "Overridden")
})

test("test the turbo:frame-missing event following a link to a page without a matching frame can drive a Visit", async ({
page,
}) => {
await page.locator("#missing").evaluate((frame) => {
frame.addEventListener(
"turbo:frame-missing",
((event: CustomEvent) => {
event.preventDefault()
const { response, visit } = event.detail

visit(response)
}) as EventListener,
{ once: true }
)
})
await page.click("#missing-frame-link")
await nextEventOnTarget(page, "missing", "turbo:frame-missing")
await nextEventNamed(page, "turbo:load")

assert.equal(await page.textContent("h1"), "Frames: #frame")
assert.notOk(await hasSelector(page, "turbo-frame#missing"))

await page.goBack()
await nextEventNamed(page, "turbo:load")

assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
assert.equal(await innerHTMLForSelector(page, "#missing"), "")
})

test("test following a link to a page with a matching frame does not dispatch a turbo:frame-missing event", async ({
page,
}) => {
Expand Down

0 comments on commit ebf4471

Please sign in to comment.