Skip to content

Commit

Permalink
Dispatch turbo:frame-render when Frame connects
Browse files Browse the repository at this point in the history
Closes hotwired/turbo-rails#379

Currently, a `<turbo-frame>` element only dispatches events when its
contents are fetched over HTTP. The `turbo:frame-render` and
`turbo:frame-load` events are dispatched sequentially and immediately
following one another. The `turbo:frame-render` detail exposes a
reference to the `FetchResponse`, while the `turbo:frame-load` event has
no details.

Consumer applications might expect a `<turbo-frame>` to dispatch a
`turbo:frame-render` event immediately whenever a `<turbo-frame>` is
connected to the document (similar to the way that `turbo:render` is
dispatched prior to `turbo:load`, even on restoration Visits).

For the sake of consistency and distinguishing between two events that
are more or less equivalent and temporally interchangeable, this commit
proposes that a `<turbo-frame>` should dispatch a `turbo:frame-render`
_whenever its contents change_, including its initial render.

As a complementary change to that behavior, the `turbo:frame-load`
events will be dispatched with `detail.fetchResponse` as a reference to
the `FetchResponse`.

Ideally, the `turbo:frame-render` would not expose the
`detail.fetchResponse` reference. However, removing it would be a
breaking change. To that end, this commit dispatches a `detail` that
prints a console warning (encouraging a migration to `turbo:frame-load`)
whenever a `turbo:frame-render` listener reads the
`detail.fetchResponse` property. Future major releases can remove the
property and warning.
  • Loading branch information
seanpdoyle committed Dec 30, 2022
1 parent 2f0d46f commit 47fb78d
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 16 deletions.
6 changes: 4 additions & 2 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ export class FrameController
this.formLinkClickObserver.start()
this.linkInterceptor.start()
this.formSubmitObserver.start()

session.frameRendered(this.element)
}
}

Expand Down Expand Up @@ -177,8 +179,8 @@ export class FrameController

await this.view.render(renderer)
this.complete = true
session.frameRendered(fetchResponse, this.element)
session.frameLoaded(this.element)
session.frameRendered(this.element, fetchResponse)
session.frameLoaded(this.element, fetchResponse)
this.fetchResponseLoaded(fetchResponse)
} else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
console.warn(
Expand Down
33 changes: 23 additions & 10 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Visit, VisitOptions } from "./drive/visit"
import { PageSnapshot } from "./drive/page_snapshot"
import { FrameElement } from "../elements/frame_element"
import { FrameViewRenderOptions } from "./frames/frame_view"
import { FetchRequest } from "../http/fetch_request"
import { FetchResponse } from "../http/fetch_response"
import { Preloader, PreloaderDelegate } from "./drive/preloader"

Expand All @@ -29,9 +30,10 @@ export type TurboBeforeCacheEvent = CustomEvent
export type TurboBeforeRenderEvent = CustomEvent<{ newBody: HTMLBodyElement } & PageViewRenderOptions>
export type TurboBeforeVisitEvent = CustomEvent<{ url: string }>
export type TurboClickEvent = CustomEvent<{ url: string; originalEvent: MouseEvent }>
export type TurboFrameLoadEvent = CustomEvent
export type TurboFrameLoadEvent = CustomEvent<{ fetchResponse: FetchResponse }>
export type TurboBeforeFrameRenderEvent = CustomEvent<{ newFrame: FrameElement } & FrameViewRenderOptions>
export type TurboFrameRenderEvent = CustomEvent<{ fetchResponse: FetchResponse }>
export type TurboFetchRequestErrorEvent = CustomEvent<{ request: FetchRequest; error: Error }>
export type TurboFrameRenderEvent = CustomEvent
export type TurboLoadEvent = CustomEvent<{ url: string; timing: TimingData }>
export type TurboRenderEvent = CustomEvent
export type TurboVisitEvent = CustomEvent<{ url: string; action: Action }>
Expand Down Expand Up @@ -305,12 +307,12 @@ export class Session

