Preserve page state while promoting Frame-to-Visit

src/core/drive/navigator.ts

@@ -133,10 +133,6 @@ 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/page_renderer.ts

@@ -11,7 +11,9 @@ export class PageRenderer extends Renderer<HTMLBodyElement, PageSnapshot> {
   }
 
   async render() {
-    this.replaceBody()
+    if (this.willRender) {
+      this.replaceBody()
+    }
   }
 
   finishRendering() {

src/core/drive/page_view.ts

@@ -15,8 +15,8 @@ export class PageView extends View<Element, PageSnapshot, PageViewRenderer, Page
   readonly snapshotCache = new SnapshotCache(10)
   lastRenderedLocation = new URL(location.href)
 
-  renderPage(snapshot: PageSnapshot, isPreview = false) {
-    const renderer = new PageRenderer(this.snapshot, snapshot, isPreview)
+  renderPage(snapshot: PageSnapshot, isPreview = false, willRender = true) {
+    const renderer = new PageRenderer(this.snapshot, snapshot, isPreview, willRender)
     return this.render(renderer)
   }
 
@@ -34,7 +34,9 @@ export class PageView extends View<Element, PageSnapshot, PageViewRenderer, Page
       this.delegate.viewWillCacheSnapshot()
       const { snapshot, lastRenderedLocation: location } = this
       await nextEventLoopTick()
-      this.snapshotCache.put(location, snapshot.clone())
+      const cachedSnapshot = snapshot.clone()
+      this.snapshotCache.put(location, cachedSnapshot)
+      return cachedSnapshot
     }
   }
 

src/core/drive/visit.ts

@@ -3,6 +3,7 @@ import { FetchMethod, FetchRequest, FetchRequestDelegate } from "../../http/fetc
 import { FetchResponse } from "../../http/fetch_response"
 import { History } from "./history"
 import { getAnchor } from "../url"
+import { Snapshot } from "../snapshot"
 import { PageSnapshot } from "./page_snapshot"
 import { Action } from "../types"
 import { uuid } from "../../util"
@@ -15,7 +16,6 @@ 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
 }
