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

Frame Visits: Cache Snapshot later in process #488

Merged
merged 1 commit into from
Jul 17, 2022
Merged
Show file tree
Hide file tree
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
38 changes: 20 additions & 18 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class FrameController
private connected = false
private hasBeenLoaded = false
private ignoredAttributes: Set<FrameElementObservedAttribute> = new Set()
private previousContents?: DocumentFragment

constructor(element: FrameElement) {
this.element = element
Expand Down Expand Up @@ -124,7 +125,7 @@ export class FrameController
if (html) {
const { body } = parseHTMLDocument(html)
const snapshot = new Snapshot(await this.extractForeignFrameElement(body))
const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false)
const renderer = new FrameRenderer(this, this.view.snapshot, snapshot, false, false)
if (this.view.renderPromise) await this.view.renderPromise
await this.view.render(renderer)
this.complete = true
Expand Down Expand Up @@ -250,6 +251,22 @@ export class FrameController

viewInvalidated() {}

// Frame renderer delegate
frameContentsExtracted(fragment: DocumentFragment) {
this.previousContents = fragment
}

visitCachedSnapshot = ({ element }: Snapshot) => {
const frame = element.querySelector("#" + this.element.id)

if (frame && this.previousContents) {
frame.innerHTML = ""
frame.append(this.previousContents)
}

delete this.previousContents
}

// Private

private async visit(url: URL) {
Expand Down Expand Up @@ -280,7 +297,8 @@ export class FrameController
const action = getAttribute("data-turbo-action", submitter, element, frame)

if (isAction(action)) {
const { visitCachedSnapshot } = new SnapshotSubstitution(frame)
const { visitCachedSnapshot } = frame.delegate

frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
if (frame.src) {
const { statusCode, redirected } = fetchResponse
Expand Down Expand Up @@ -427,22 +445,6 @@ export class FrameController
}
}

class SnapshotSubstitution {
private readonly clone: Node
private readonly id: string

constructor(element: FrameElement) {
this.clone = element.cloneNode(true)
this.id = element.id
}

visitCachedSnapshot = ({ element }: Snapshot) => {
const { id, clone } = this

element.querySelector("#" + id)?.replaceWith(clone)
}
}

function getFrameElementById(id: string | null) {
if (id != null) {
const element = document.getElementById(id)
Expand Down
20 changes: 19 additions & 1 deletion src/core/frames/frame_renderer.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
import { FrameElement } from "../../elements/frame_element"
import { nextAnimationFrame } from "../../util"
import { Renderer } from "../renderer"
import { Snapshot } from "../snapshot"

export interface FrameRendererDelegate {
frameContentsExtracted(fragment: DocumentFragment): void
}

export class FrameRenderer extends Renderer<FrameElement> {
private readonly delegate: FrameRendererDelegate

constructor(
delegate: FrameRendererDelegate,
currentSnapshot: Snapshot<FrameElement>,
newSnapshot: Snapshot<FrameElement>,
isPreview: boolean,
willRender = true
) {
super(currentSnapshot, newSnapshot, isPreview, willRender)
this.delegate = delegate
}

get shouldRender() {
return true
}
Expand All @@ -22,7 +40,7 @@ export class FrameRenderer extends Renderer<FrameElement> {
loadFrameElement() {
const destinationRange = document.createRange()
destinationRange.selectNodeContents(this.currentElement)
destinationRange.deleteContents()
this.delegate.frameContentsExtracted(destinationRange.extractContents())

const frameElement = this.newElement
const sourceRange = frameElement.ownerDocument?.createRange()
Expand Down
2 changes: 2 additions & 0 deletions src/elements/frame_element.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { FetchResponse } from "../http/fetch_response"
import { Snapshot } from "../core/snapshot"

export enum FrameLoadingStyle {
eager = "eager",
Expand All @@ -18,6 +19,7 @@ export interface FrameElementDelegate {
linkClickIntercepted(element: Element, url: string): void
loadResponse(response: FetchResponse): void
fetchResponseLoaded: (fetchResponse: FetchResponse) => void
visitCachedSnapshot: (snapshot: Snapshot) => void
isLoading: boolean
}

Expand Down
2 changes: 2 additions & 0 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,12 @@ test("test navigating back after pushing URL state from a turbo-frame[data-turbo

const title = await page.textContent("h1")
const frameTitle = await page.textContent("#frame h2")
const src = new URL((await attributeForSelector(page, "#frame", "src")) || "")

assert.equal(title, "Frames")
assert.equal(frameTitle, "Frames: #frame")
assert.equal(pathname(page.url()), "/src/tests/fixtures/frames.html")
assert.equal(src.pathname, "/src/tests/fixtures/frames/frame.html")
assert.equal(await propertyForSelector(page, "#frame", "src"), null)
})

Expand Down
2 changes: 1 addition & 1 deletion src/tests/functional/loading_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ test("test changing [src] attribute on a [complete] frame with loading=lazy defe
await page.click("#one")
await nextEventNamed(page, "turbo:load")
await page.goBack()
await nextBody(page)
await nextEventNamed(page, "turbo:load")
await noNextEventNamed(page, "turbo:frame-load")

let src = new URL((await attributeForSelector(page, "#hello", "src")) || "")
Expand Down