Frame Visits: Cache Snapshot later in process

Follow-up to [#441][]
Depends on [#487][]
Closes #472

---

When caching Snapshots during a `Visit`, elements are not cached until
after the `turbo:before-cache` event fires. This affords client
applications with an opportunity to disconnect and deconstruct and
JavaScript state, and provides an opportunity to encode that state into
the HTML so that it can survive the caching process.

The timing of the construction of the `SnapshotSubstitution` instance
occurs too early in the frame rendering process: the `<turbo-frame>`
descendants have not been disconnected. The handling of the
`<turbo-frame>` caching is already an exception from the norm.

Unfortunately, the current implementation is caching _too early_ in the
process. If the Snapshot were cached too late in the process with the
rest of the page (as described in [#441][]), the `[src]`
attribute and descendant content would have already changed, so any
previous state would be lost.

This commit strikes a balance between the two extremes by introducing
the `FrameRendererDelegate` interface and the `frameContentsExtracted()`
hook. During `<turbo-frame>` rendering, the `FrameRenderer` instance
selects a [Range][] of nodes and removes them by calling
[Range.deleteContents][]. The `deleteContents()` method removes the
Nodes and discards them.

This commit replaces the `deleteContents()` call with one to
[Range.extractContents][], so that the Nodes are retained as a
[DocumentFragment][] instance. While handling the callback, the
`FrameController` retains that instance by setting an internal
`previousContents` property.

Later on in the Frame rendering-to-Visit-promotion process, the
`FrameController` implements the `visitCachedSnapshot()` hook to read
from the `previousContents` property and substitute the frame's contents
with the `previousContents`, replacing the need for the
`SnapshotSubstitution` class.

[#441]: #441
[#487]: #487
[Range]: https://developer.mozilla.org/en-US/docs/Web/API/Range
[Range.deleteContents]: https://developer.mozilla.org/en-US/docs/Web/API/range/deleteContents
[Range.extractContents]: https://developer.mozilla.org/en-US/docs/Web/API/Range/extractContents
[DocumentFragment]: https://developer.mozilla.org/en-US/docs/Web/API/DocumentFragment

src/core/frames/frame_controller.ts

@@ -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
@@ -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
@@ -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) {
@@ -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
@@ -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)

src/core/frames/frame_renderer.ts

@@ -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
   }
@@ -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()

src/elements/frame_element.ts

@@ -1,4 +1,5 @@
 import { FetchResponse } from "../http/fetch_response"
+import { Snapshot } from "../core/snapshot"
 
 export enum FrameLoadingStyle {
   eager = "eager",
@@ -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
 }
 

src/tests/functional/frame_tests.ts

@@ -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)
 })
 

src/tests/functional/loading_tests.ts

@@ -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")) || "")