// Frame element

frameLoaded(frame: FrameElement) {
this.notifyApplicationAfterFrameLoad(frame)
frameLoaded(frame: FrameElement, fetchResponse: FetchResponse) {
this.notifyApplicationAfterFrameLoad(frame, fetchResponse)
}

frameRendered(fetchResponse: FetchResponse, frame: FrameElement) {
this.notifyApplicationAfterFrameRender(fetchResponse, frame)
frameRendered(frame: FrameElement, fetchResponse?: FetchResponse) {
this.notifyApplicationAfterFrameRender(frame, fetchResponse)
}

// Application events
Expand Down Expand Up @@ -374,13 +376,24 @@ export class Session
)
}

notifyApplicationAfterFrameLoad(frame: FrameElement) {
return dispatch<TurboFrameLoadEvent>("turbo:frame-load", { target: frame })
notifyApplicationAfterFrameLoad(frame: FrameElement, fetchResponse: FetchResponse) {
return dispatch<TurboFrameLoadEvent>("turbo:frame-load", {
detail: { fetchResponse },
target: frame,
})
}

notifyApplicationAfterFrameRender(fetchResponse: FetchResponse, frame: FrameElement) {
notifyApplicationAfterFrameRender(frame: FrameElement, fetchResponse?: FetchResponse) {
const detailWithDeprecation = {
get fetchResponse() {
console.warn(
"DEPRECATED: If your event listener needs to access the detail.fetchResponse, listen for the turbo:frame-load event"
)
return fetchResponse
},
}
return dispatch<TurboFrameRenderEvent>("turbo:frame-render", {
detail: { fetchResponse },
detail: detailWithDeprecation,
target: frame,
cancelable: true,
})
Expand Down
4 changes: 2 additions & 2 deletions src/tests/functional/drive_disabled_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ test("test drive disabled by default; submit form inside data-turbo='true'", asy

test("test drive disabled by default; links within <turbo-frame> navigate with Turbo", async ({ page }) => {
await page.click("#frame a")
await nextEventOnTarget(page, "frame", "turbo:frame-render")
await nextEventOnTarget(page, "frame", "turbo:frame-load")
})

test("test drive disabled by default; forms within <turbo-frame> navigate with Turbo", async ({ page }) => {
await page.click("#frame button")
await nextEventOnTarget(page, "frame", "turbo:frame-render")
await nextEventOnTarget(page, "frame", "turbo:frame-load")
})

test("test drive disabled by default; slot within <turbo-frame> navigate with Turbo", async ({ page }) => {
Expand Down
18 changes: 16 additions & 2 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -424,11 +424,25 @@ test("test redirecting in a form is still navigatable after redirect", async ({
assert.equal(await page.textContent("turbo-frame#form-redirect h2"), "Form Redirect")
})

test("test 'turbo:frame-render' is triggered when a frame is connected to the document", async ({ page }) => {
await page.evaluate(() => {
const frame = document.getElementById("frame")

if (frame) {
document.body.replaceChildren(frame)
}
})

const { fetchResponse } = await nextEventOnTarget(page, "frame", "turbo:frame-render")

assert.notOk(fetchResponse, "dispatches turbo:frame-render without fetchResponse")
})

test("test 'turbo:frame-render' is triggered after frame has finished rendering", async ({ page }) => {
await page.click("#frame-part")

await nextEventNamed(page, "turbo:frame-render") // recursive
const { fetchResponse } = await nextEventNamed(page, "turbo:frame-render")
await nextEventOnTarget(page, "part", "turbo:frame-render")
const { fetchResponse } = await nextEventOnTarget(page, "part", "turbo:frame-load")

assert.include(fetchResponse.response.url, "/src/tests/fixtures/frames/part.html")
})
Expand Down

0 comments on commit 47fb78d

Please sign in to comment.