From 6aba5c6e9913ec067c86142d4f9230e2aa1e0abc Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 11 Nov 2021 14:03:32 -0500 Subject: [PATCH] Introduce `turbo:frame-missing` event Closes [hotwired/turbo#432][] Follow-up to [hotwired/turbo#94][] Follow-up to [hotwired/turbo#31][] When a response from _within_ a frame is missing a matching frame, fire the `turbo:frame-missing` event. There is an existing [contract][] that dictates a request from within a frame stays within a frame. However, if an application is interested in reacting to a response without a frame, dispatch a `turbo:frame-missing` event. The event's `target` is the `FrameElement`, and the `detail` contains the `fetchResponse:` key. The `event.detail.visit` key provides handlers with a way to transform the `fetchResponse` into a `Turbo.visit()` call without any knowledge of the internal structure or logic necessary to do so. Event listeners for `turbo:frame-missing` can invoke the `event.detail.visit` directly to invoke `Turbo.visit()` behind the scenes. The yielded `visit()` function accepts `Partial` as an optional argument: ```js addEventListener("turbo:frame-missing", async ({ target, detail: { fetchResponse, visit } }) => { // the details of `shouldRedirectOnMissingFrame(element: FrameElement)` // are up to the application to decide if (shouldRedirectOnMissingFrame(target)) { visit({ action: "replace" }) } }) ``` [contract]: https://github.com/hotwired/turbo/issues/94#issuecomment-756968205 [hotwired/turbo#432]: https://github.com/hotwired/turbo/issues/432 [hotwired/turbo#94]: https://github.com/hotwired/turbo/issues/94 [hotwired/turbo#31]: https://github.com/hotwired/turbo/issues/31 --- src/core/frames/frame_controller.ts | 14 ++++++------ src/core/session.ts | 11 ++++++++++ src/elements/frame_element.ts | 1 + src/tests/fixtures/frames.html | 7 ++++++ src/tests/fixtures/test.js | 1 + src/tests/functional/frame_tests.ts | 33 ++++++++++++++++++++++++++++- 6 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 53a585331..031cb30ec 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -111,8 +111,12 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest const renderer = new FrameRenderer(this.view.snapshot, snapshot, false) if (this.view.renderPromise) await this.view.renderPromise await this.view.render(renderer) - session.frameRendered(fetchResponse, this.element) - session.frameLoaded(this.element) + if (snapshot.element.delegate.isActive) { + session.frameRendered(fetchResponse, this.element) + session.frameLoaded(this.element) + } else { + session.frameMissing(fetchResponse, this.element) + } } } catch (error) { console.error(error) @@ -295,10 +299,8 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest await element.loaded return await this.extractForeignFrameElement(element) } - - console.error(`Response has no matching element`) - } catch (error) { - console.error(error) + } catch { + return new FrameElement() } return new FrameElement() diff --git a/src/core/session.ts b/src/core/session.ts index 70bd16133..f968aa386 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -267,6 +267,17 @@ export class Session implements FormSubmitObserverDelegate, HistoryDelegate, Lin this.notifyApplicationAfterFrameRender(fetchResponse, frame); } + frameMissing(fetchResponse: FetchResponse, target: FrameElement) { + const visit = async (options: Partial = {}) => { + const responseHTML = await fetchResponse.responseHTML + const { location, redirected, statusCode } = fetchResponse + + this.visit(location, { response: { redirected, statusCode, responseHTML }, ...options }) + } + + dispatch("turbo:frame-missing", { target, detail: { fetchResponse, visit } }) + } + // Application events applicationAllowsFollowingLinkToLocation(link: Element, location: URL) { diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index bfde5d66c..d2e52fab9 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -12,6 +12,7 @@ export interface FrameElementDelegate { linkClickIntercepted(element: Element, url: string): void loadResponse(response: FetchResponse): void isLoading: boolean + isActive: boolean } /** diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html index acfc17257..a061e4c33 100644 --- a/src/tests/fixtures/frames.html +++ b/src/tests/fixtures/frames.html @@ -9,6 +9,8 @@ addEventListener("click", ({ target }) => { if (target.id == "add-turbo-action-to-frame") { target.closest("turbo-frame")?.setAttribute("data-turbo-action", "advance") + } else if (target.id == "propose-visit-when-frame-missing") { + addEventListener("turbo:frame-missing", ({ detail: { visit } }) => visit(), { once: true }) } }) @@ -78,6 +80,9 @@

Frames: #nested-child

Missing frame +
+ +
@@ -104,5 +109,7 @@

Frames: #nested-child

+ + diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js index 0cc3ea174..880f7be12 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -29,4 +29,5 @@ "turbo:visit", "turbo:frame-load", "turbo:frame-render", + "turbo:frame-missing", ]) diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index 979b3074e..b3e13a3e8 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -19,6 +19,8 @@ export class FrameTests extends TurboDriveTestCase { async "test a frame whose src references itself does not infinitely loop"() { await this.clickSelector("#frame-self") + await this.nextEventOnTarget("frame", "turbo:before-fetch-request") + await this.nextEventOnTarget("frame", "turbo:before-fetch-response") await this.nextEventOnTarget("frame", "turbo:frame-render") await this.nextEventOnTarget("frame", "turbo:frame-load") @@ -37,8 +39,11 @@ export class FrameTests extends TurboDriveTestCase { async "test following a link to a page without a matching frame results in an empty frame"() { await this.clickSelector("#missing a") - await this.nextBeat + + const { fetchResponse } = await this.nextEventOnTarget("missing", "turbo:frame-missing") + this.assert.notOk(await this.innerHTMLForSelector("#missing")) + this.assert.ok(fetchResponse) } async "test following a link within a frame with a target set navigates the target frame"() { @@ -407,6 +412,28 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") } + async "test navigating frame resulting in response without matching frame can be re-purposed to navigate entire page"() { + await this.proposeVisitWhenFrameIsMissingInResponse() + await this.clickSelector("#missing a") + await this.nextEventOnTarget("missing", "turbo:frame-missing") + await this.nextBody + + this.assert.notOk(await this.hasSelector("#missing")) + this.assert.equal(await (await this.querySelector("h1")).getVisibleText(), "Frames: #frame") + this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") + } + + async "test submitting frame resulting in response without matching frame can be re-purposed to navigate entire page"() { + await this.proposeVisitWhenFrameIsMissingInResponse() + await this.clickSelector("#missing button") + await this.nextEventOnTarget("missing", "turbo:frame-missing") + await this.nextBody + + this.assert.notOk(await this.hasSelector("#missing")) + this.assert.equal(await (await this.querySelector("h1")).getVisibleText(), "Frames: #frame") + this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html") + } + async "test turbo:before-fetch-request fires on the frame element"() { await this.clickSelector("#hello a") this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-request")) @@ -420,6 +447,10 @@ export class FrameTests extends TurboDriveTestCase { get frameScriptEvaluationCount(): Promise { return this.evaluate(() => window.frameScriptEvaluationCount) } + + async proposeVisitWhenFrameIsMissingInResponse(): Promise { + return await this.clickSelector("#propose-visit-when-frame-missing") + } } declare global {