@@ -39,17 +39,19 @@ export enum VisitState {
 
 export type VisitOptions = {
   action: Action,
-  delegate: Partial<VisitDelegate>
   historyChanged: boolean,
   referrer?: URL,
   snapshotHTML?: string,
   response?: VisitResponse
+  visitCachedSnapshot(snapshot: Snapshot): void
+  willRender: boolean
 }
 
 const defaultOptions: VisitOptions = {
   action: "advance",
-  delegate: {},
   historyChanged: false,
+  visitCachedSnapshot: () => {},
+  willRender: true,
 }
 
 export type VisitResponse = {
@@ -71,7 +73,8 @@ export class Visit implements FetchRequestDelegate {
   readonly action: Action
   readonly referrer?: URL
   readonly timingMetrics: TimingMetrics = {}
-  readonly optionalDelegate: Partial<VisitDelegate>
+  readonly visitCachedSnapshot: (snapshot: Snapshot) => void
+  readonly willRender: boolean
 
   followedRedirect = false
   frame?: number
@@ -91,14 +94,16 @@ export class Visit implements FetchRequestDelegate {
     this.location = location
     this.restorationIdentifier = restorationIdentifier || uuid()
 
-    const { action, historyChanged, referrer, snapshotHTML, response, delegate: optionalDelegate } = { ...defaultOptions, ...options }
+    const { action, historyChanged, referrer, snapshotHTML, response, visitCachedSnapshot, willRender } = { ...defaultOptions, ...options }
     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
+    this.visitCachedSnapshot = visitCachedSnapshot
+    this.willRender = willRender
+    this.scrolled = !willRender
   }
 
   get adapter() {
@@ -127,7 +132,6 @@ 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)
     }
   }
 
@@ -213,7 +217,7 @@ export class Visit implements FetchRequestDelegate {
         this.cacheSnapshot()
         if (this.view.renderPromise) await this.view.renderPromise
         if (isSuccessful(statusCode) && responseHTML != null) {
-          await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML))
+          await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), false, this.willRender)
           this.adapter.visitRendered(this)
           this.complete()
         } else {
@@ -255,7 +259,7 @@ export class Visit implements FetchRequestDelegate {
           this.adapter.visitRendered(this)
         } else {
           if (this.view.renderPromise) await this.view.renderPromise
-          await this.view.renderPage(snapshot, isPreview)
+          await this.view.renderPage(snapshot, isPreview, this.willRender)
           this.adapter.visitRendered(this)
           if (!isPreview) {
             this.complete()
@@ -386,15 +390,14 @@ export class Visit implements FetchRequestDelegate {
     } else if (this.action == "restore") {
       return !this.hasCachedSnapshot()
     } else {
-      return true
+      return this.willRender
     }
   }
 
   cacheSnapshot() {
     if (!this.snapshotCached) {
-      this.view.cacheSnapshot()
+      this.view.cacheSnapshot().then(snapshot => snapshot && this.visitCachedSnapshot(snapshot))
       this.snapshotCached = true
-      if (this.optionalDelegate.visitCachedSnapshot) this.optionalDelegate.visitCachedSnapshot(this)
     }
   }
 

src/core/frames/frame_controller.ts

@@ -4,7 +4,6 @@ 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, VisitDelegate } from "../drive/visit"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
 import { getAction, expandURL, urlsAreEqual, locationIsVisitable, Locatable } from "../url"
@@ -23,6 +22,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   readonly formInterceptor: FormInterceptor
   currentURL?: string | null
   formSubmission?: FormSubmission
+  fetchResponseLoaded = (fetchResponse: FetchResponse) => {}
   private currentFetchRequest: FetchRequest | null = null
   private resolveVisitPromise = () => {}
   private connected = false
@@ -108,15 +108,18 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
       if (html) {
         const { body } = parseHTMLDocument(html)
         const snapshot = new Snapshot(await this.extractForeignFrameElement(body))
-        const renderer = new FrameRenderer(this.view.snapshot, snapshot, false)
+        const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false)
         if (this.view.renderPromise) await this.view.renderPromise
         await this.view.render(renderer)
         session.frameRendered(fetchResponse, this.element)
         session.frameLoaded(this.element)
+        this.fetchResponseLoaded(fetchResponse)
       }
     } catch (error) {
       console.error(error)
       this.view.invalidate()
+    } finally {
+      this.fetchResponseLoaded = () => {}
     }
   }
 
@@ -261,19 +264,16 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
     const action = getAttribute("data-turbo-action", submitter, element, frame)
 
     if (isAction(action)) {
-      const delegate = new SnapshotSubstitution(frame)
-      const proposeVisit = (event: Event) => {
-        const { target, detail: { fetchResponse } } = event as CustomEvent
-        if (target instanceof FrameElement && target.src) {
+      const { visitCachedSnapshot } = new SnapshotSubstitution(frame)
+      frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
+        if (frame.src) {
           const { statusCode, redirected } = fetchResponse
-          const responseHTML = target.ownerDocument.documentElement.outerHTML
+          const responseHTML = frame.ownerDocument.documentElement.outerHTML
           const response = { statusCode, redirected, responseHTML }
 
-          session.visit(target.src, { action, response, delegate })
+          session.visit(frame.src, { action, response, visitCachedSnapshot, willRender: false })
         }
       }
-
-      frame.addEventListener("turbo:frame-render", proposeVisit , { once: true })
     }
   }
 
@@ -395,26 +395,19 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   }
 }
 
