Integrate Frame to Page Visits with Snapshot Cache

[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, expands the `VisitDelegate` to include a caching
callback, then expands the `VisitOptions` type to include a
`Partial<VisitDelegate>`. Throughout the lifecycle, a `Visit` will
delegate to _both_ its instance property and any present `VisitOption`
delegate hooks.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430

src/core/drive/navigator.ts

@@ -133,6 +133,10 @@ export class Navigator {
     this.delegate.visitCompleted(visit)
   }
 
+  visitCachedSnapshot(visit: Visit) {
+    this.delegate.visitCachedSnapshot(visit)
+  }
+
   locationWithActionIsSamePage(location: URL, action?: Action): boolean {
     const anchor = getAnchor(location)
     const currentAnchor = getAnchor(this.view.lastRenderedLocation)

src/core/drive/visit.ts

@@ -15,6 +15,7 @@ export interface VisitDelegate {
 
   visitStarted(visit: Visit): void
   visitCompleted(visit: Visit): void
+  visitCachedSnapshot(visit: Visit): void
   locationWithActionIsSamePage(location: URL, action: Action): boolean
   visitScrolledToSamePageLocation(oldURL: URL, newURL: URL): void
 }
@@ -38,6 +39,7 @@ export enum VisitState {
 
 export type VisitOptions = {
   action: Action,
+  delegate: Partial<VisitDelegate>
   historyChanged: boolean,
   willRender: boolean
   referrer?: URL,
@@ -47,6 +49,7 @@ export type VisitOptions = {
 
 const defaultOptions: VisitOptions = {
   action: "advance",
+  delegate: {},
   historyChanged: false,
   willRender: true
 }
@@ -70,6 +73,7 @@ export class Visit implements FetchRequestDelegate {
   readonly action: Action
   readonly referrer?: URL
   readonly timingMetrics: TimingMetrics = {}
+  readonly optionalDelegate: Partial<VisitDelegate>
 
   willRender: boolean
   followedRedirect = false
@@ -90,14 +94,15 @@ export class Visit implements FetchRequestDelegate {
     this.location = location
     this.restorationIdentifier = restorationIdentifier || uuid()
 
-    const { action, historyChanged, referrer, snapshotHTML, response, willRender } = { ...defaultOptions, ...options }
+    const { action, historyChanged, referrer, snapshotHTML, response, willRender, delegate: optionalDelegate } = { ...defaultOptions, ...options }
     this.willRender = willRender
     this.action = action
     this.historyChanged = historyChanged
     this.referrer = referrer
     this.snapshotHTML = snapshotHTML
     this.response = response
     this.isSamePage = this.delegate.locationWithActionIsSamePage(this.location, this.action)
+    this.optionalDelegate = optionalDelegate
   }
 
   get adapter() {
@@ -126,6 +131,7 @@ export class Visit implements FetchRequestDelegate {
       this.state = VisitState.started
       this.adapter.visitStarted(this)
       this.delegate.visitStarted(this)
+      if (this.optionalDelegate.visitStarted) this.optionalDelegate.visitStarted(this)
     }
   }
 
@@ -392,6 +398,7 @@ export class Visit implements FetchRequestDelegate {
     if (!this.snapshotCached) {
       this.view.cacheSnapshot()
       this.snapshotCached = true
+      if (this.optionalDelegate.visitCachedSnapshot) this.optionalDelegate.visitCachedSnapshot(this)
     }
   }
 

src/core/frames/frame_controller.ts

@@ -4,6 +4,7 @@ import { FetchResponse } from "../../http/fetch_response"
 import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
 import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
 import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
+import { Visit } from "../drive/visit"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
 import { getAction, expandURL, urlsAreEqual, locationIsVisitable, Locatable } from "../url"
@@ -260,11 +261,24 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
     const action = getAttribute("data-turbo-action", submitter, element, frame)
 
     if (isAction(action)) {
-      const proposeVisit = async (event: Event) => {
-        const { detail: { fetchResponse: { location, redirected, statusCode } } } = event as CustomEvent
-        const responseHTML = document.documentElement.outerHTML
-
-        session.visit(location, { willRender: false, action, response: { redirected, responseHTML, statusCode } })
+      const clone = frame.cloneNode(true)
+      const proposeVisit = () => {
+        const { ownerDocument, id, src } = frame
+        if (src) {
+          const snapshotHTML = ownerDocument.documentElement.outerHTML
+          let snapshot: Snapshot
+
+          const delegate = {
+            visitStarted(visit: Visit) {
+              snapshot = visit.view.snapshot
+            },
+            visitCachedSnapshot() {
+              snapshot.element.querySelector("#" + id)?.replaceWith(clone)
+            }
+          }
+
+          session.visit(src, { willRender: false, action, snapshotHTML, delegate })
+        }
       }
 
       frame.addEventListener("turbo:frame-render", proposeVisit , { once: true })

src/core/session.ts

@@ -189,6 +189,9 @@ export class Session implements FormSubmitObserverDelegate, HistoryDelegate, Lin
     this.notifyApplicationAfterPageLoad(visit.getTimingMetrics())
   }
 
+  visitCachedSnapshot(visit: Visit) {
+  }
+
   locationWithActionIsSamePage(location: URL, action?: Action): boolean {
     return this.navigator.locationWithActionIsSamePage(location, action)
   }

src/tests/functional/frame_tests.ts

@@ -355,6 +355,38 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
   }
 
+  async "test navigating back after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames previous contents"() {
+    await this.clickSelector("#add-turbo-action-to-frame")
+    await this.clickSelector("#link-frame")
+    await this.nextBody
+    await this.goBack()
+    await this.nextBody
+
+    const title = await this.querySelector("h1")
+    const frameTitle = await this.querySelector("#frame h2")
+
+    this.assert.equal(await title.getVisibleText(), "Frames")
+    this.assert.equal(await frameTitle.getVisibleText(), "Frames: #frame")
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/frames.html")
+  }
+
+  async "test navigating back then forward after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames next contents"() {
+    await this.clickSelector("#add-turbo-action-to-frame")
+    await this.clickSelector("#link-frame")
+    await this.nextBody
+    await this.goBack()
+    await this.nextBody
+    await this.goForward()
+    await this.nextBody
+
+    const title = await this.querySelector("h1")
+    const frameTitle = await this.querySelector("#frame h2")
+
+    this.assert.equal(await title.getVisibleText(), "Frames")
+    this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
+  }
+
   async "test turbo:before-fetch-request fires on the frame element"() {
     await this.clickSelector("#hello a")
     this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-request"))