Reload page if failed form changes tracked content (#759)

When a form submission fails with a client error, the response from the form replaces the current page content. In Turbo 7.1.x, whenever this replacement would affect resources marked with `data-turbo-track="reload"`, a reload of the page is triggered instead. In 7.2.0, we do not perform this reload. Instead, the form's response is discarded. This difference can affect situations like expired sessions, where the previous behaviour of reloading the page would cause a new session to be established. If we instead allow the response to be discarded, actions can fail silently. This commit changes the behaviour to reload the current page when `reload` is called outside of a current visit, in order to match the behaviour of 7.1, and to not discard the response from the form submission.
hotwired · Oct 12, 2022 · 728a561 · 728a561
1 parent b3facd8
commit 728a561
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 3 deletions.
diff --git a/src/core/native/browser_adapter.ts b/src/core/native/browser_adapter.ts
@@ -123,9 +123,7 @@ export class BrowserAdapter implements Adapter {
   reload(reason: ReloadReason) {
     dispatch("turbo:reload", { detail: reason })
 
-    if (!this.location) return
-
-    window.location.href = this.location.toString()
+    window.location.href = this.location?.toString() || window.location.href
   }
 
   get navigator() {

diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html
@@ -33,6 +33,7 @@
       <h1>Rendering</h1>
       <p><a id="same-origin-link" href="/src/tests/fixtures/one.html">Same-origin link</a></p>
       <p><a id="tracked-asset-change-link" href="/src/tests/fixtures/tracked_asset_change.html">Tracked asset change</a></p>
+      <form id="tracked-asset-change-form" action="/__turbo/notfound" method="post"><button type="submit">Submit</button></form>
       <p><a id="tracked-nonce-tag-link" href="/src/tests/fixtures/tracked_nonce_tag.html">Tracked nonce tag</a></p>
       <p><a id="additional-assets-link" href="/src/tests/fixtures/additional_assets.html">Additional assets</a></p>
       <p><a id="head-script-link" href="/src/tests/fixtures/head_script.html">Head script</a></p>

diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts
@@ -65,6 +65,37 @@ test("test reloads when tracked elements change", async ({ page }) => {
   assert.equal(reason, "tracked_element_mismatch")
 })
 
+test("test reloads when tracked elements change due to failed form submission", async ({ page }) => {
+  await page.click("#tracked-asset-change-form button")
+  await page.evaluate(() => {
+    window.addEventListener(
+      "turbo:reload",
+      (e: any) => {
+        localStorage.setItem("reason", e.detail.reason)
+      },
+      { once: true }
+    )
+
+    window.addEventListener(
+      "beforeunload",
+      () => {
+        localStorage.setItem("unloaded", "true")
+      },
+      { once: true }
+    )
+  })
+
+  await page.click("#tracked-asset-change-form button")
+
+  const reason = await page.evaluate(() => localStorage.getItem("reason"))
+  const unloaded = await page.evaluate(() => localStorage.getItem("unloaded"))
+
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/rendering.html")
+  assert.equal(await visitAction(page), "load")
+  assert.equal(reason, "tracked_element_mismatch")
+  assert.equal(unloaded, "true")
+})
+
 test("test before-render event supports custom render function", async ({ page }) => {
   await page.evaluate(() =>
     addEventListener("turbo:before-render", (event) => {

diff --git a/src/tests/server.ts b/src/tests/server.ts
@@ -86,6 +86,10 @@ router.post("/messages", (request, response) => {
   }
 })
 
+router.post("/notfound", (request, response) => {
+  response.type("html").status(404).send("<html><body><h1>Not found</h1></body></html>")
+})
+
 router.get("/stream-response", (request, response) => {
   const params = { ...request.body, ...request.query }
   const { content, target, targets } = params