Update the history before rendering promoted frames

src/core/drive/visit.ts

@@ -6,7 +6,7 @@ import { getAnchor } from "../url"
 import { Snapshot } from "../snapshot"
 import { PageSnapshot } from "./page_snapshot"
 import { Action } from "../types"
-import { uuid } from "../../util"
+import { getHistoryMethodForAction, uuid } from "../../util"
 import { PageView } from "./page_view"
 
 export interface VisitDelegate {
@@ -45,13 +45,16 @@ export type VisitOptions = {
   response?: VisitResponse
   visitCachedSnapshot(snapshot: Snapshot): void
   willRender: boolean
+  updateHistory: boolean
+  restorationIdentifier?: string
 }
 
 const defaultOptions: VisitOptions = {
   action: "advance",
   historyChanged: false,
   visitCachedSnapshot: () => {},
   willRender: true,
+  updateHistory: true,
 }
 
 export type VisitResponse = {
@@ -75,6 +78,7 @@ export class Visit implements FetchRequestDelegate {
   readonly timingMetrics: TimingMetrics = {}
   readonly visitCachedSnapshot: (snapshot: Snapshot) => void
   readonly willRender: boolean
+  readonly updateHistory: boolean
 
   followedRedirect = false
   frame?: number
@@ -99,10 +103,11 @@ export class Visit implements FetchRequestDelegate {
     this.location = location
     this.restorationIdentifier = restorationIdentifier || uuid()
 
-    const { action, historyChanged, referrer, snapshotHTML, response, visitCachedSnapshot, willRender } = {
-      ...defaultOptions,
-      ...options,
-    }
+    const { action, historyChanged, referrer, snapshotHTML, response, visitCachedSnapshot, willRender, updateHistory } =
+      {
+        ...defaultOptions,
+        ...options,
+      }
     this.action = action
     this.historyChanged = historyChanged
     this.referrer = referrer
@@ -111,6 +116,7 @@ export class Visit implements FetchRequestDelegate {
     this.isSamePage = this.delegate.locationWithActionIsSamePage(this.location, this.action)
     this.visitCachedSnapshot = visitCachedSnapshot
     this.willRender = willRender
+    this.updateHistory = updateHistory
     this.scrolled = !willRender
   }
 
@@ -171,9 +177,9 @@ export class Visit implements FetchRequestDelegate {
   }
 
   changeHistory() {
-    if (!this.historyChanged) {
+    if (!this.historyChanged && this.updateHistory) {
       const actionForHistory = this.location.href === this.referrer?.href ? "replace" : this.action
-      const method = this.getHistoryMethodForAction(actionForHistory)
+      const method = getHistoryMethodForAction(actionForHistory)
       this.history.update(method, this.location, this.restorationIdentifier)
       this.historyChanged = true
     }

src/core/frames/frame_controller.ts

@@ -7,7 +7,15 @@ import {
 import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
 import { FetchResponse } from "../../http/fetch_response"
 import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
-import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
+import {
+  clearBusyState,
+  getAttribute,
+  parseHTMLDocument,
+  markAsBusy,
+  uuid,
+  getHistoryMethodForAction,
+  getVisitAction,
+} from "../../util"
 import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
 import { Snapshot } from "../snapshot"
 import { ViewDelegate } from "../view"
@@ -17,7 +25,8 @@ import { FrameView } from "./frame_view"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
 import { FrameRenderer } from "./frame_renderer"
 import { session } from "../index"
-import { isAction } from "../types"
+import { isAction, Action } from "../types"
+import { VisitOptions } from "../drive/visit"
 
 export class FrameController
   implements
@@ -41,13 +50,18 @@ export class FrameController
   private connected = false
   private hasBeenLoaded = false
   private ignoredAttributes: Set<FrameElementObservedAttribute> = new Set()
+  private action: Action | null = null
+  private frame?: FrameElement
+  private resolveInterceptionPromise = (_value: any) => {}
+  readonly restorationIdentifier: string
 
   constructor(element: FrameElement) {
     this.element = element
     this.view = new FrameView(this, this.element)
     this.appearanceObserver = new AppearanceObserver(this, this.element)
     this.linkInterceptor = new LinkInterceptor(this, this.element)
     this.formInterceptor = new FormInterceptor(this, this.element)
+    this.restorationIdentifier = uuid()
   }
 
   connect() {
@@ -126,6 +140,12 @@ export class FrameController
         const snapshot = new Snapshot(await this.extractForeignFrameElement(body))
         const renderer = new FrameRenderer(this.view.snapshot, snapshot, false, false)
         if (this.view.renderPromise) await this.view.renderPromise
+        this.changeHistory()
+
+        const renderInterception = new Promise((resolve) => (this.resolveInterceptionPromise = resolve))
+        const immediateRender = session.frameWillRender(fetchResponse, this.element, this.resolveInterceptionPromise)
+        if (!immediateRender) await renderInterception
+
         await this.view.render(renderer)
         this.complete = true
         session.frameRendered(fetchResponse, this.element)
@@ -277,27 +297,39 @@ export class FrameController
   }
 
   private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
-    const action = getAttribute("data-turbo-action", submitter, element, frame)
+    this.action = getVisitAction(submitter, element, frame)
+    this.frame = frame
 
-    if (isAction(action)) {
+    if (isAction(this.action)) {
       const { visitCachedSnapshot } = new SnapshotSubstitution(frame)
       frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
         if (frame.src) {
           const { statusCode, redirected } = fetchResponse
           const responseHTML = frame.ownerDocument.documentElement.outerHTML
           const response = { statusCode, redirected, responseHTML }
-
-          session.visit(frame.src, {
-            action,
+          const options: Partial<VisitOptions> = {
             response,
             visitCachedSnapshot,
             willRender: false,
-          })
+            updateHistory: false,
+            restorationIdentifier: this.restorationIdentifier,
+          }
+
+          if (this.action) options.action = this.action
+
+          session.visit(frame.src, options)
         }
       }
     }
   }
 
+  changeHistory() {
+    if (this.action && this.frame) {
+      const method = getHistoryMethodForAction(this.action)
+      session.history.update(method, expandURL(this.frame.src || ""), this.restorationIdentifier)
+    }
+  }
+
   private findFrameElement(element: Element, submitter?: HTMLElement) {
     const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target")
     return getFrameElementById(id) ?? this.element

src/core/native/browser_adapter.ts

@@ -24,7 +24,7 @@ export class BrowserAdapter implements Adapter {
   }
 
   visitProposedToLocation(location: URL, options?: Partial<VisitOptions>) {
-    this.navigator.startVisit(location, uuid(), options)
+    this.navigator.startVisit(location, options?.restorationIdentifier || uuid(), options)
   }
 
   visitStarted(visit: Visit) {

src/core/session.ts

@@ -294,6 +294,11 @@ export class Session
     this.notifyApplicationAfterFrameRender(fetchResponse, frame)
   }
 
+  frameWillRender(fetchResponse: FetchResponse, frame: FrameElement, resume: (value: any) => void) {
+    const event = this.notifyApplicationBeforeFrameRender(fetchResponse, frame, resume)
+    return !event.defaultPrevented
+  }
+
   // Application events
 
   applicationAllowsFollowingLinkToLocation(link: Element, location: URL) {
@@ -369,6 +374,14 @@ export class Session
     })
   }
 
+  notifyApplicationBeforeFrameRender(fetchResponse: FetchResponse, frame: FrameElement, resume: (value: any) => void) {
+    return dispatch("turbo:before-frame-render", {
+      detail: { fetchResponse, resume },
+      target: frame,
+      cancelable: true,
+    })
+  }
+
   // Helpers
 
   formElementDriveEnabled(element?: Element) {

src/tests/fixtures/tabs.html

@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Tabs</title>
+    <script src="/dist/turbo.es2017-umd.js"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <h1>Tabs</h1>
+
+    <turbo-frame id="tab-frame" data-turbo-action="advance">
+      <div>
+        <a id="tab-1" href="/src/tests/fixtures/tabs.html">Tab 1</a>
+        <a id="tab-2" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
+        <a id="tab-3" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
+      </div>
+
+      <div id="tab-content">One</div>
+    </turbo-frame>
+  </body>
+</html>

src/tests/fixtures/tabs/three.html

@@ -0,0 +1,9 @@
+<turbo-frame id="tab-frame" data-turbo-action="advance">
+  <div>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs.html">Tab 1</a>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
+  </div>
+
+  <div id="tab-content">Three</div>
+</turbo-frame>

src/tests/fixtures/tabs/two.html

@@ -0,0 +1,9 @@
+<turbo-frame id="tab-frame" data-turbo-action="advance">
+  <div>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs.html">Tab 1</a>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
+    <a id="drive_enabled" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
+  </div>
+
+  <div id="tab-content">Two</div>
+</turbo-frame>

src/tests/fixtures/test.js

@@ -29,6 +29,7 @@
   "turbo:before-fetch-request",
   "turbo:before-fetch-response",
   "turbo:visit",
+  "turbo:before-frame-render",
   "turbo:frame-load",
   "turbo:frame-render",
   "turbo:reload"

src/tests/functional/frame_navigation_tests.ts

@@ -22,6 +22,29 @@ export class FrameNavigationTests extends TurboDriveTestCase {
 
     await this.nextEventOnTarget("frame", "turbo:frame-load")
   }
+
+  async "test promoted frame navigation updates the URL before rendering"() {
+    await this.goToLocation("/src/tests/fixtures/tabs.html")
+
+    this.remote.execute(() => {
+      addEventListener("turbo:before-frame-render", () => {
+        localStorage.setItem("beforeRenderUrl", window.location.pathname)
+        localStorage.setItem("beforeRenderContent", document.querySelector("#tab-content")?.textContent || "")
+      })
+    })
+
+    await this.clickSelector("#tab-2")
+    await this.nextEventNamed("turbo:before-frame-render")
+
+    this.assert.equal(await this.getFromLocalStorage("beforeRenderUrl"), "/src/tests/fixtures/tabs/two.html")
+    this.assert.equal(await this.getFromLocalStorage("beforeRenderContent"), "One")
+
+    await this.nextEventNamed("turbo:frame-render")
+
+    const content = await this.querySelector("#tab-content")
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/tabs/two.html")
+    this.assert.equal(await content.getVisibleText(), "Two")
+  }
 }
 
 FrameNavigationTests.registerSuite()

src/tests/functional/frame_tests.ts

@@ -609,6 +609,40 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.equal(await this.pathname, "/src/tests/fixtures/page_with_eager_frame.html")
   }
 
+  async "test pauses and resumes frame renders"() {
+    this.setupPausableRender()
+
+    await this.clickSelector("#hello a")
+    await this.nextBeat
+
+    this.assert.strictEqual(await this.getAlertText(), "Continue rendering?")
+    await this.acceptAlert()
+
+    await this.nextBeat
+
+    const frameText = await this.querySelector("#frame h2")
+    this.assert.equal(await frameText.getVisibleText(), "Frame: Loaded")
+  }
+
+  async "test aborts frame rendering"() {
+    this.setupPausableRender()
+
+    await this.clickSelector("#hello a")
+    await this.nextBeat
+
+    this.assert.strictEqual(await this.getAlertText(), "Continue rendering?")
+    await this.dismissAlert()
+
+    await this.nextBeat
+    this.assert.strictEqual(await this.getAlertText(), "Rendering aborted")
+    await this.acceptAlert()
+
+    await this.nextBeat
+
+    const frameText = await this.querySelector("#frame h2")
+    this.assert.equal(await frameText.getVisibleText(), "Frames: #frame")
+  }
+
   async withoutChangingEventListenersCount(callback: () => void) {
     const name = "eventListenersAttachedToDocument"
     const setup = () => {
@@ -678,6 +712,20 @@ export class FrameTests extends TurboDriveTestCase {
   get frameScriptEvaluationCount(): Promise<number | undefined> {
     return this.evaluate(() => window.frameScriptEvaluationCount)
   }
+
+  setupPausableRender() {
+    this.remote.execute(() => {
+      addEventListener("turbo:before-frame-render", (event) => {
+        event.preventDefault()
+
+        if (confirm("Continue rendering?")) {
+          ;(event as CustomEvent).detail.resume()
+        } else {
+          alert("Rendering aborted")
+        }
+      })
+    })
+  }
 }
 
 declare global {

src/util.ts

@@ -1,3 +1,5 @@
+import { Action, isAction } from "./core/types"
+
 export type DispatchOptions = {
   target: EventTarget
   cancelable: boolean
@@ -92,3 +94,19 @@ export function clearBusyState(...elements: Element[]) {
     element.removeAttribute("aria-busy")
   }
 }
+
+export function getHistoryMethodForAction(action: Action) {
+  switch (action) {
+    case "replace":
+      return history.replaceState
+    case "advance":
+    case "restore":
+      return history.pushState
+  }
+}
+
+export function getVisitAction(...elements: (Element | undefined)[]): Action | null {
+  const action = getAttribute("data-turbo-action", ...elements)
+
+  return isAction(action) ? action : null
+}