Skip to content

Commit

Permalink
Form submissions from frames should clear cache (hotwired#882)
Browse files Browse the repository at this point in the history
Usually when submitting a form with a method other than `GET`, we clear
the snapshot cache. This helps to avoid cases where we might show a
stale view of some data that we just mutated.

Form submissions from inside frames were not doing this. But they ought
to, for the same reason: that form submission may have mutated some data
that causes other recent pages to become stale.

To avoid this, we can clear the cache after processing a non-`GET` form,
or after any failed form submission, to mirror what we do when the form
is outside of any frames.
  • Loading branch information
kevinmcconnell authored Feb 27, 2023
1 parent 483ef32 commit 39affe5
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
6 changes: 5 additions & 1 deletion src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,16 @@ export class FrameController
const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter)

frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)

frame.delegate.loadResponse(response)

if (formSubmission.method !== FetchMethod.get) {
session.clearCache()
}
}

formSubmissionFailedWithResponse(formSubmission: FormSubmission, fetchResponse: FetchResponse) {
this.element.delegate.loadResponse(fetchResponse)
session.clearCache()
}

formSubmissionErrored(formSubmission: FormSubmission, error: Error) {
Expand Down
2 changes: 2 additions & 0 deletions src/tests/fixtures/frames.html
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ <h2>Frames: #nested-child</h2>
<a id="navigate-form-redirect" href="/src/tests/fixtures/frames/form-redirect.html" data-turbo-frame="form-redirect">Visit form-redirect.html</a>
<turbo-frame id="form-redirect"></turbo-frame>

<a id="navigate-form-redirect-as-new" href="/src/tests/fixtures/frames/form-redirect.html">Visit form-redirect.html as new page</a>

<turbo-frame id="part">
<a id="frame-part" href="/src/tests/fixtures/frames/part.html">Load #part</a>
</turbo-frame>
Expand Down
18 changes: 17 additions & 1 deletion src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Page, test } from "@playwright/test"
import { Page, test, expect } from "@playwright/test"
import { assert, Assertion } from "chai"
import {
attributeForSelector,
Expand Down Expand Up @@ -877,6 +877,22 @@ test("test navigating a eager frame with a link[method=get] that does not fetch
assert.equal(pathname(page.url()), "/src/tests/fixtures/page_with_eager_frame.html")
})

test("form submissions from frames clear snapshot cache", async ({ page }) => {
await page.evaluate(() => {
document.querySelector("h1")!.textContent = "Changed"
})

await expect(page.locator("h1")).toHaveText("Changed")

await page.click("#navigate-form-redirect-as-new")
await expect(page.locator("h1")).toHaveText("Page One Form")
await page.click("#submit-form")
await expect(page.locator("h2")).toHaveText("Form Redirected")
await page.goBack()

await expect(page.locator("h1")).not.toHaveText("Changed")
})

async function withoutChangingEventListenersCount(page: Page, callback: () => Promise<void>) {
const name = "eventListenersAttachedToDocument"
const setup = () => {
Expand Down

0 comments on commit 39affe5

Please sign in to comment.