From 256418fee0178ee483d82cd9bb579bd5df5a151f Mon Sep 17 00:00:00 2001
From: Sean Doyle <seanpdoyle@users.noreply.github.com>
Date: Thu, 11 Aug 2022 03:29:54 -0400
Subject: [PATCH] Bugfix: Redirects and `[data-turbo-cache=false]` (#674)

When redirecting to a page that contains elements marked with
`[data-turbo-cache="false"]`, those elements are removed _before_ the
initial render, instead of _after_ the render and _before_ the page is
cached.

This behavior seems to have stemmed from [hotwired/turbo#516][], which
was shipped in response to [hotwired/turbo#515][].

As an alternative to the `willRender: false` option passed to
`this.adapter.visitProposedToLocation` in `Visit.followRedirect`, the
implementation can instead [rely on the presence of the
`turbo-frame[complete]`][comment] to guard against double fetching.

To guard against regressions, this commit adds coverage for the unwanted
behavior by redirecting from `navigation.html` to `cache_observer.html`,
and asserting the presence of a `[data-turbo-cache="false"]` element
that resembles and application's Flash messaging.

[hotwired/turbo#515]: https://github.com/hotwired/turbo/issues/515
[hotwired/turbo#516]: https://github.com/hotwired/turbo/pull/516
[comment]: https://github.com/hotwired/turbo/issues/515#issuecomment-1159738755
[hotwired/turbo#487]: https://github.com/hotwired/turbo/pull/487
---
 src/core/drive/visit.ts                      |  1 -
 src/tests/fixtures/navigation.html           |  1 +
 src/tests/functional/cache_observer_tests.ts | 20 +++++++++++++++-----
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts
index 6d709f2ff..0763aa13a 100644
--- a/src/core/drive/visit.ts
+++ b/src/core/drive/visit.ts
@@ -322,7 +322,6 @@ export class Visit implements FetchRequestDelegate {
     if (this.redirectedToLocation && !this.followedRedirect && this.response?.redirected) {
       this.adapter.visitProposedToLocation(this.redirectedToLocation, {
         action: "replace",
-        willRender: false,
         response: this.response,
       })
       this.followedRedirect = true
diff --git a/src/tests/fixtures/navigation.html b/src/tests/fixtures/navigation.html
index 54cd56547..5d89a8df0 100644
--- a/src/tests/fixtures/navigation.html
+++ b/src/tests/fixtures/navigation.html
@@ -68,6 +68,7 @@ <h1>Navigation</h1>
       <p><custom-link-element id="custom-link-element" link="/src/tests/fixtures/one.html" text="Same-origin unannotated custom element link"></custom-link-element></p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
       <p><a id="targets-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe</a></p>
+      <p><a id="redirect-to-cache-observer" href="/__turbo/redirect?path=/src/tests/fixtures/cache_observer.html">Redirect to cache_observer.html</a></p>
     </section>
 
     <turbo-frame id="hello" disabled></turbo-frame>
diff --git a/src/tests/functional/cache_observer_tests.ts b/src/tests/functional/cache_observer_tests.ts
index edc923391..0b61fc7ee 100644
--- a/src/tests/functional/cache_observer_tests.ts
+++ b/src/tests/functional/cache_observer_tests.ts
@@ -2,15 +2,25 @@ import { test } from "@playwright/test"
 import { assert } from "chai"
 import { hasSelector, nextBody } from "../helpers/page"
 
-test.beforeEach(async ({ page }) => {
+test("test removes stale elements", async ({ page }) => {
   await page.goto("/src/tests/fixtures/cache_observer.html")
-})
 
-test("test removes stale elements", async ({ page }) => {
-  assert(await hasSelector(page, "#flash"))
-  page.click("#link")
+  assert.equal(await page.textContent("#flash"), "Rendering")
+
+  await page.click("#link")
   await nextBody(page)
   await page.goBack()
   await nextBody(page)
+
   assert.notOk(await hasSelector(page, "#flash"))
 })
+
+test("test following a redirect renders a [data-turbo-cache=false] element before the cache omits it", async ({
+  page,
+}) => {
+  await page.goto("/src/tests/fixtures/navigation.html")
+  await page.click("#redirect-to-cache-observer")
+  await nextBody(page)
+
+  assert.equal(await page.textContent("#flash"), "Rendering")
+})