Skip to content

Commit

Permalink
Bugfix: Redirects and [data-turbo-cache=false]
Browse files Browse the repository at this point in the history
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#516][], which
was shipped in response to [hotwired#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#515]: hotwired#515
[hotwired#516]: hotwired#516
[comment]: hotwired#515 (comment)
[hotwired#487]: hotwired#487
  • Loading branch information
seanpdoyle committed Aug 9, 2022
1 parent c4e0aba commit 4722714
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
1 change: 0 additions & 1 deletion src/core/drive/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/tests/fixtures/navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
20 changes: 15 additions & 5 deletions src/tests/functional/cache_observer_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 302 Found redirect with a page with a [data-turbo-cache=false] element renders it 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")
})

0 comments on commit 4722714

Please sign in to comment.