From 3a204970bdb352a79058e628be8c7b62f21d8e32 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 11 Nov 2021 14:03:32 -0500 Subject: [PATCH 1/4] 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. Unless it's canceled (by calling `event.preventDefault()`), Turbo Drive will visit the frame's URL as a full-page navigation. The event listener is also a good opportunity to change the `` element itself to prevent future missing responses. For example, if the reason the frame is missing is access (an expired session, for example), the call to `visit()` can be made with `{ action: "replace" }` to remove the current page from Turbo's page history. [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 | 61 +++++++++++++++++++---------- src/core/index.ts | 1 + src/core/session.ts | 7 ++++ src/tests/fixtures/test.js | 1 + src/tests/functional/frame_tests.ts | 55 ++++++++++++++++++++++++-- 5 files changed, 101 insertions(+), 24 deletions(-) diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 85254930d..19b216066 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -31,6 +31,8 @@ import { isAction, Action } from "../types" import { VisitOptions } from "../drive/visit" import { TurboBeforeFrameRenderEvent } from "../session" +export type TurboFrameMissingEvent = CustomEvent<{ fetchResponse: FetchResponse }> + export class FrameController implements AppearanceObserverDelegate, @@ -145,23 +147,29 @@ export class FrameController const html = await fetchResponse.responseHTML if (html) { const { body } = parseHTMLDocument(html) - const snapshot = new Snapshot(await this.extractForeignFrameElement(body)) - const renderer = new FrameRenderer( - this, - this.view.snapshot, - snapshot, - FrameRenderer.renderElement, - false, - false - ) - if (this.view.renderPromise) await this.view.renderPromise - this.changeHistory() - - await this.view.render(renderer) - this.complete = true - session.frameRendered(fetchResponse, this.element) - session.frameLoaded(this.element) - this.fetchResponseLoaded(fetchResponse) + const newFrameElement = await this.extractForeignFrameElement(body) + + 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() + + await this.view.render(renderer) + this.complete = true + session.frameRendered(fetchResponse, this.element) + session.frameLoaded(this.element) + this.fetchResponseLoaded(fetchResponse) + } else if (this.sessionWillHandleMissingFrame(fetchResponse)) { + await session.frameMissing(this.element, fetchResponse) + } } } catch (error) { console.error(error) @@ -378,12 +386,24 @@ export class FrameController } } + private sessionWillHandleMissingFrame(fetchResponse: FetchResponse) { + this.element.setAttribute("complete", "") + + const event = dispatch("turbo:frame-missing", { + target: this.element, + detail: { fetchResponse }, + cancelable: true, + }) + + return !event.defaultPrevented + } + private findFrameElement(element: Element, submitter?: HTMLElement) { const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target") return getFrameElementById(id) ?? this.element } - async extractForeignFrameElement(container: ParentNode): Promise { + async extractForeignFrameElement(container: ParentNode): Promise { let element const id = CSS.escape(this.id) @@ -398,13 +418,12 @@ export class FrameController await element.loaded return await this.extractForeignFrameElement(element) } - - console.error(`Response has no matching element`) } catch (error) { console.error(error) + return new FrameElement() } - return new FrameElement() + return null } private formActionIsVisitable(form: HTMLFormElement, submitter?: HTMLElement) { diff --git a/src/core/index.ts b/src/core/index.ts index 721d2989c..ed6f68484 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -27,6 +27,7 @@ export { } from "./session" export { TurboSubmitStartEvent, TurboSubmitEndEvent } from "./drive/form_submission" +export { TurboFrameMissingEvent } from "./frames/frame_controller" export { TurboBeforeFetchRequestEvent, TurboBeforeFetchResponseEvent } from "../http/fetch_request" export { TurboBeforeStreamRenderEvent } from "../elements/stream_element" diff --git a/src/core/session.ts b/src/core/session.ts index 23fcc1eaa..2411bd3d1 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -305,6 +305,13 @@ export class Session this.notifyApplicationAfterFrameRender(fetchResponse, frame) } + async frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise { + 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/test.js b/src/tests/fixtures/test.js index acb543061..510e8c2ea 100644 --- a/src/tests/fixtures/test.js +++ b/src/tests/fixtures/test.js @@ -50,5 +50,6 @@ "turbo:before-frame-render", "turbo:frame-load", "turbo:frame-render", + "turbo:frame-missing", "turbo:reload" ]) diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index e5e160433..565b892b6 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -113,10 +113,59 @@ 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 results in an empty frame", async ({ page }) => { +test("test following a link to a page without a matching frame dispatches a turbo:frame-missing event", async ({ + page, +}) => { await page.click("#missing a") - await nextBeat() - assert.notOk(await innerHTMLForSelector(page, "#missing")) + await noNextEventOnTarget(page, "missing", "turbo:frame-render") + await noNextEventOnTarget(page, "missing", "turbo:frame-load") + const { fetchResponse } = await nextEventOnTarget(page, "missing", "turbo:frame-missing") + await nextEventNamed(page, "turbo:load") + + assert.ok(fetchResponse, "dispatchs turbo:frame-missing with event.detail.fetchResponse") + assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.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 following a link to a page without a matching frame dispatches a turbo:frame-missing event that can be cancelled", async ({ + page, +}) => { + await page.locator("#missing").evaluate((frame) => { + frame.addEventListener( + "turbo:frame-missing", + (event) => { + event.preventDefault() + + if (event.target instanceof HTMLElement) { + event.target.textContent = "Overridden" + } + }, + { once: true } + ) + }) + await page.click("#missing a") + await nextEventOnTarget(page, "missing", "turbo:frame-missing") + + assert.equal(await page.textContent("#missing"), "Overridden") +}) + +test("test following a link to a page with a matching frame does not dispatch a turbo:frame-missing event", async ({ + page, +}) => { + await page.click("#link-frame") + await noNextEventNamed(page, "turbo:frame-missing") + await nextEventOnTarget(page, "frame", "turbo:frame-load") + + const src = await attributeForSelector(page, "#frame", "src") + assert( + src?.includes("/src/tests/fixtures/frames/frame.html"), + "navigates frame without dispatching turbo:frame-missing" + ) }) test("test following a link within a frame with a target set navigates the target frame", async ({ page }) => { From 22c3ff3e7595627015ac5135177f7f2868c1bea4 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Mon, 1 Aug 2022 13:00:38 -0400 Subject: [PATCH 2/4] re-run CI From 1043f0def44b182e7e34bb8d631ea38fc7c3277a Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Mon, 1 Aug 2022 13:55:21 -0400 Subject: [PATCH 3/4] issue a new request for the full page of content --- src/core/session.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/session.ts b/src/core/session.ts index 2411bd3d1..23f6e9d08 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -305,11 +305,8 @@ export class Session this.notifyApplicationAfterFrameRender(fetchResponse, frame) } - async frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise { - const responseHTML = await fetchResponse.responseHTML - const { location, redirected, statusCode } = fetchResponse - - return this.visit(location, { response: { redirected, statusCode, responseHTML } }) + frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise { + return this.visit(fetchResponse.location) } // Application events From 8c24548df918f0286d0280dcdd32671ea2b2c025 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Wed, 3 Aug 2022 17:26:59 -0400 Subject: [PATCH 4/4] Add console warning if a full-page visit is triggered as a result of missing matching frame --- src/core/session.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/session.ts b/src/core/session.ts index 23f6e9d08..7322b2bbd 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -306,6 +306,7 @@ export class Session } frameMissing(frame: FrameElement, fetchResponse: FetchResponse): Promise { + console.warn(`Completing full-page visit as matching frame for #${frame.id} was missing from the response`) return this.visit(fetchResponse.location) }