-class SnapshotSubstitution implements Partial<VisitDelegate> {
+class SnapshotSubstitution {
   private readonly clone: Node
   private readonly id: string
-  private snapshot?: Snapshot
 
   constructor(element: FrameElement) {
     this.clone = element.cloneNode(true)
     this.id = element.id
   }
 
-  visitStarted(visit: Visit) {
-    this.snapshot = visit.view.snapshot
-  }
-
-  visitCachedSnapshot() {
-    const { snapshot, id, clone } = this
+  visitCachedSnapshot = ({ element }: Snapshot) => {
+    const { id, clone } = this
 
-    if (snapshot) {
-      snapshot.element.querySelector("#" + id)?.replaceWith(clone)
-    }
+    element.querySelector("#" + id)?.replaceWith(clone)
   }
 }
 

src/core/renderer.ts

@@ -10,13 +10,15 @@ export abstract class Renderer<E extends Element, S extends Snapshot<E> = Snapsh
   readonly currentSnapshot: S
   readonly newSnapshot: S
   readonly isPreview: boolean
+  readonly willRender: boolean
   readonly promise: Promise<void>
   private resolvingFunctions?: ResolvingFunctions<void>
 
-  constructor(currentSnapshot: S, newSnapshot: S, isPreview: boolean) {
+  constructor(currentSnapshot: S, newSnapshot: S, isPreview: boolean, willRender = true) {
     this.currentSnapshot = currentSnapshot
     this.newSnapshot = newSnapshot
     this.isPreview = isPreview
+    this.willRender = willRender
     this.promise = new Promise((resolve, reject) => this.resolvingFunctions = { resolve, reject })
   }
 

src/core/session.ts

@@ -189,9 +189,6 @@ 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/elements/frame_element.ts

@@ -11,6 +11,7 @@ export interface FrameElementDelegate {
   formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement): void
   linkClickIntercepted(element: Element, url: string): void
   loadResponse(response: FetchResponse): void
+  fetchResponseLoaded: (fetchResponse: FetchResponse) => void
   isLoading: boolean
 }
 

src/tests/fixtures/frames.html

@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html id="html">
+<html id="html" data-skip-event-details="turbo:before-render">
   <head>
     <meta charset="utf-8">
     <title>Frame</title>
@@ -9,9 +9,14 @@
       addEventListener("click", ({ target }) => {
         if (target.id == "add-turbo-action-to-frame") {
           target.closest("turbo-frame")?.setAttribute("data-turbo-action", "advance")
+        } else if (target.id == "remove-target-from-hello") {
+          document.getElementById("hello").removeAttribute("target")
         }
       })
     </script>
+    <style>
+      .push-off-screen { margin-top: 1000px; }
+    </style>
   </head>
   <body>
     <h1>Frames</h1>
@@ -45,8 +50,12 @@ <h2>Frames: #frame</h2>
       <h2>Frames: #hello</h2>
 
       <a href="/src/tests/fixtures/frames/frame.html">Load #frame</a>
+      <button type="button" id="remove-target-from-hello">Remove #hello[target]</button>
+
     </turbo-frame>
 
+    <a id="link-hello-advance" href="/src/tests/fixtures/frames/hello.html" data-turbo-frame="hello" data-turbo-action="advance">advance #hello</a>
+
     <turbo-frame id="nested-root" target="frame">
       <h2>Frames: #nested-root</h2>
       <a id="inner-outer-frame-link" href="/src/tests/fixtures/frames/frame.html" data-turbo-frame="nested-child">Inner/Outer frame link</a>
@@ -104,5 +113,9 @@ <h2>Frames: #nested-child</h2>
     <form data-turbo-frame="frame" method="get" action="/src/tests/fixtures/frames/frame.html">
       <input id="outer-frame-submit" type="submit" value="Outer form submit">
     </form>
+
+    <hr class="push-off-screen">
+    <input id="below-the-fold-input">
+    <a id="below-the-fold-link-frame-action" data-turbo-action="advance" data-turbo-frame="frame" href="/src/tests/fixtures/frames/frame.html">Navigate #frame</a>
   </body>
 </html>

src/tests/fixtures/test.js

@@ -7,7 +7,9 @@
   }
 
   function eventListener(event) {
-    eventLogs.push([event.type, event.detail, event.target.id])
+    const skipped = document.documentElement.getAttribute("data-skip-event-details") || ""
+
+    eventLogs.push([event.type, skipped.includes(event.type) ? {} : event.detail, event.target.id])
   }
   window.mutationLogs = []