From be4c81fdbb4e1091bf80de32ef0f4da0f0bd8eee Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Thu, 29 Feb 2024 21:44:53 -0500
Subject: [PATCH] Prevent Refresh from interrupting ongoing Visit

Closes [#1209]

Defers the `Session.refresh` calls while handling `<turbo-stream
action="refresh">` elements if there is a `Visit` that's already
initiated and being handled by the `Navigator`.

[#1209]: https://github.com/hotwired/turbo/issues/1209
---
 src/core/drive/navigator.js                |  3 ++-
 src/core/session.js                        |  2 +-
 src/tests/fixtures/page_refresh.html       |  1 +
 src/tests/fixtures/test.js                 |  4 ++++
 src/tests/functional/page_refresh_tests.js | 27 ++++++++++++++++++----
 src/tests/helpers/page.js                  | 10 ++++++--
 6 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js
index 8d8d3e0be..e034a0be9 100644
--- a/src/core/drive/navigator.js
+++ b/src/core/drive/navigator.js
@@ -17,7 +17,7 @@ export class Navigator {
 
   startVisit(locatable, restorationIdentifier, options = {}) {
     this.stop()
-    this.currentVisit = new Visit(this, expandURL(locatable), restorationIdentifier, {
+    this.currentVisit = Visit(this, expandURL(locatable), restorationIdentifier, {
       referrer: this.location,
       ...options
     })
@@ -125,6 +125,7 @@ export class Navigator {
 
   visitCompleted(visit) {
     this.delegate.visitCompleted(visit)
+    delete this.currentVisit
   }
 
   locationWithActionIsSamePage(location, action) {
diff --git a/src/core/session.js b/src/core/session.js
index cdb978348..1047d4463 100644
--- a/src/core/session.js
+++ b/src/core/session.js
@@ -109,7 +109,7 @@ export class Session {
 
   refresh(url, requestId) {
     const isRecentRequest = requestId && this.recentRequests.has(requestId)
-    if (!isRecentRequest) {
+    if (!isRecentRequest && !this.navigator.currentVisit) {
       this.visit(url, { action: "replace", shouldCacheSnapshot: false })
     }
   }
diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html
index c0586677f..df5338b43 100644
--- a/src/tests/fixtures/page_refresh.html
+++ b/src/tests/fixtures/page_refresh.html
@@ -81,6 +81,7 @@
     <h1 id="title">Page to be refreshed</h1>
 
     <a href="/src/tests/fixtures/page_refresh.html" id="reload-link">Reload</a>
+    <a href="/__turbo/delayed_response" id="delayed_link">Navigate with delayed response</a>
 
     <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
       <h2>Frame to be morphed</h2>
diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js
index 7ba0f39e7..0dd66d48b 100644
--- a/src/tests/fixtures/test.js
+++ b/src/tests/fixtures/test.js
@@ -91,6 +91,10 @@
   "turbo:reload"
 ])
 
+window.visitLogs = []
+
+addEventListener("turbo:visit", ({ detail }) => window.visitLogs.push(detail))
+
 customElements.define(
   "custom-link-element",
   class extends HTMLElement {
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
index c5c116c08..84d765cb5 100644
--- a/src/tests/functional/page_refresh_tests.js
+++ b/src/tests/functional/page_refresh_tests.js
@@ -8,7 +8,8 @@ import {
   nextEventOnTarget,
   noNextEventOnTarget,
   noNextEventNamed,
-  getSearchParam
+  getSearchParam,
+  refreshWithStream
 } from "../helpers/page"
 
 test("renders a page refresh with morphing", async ({ page }) => {
@@ -26,16 +27,32 @@ test("async page refresh with turbo-stream", async ({ page }) => {
 
   await page.evaluate(() => document.querySelector("#title").innerText = "Updated")
   await expect(page.locator("#title")).toHaveText("Updated")
-
-  await page.evaluate(() => {
-    document.body.insertAdjacentHTML("beforeend", `<turbo-stream action="refresh"></turbo-stream>`)
-  })
+  await refreshWithStream(page)
 
   await expect(page.locator("#title")).not.toHaveText("Updated")
   await expect(page.locator("#title")).toHaveText("Page to be refreshed")
   expect(await noNextEventNamed(page, "turbo:before-cache")).toBeTruthy()
 })
 
+test("async page refresh with turbo-stream sequentially initiate Visits", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/page_refresh.html")
+  await refreshWithStream(page)
+  await nextEventNamed(page, "turbo:morph")
+  await nextEventNamed(page, "turbo:load")
+
+  await refreshWithStream(page)
+  await nextEventNamed(page, "turbo:morph")
+  await nextEventNamed(page, "turbo:load")
+})
+
+test("async page refresh with turbo-stream does not interrupt an initiated Visit", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/page_refresh.html")
+  await page.click("#delayed_link")
+  await refreshWithStream(page)
+
+  await expect(page.locator("h1")).toHaveText("One")
+})
+
 test("dispatches a turbo:before-morph-element and turbo:morph-element event for each morphed element", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
   await page.fill("#form-text", "Morph me")
diff --git a/src/tests/helpers/page.js b/src/tests/helpers/page.js
index 760b6631f..34069bed7 100644
--- a/src/tests/helpers/page.js
+++ b/src/tests/helpers/page.js
@@ -226,6 +226,10 @@ export function readMutationLogs(page, length) {
   return readArray(page, "mutationLogs", length)
 }
 
+export function refreshWithStream(page) {
+  return page.evaluate(() => document.body.insertAdjacentHTML("beforeend", `<turbo-stream action="refresh"></turbo-stream>`))
+}
+
 export function search(url) {
   const { search } = new URL(url)
 
@@ -284,8 +288,10 @@ export function textContent(page, html) {
 export function visitAction(page) {
   return page.evaluate(() => {
     try {
-      return window.Turbo.navigator.currentVisit.action
-    } catch (error) {
+      const lastVisit = window.visitLogs[window.visitLogs.length - 1]
+
+      return lastVisit.action
+    } catch {
       return "load"
     }
   })