Skip to content

Commit

Permalink
Resolve Frame Visit caching issue (#644)
Browse files Browse the repository at this point in the history
Closes #643

First, rename the `FrameRendererDelegate.frameExtracted` callback method
to `willRenderFrame`, then pass along both the _current_ frame and the
_new_ frame as arguments.

Next, in the `FrameController.willRenderFrame` callback, assign the
`previousFrameElement` property (used elsewhere in the
`visitCachedSnapshot` callback) to contain the contents of the _current_
frame, instead of the _new_ frame.
  • Loading branch information
seanpdoyle authored Jul 28, 2022
1 parent 92761ec commit 1563ebb
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,8 @@ export class FrameController
viewInvalidated() {}

// Frame renderer delegate
frameExtracted(element: FrameElement) {
this.previousFrameElement = element
willRenderFrame(currentElement: FrameElement, _newElement: FrameElement) {
this.previousFrameElement = currentElement.cloneNode(true)
}

visitCachedSnapshot = ({ element }: Snapshot) => {
Expand Down
4 changes: 2 additions & 2 deletions src/core/frames/frame_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Render, Renderer } from "../renderer"
import { Snapshot } from "../snapshot"

export interface FrameRendererDelegate {
frameExtracted(element: FrameElement): void
willRenderFrame(currentElement: FrameElement, newElement: FrameElement): void
}

export class FrameRenderer extends Renderer<FrameElement> {
Expand Down Expand Up @@ -52,7 +52,7 @@ export class FrameRenderer extends Renderer<FrameElement> {
}

loadFrameElement() {
this.delegate.frameExtracted(this.newElement.cloneNode(true))
this.delegate.willRenderFrame(this.currentElement, this.newElement)
this.renderElement(this.currentElement, this.newElement)
}

Expand Down
33 changes: 28 additions & 5 deletions src/tests/functional/frame_navigation_tests.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
import { test } from "@playwright/test"
import { getFromLocalStorage, nextEventNamed, nextEventOnTarget, pathname } from "../helpers/page"
import { getFromLocalStorage, nextBeat, nextEventNamed, nextEventOnTarget, pathname } from "../helpers/page"
import { assert } from "chai"

test.beforeEach(async ({ page }) => {
await page.goto("/src/tests/fixtures/frame_navigation.html")
})

test("test frame navigation with descendant link", async ({ page }) => {
await page.goto("/src/tests/fixtures/frame_navigation.html")
await page.click("#inside")

await nextEventOnTarget(page, "frame", "turbo:frame-load")
})

test("test frame navigation with self link", async ({ page }) => {
await page.goto("/src/tests/fixtures/frame_navigation.html")
await page.click("#self")

await nextEventOnTarget(page, "frame", "turbo:frame-load")
})

test("test frame navigation with exterior link", async ({ page }) => {
await page.goto("/src/tests/fixtures/frame_navigation.html")
await page.click("#outside")

await nextEventOnTarget(page, "frame", "turbo:frame-load")
Expand All @@ -45,3 +44,27 @@ test("test promoted frame navigation updates the URL before rendering", async ({
assert.equal(await pathname(page.url()), "/src/tests/fixtures/tabs/two.html")
assert.equal(await page.textContent("#tab-content"), "Two")
})

test("test promoted frame navigations are cached", async ({ page }) => {
await page.goto("/src/tests/fixtures/tabs.html")

await page.click("#tab-2")
await nextEventNamed(page, "turbo:frame-render")

assert.equal(await page.textContent("#tab-content"), "Two")

await page.click("#tab-3")
await nextEventNamed(page, "turbo:frame-render")

assert.equal(await page.textContent("#tab-content"), "Three")

await page.goBack()
await nextBeat()

assert.equal(await page.textContent("#tab-content"), "Two")

await page.goBack()
await nextBeat()

assert.equal(await page.textContent("#tab-content"), "One")
})

0 comments on commit 1563ebb

Please sign in to comment.