Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

turbo:frame-missing: Do not Turbo.visit by default #677

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
turbo:frame-missing: Do not Turbo.visit by default
Restore the existing default behavior when a matching frame is missing
from a response: log an error and blank the frame.

Alongside that original 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.

[Response]: https://developer.mozilla.org/en-US/docs/Web/API/Response
  • Loading branch information
seanpdoyle committed Aug 19, 2022
commit baef5af61d493d290bc92a2c76233dbd5e37eaa9
27 changes: 21 additions & 6 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
@@ -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<VisitOptions>) => Promise<void>
export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: VisitFallback }>

export class FrameController
implements
@@ -169,8 +170,9 @@ 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.error(`Response has no matching <turbo-frame id="${this.element.id}"> element`)
this.element.innerHTML = ""
}
}
} catch (error) {
@@ -398,12 +400,25 @@ 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) {
const wrapped = new FetchResponse(url)
const responseHTML = await wrapped.responseHTML
const { location, redirected, statusCode } = wrapped

session.visit(location, { response: { redirected, statusCode, responseHTML }, ...options })
} else {
session.visit(url, options)
}
}

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

9 changes: 0 additions & 9 deletions src/core/session.ts
Original file line number Diff line number Diff line change
@@ -313,15 +313,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) {
51 changes: 37 additions & 14 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
@@ -119,30 +119,22 @@ test("test following a link to a page without a matching frame dispatches a turb
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 noNextEventNamed(page, "turbo:before-fetch-request")
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")
const { response } = await nextEventOnTarget(page, "missing", "turbo:frame-missing")

assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
assert.ok(await innerHTMLForSelector(page, "#missing"))
assert.ok(response, "dispatches turbo:frame-missing with event.detail.response")
assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html", "stays on the page")
assert.equal(await page.locator("#missing").innerHTML(), "", "blanks the contents when not canceled")
})

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 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.preventDefault()
event.target.textContent = "Overridden"
}
},
@@ -155,6 +147,37 @@ test("test following a link to a page without a matching frame dispatches a turb
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) => {
if (event instanceof CustomEvent) {
event.preventDefault()
const { response, visit } = event.detail

visit(response)
}
},
{ once: true }
)
})
await page.click("#missing a")
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,
}) => {