From 6e128894606d47be53b43e09664df9cc85ea663a Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 10 Aug 2022 09:18:08 -0400 Subject: [PATCH] `turbo:frame-missing`: Dispatch for 4xx and 5xx Closes https://github.com/hotwired/turbo/issues/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 --- src/core/frames/frame_controller.ts | 36 ++++++++++++--- src/core/session.ts | 9 ---- src/tests/fixtures/frames.html | 3 +- src/tests/functional/frame_tests.ts | 72 +++++++++++++++++++++++------ 4 files changed, 90 insertions(+), 30 deletions(-) diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index e276a8fe2..ecdb730b7 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -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" @@ -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) => Promise +export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: VisitFallback }> export class FrameController implements @@ -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) } } } catch (error) { @@ -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() } @@ -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 = {}) => { + if (url instanceof Response) { + this.visitResponse(url) + } else { + session.visit(url, options) + } + } + const event = dispatch("turbo:frame-missing", { target: this.element, - detail: { fetchResponse }, + detail: { response, visit }, cancelable: true, }) return !event.defaultPrevented } + private async visitResponse(response: Response): Promise { + 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 diff --git a/src/core/session.ts b/src/core/session.ts index 6df0c9b62..24261c901 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -313,15 +313,6 @@ export class Session this.notifyApplicationAfterFrameRender(fetchResponse, frame) } - async frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise { - 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) { diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html index b611136e5..73cc928e0 100644 --- a/src/tests/fixtures/frames.html +++ b/src/tests/fixtures/frames.html @@ -90,7 +90,8 @@

Frames: #nested-child

- Missing frame + Missing frame + 404 diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index 04db9f003..9225abf9f 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -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() @@ -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, }) => {