Form Link Submissions: Don't remove <form> until submission is comp…

…lete (#678) * Improve flaky Frame Test Instead of timing frame navigations by beat, use `nextEventOnTarget`, target the frame, then assert about its header text content. * Form Link Submissions: Don't remove `<form>` until submission is complete When an `<a>` element annotated with `[data-turbo-method]` is clicked, Turbo Drive creates, appends, submits, and removes a `<form>` element behind the scenes. Unfortunately, prior to this commit, the `<form>` element is submitted then removed synchronously and immediately. This means that by the time the `submit` events (and resulting `turbo:before-fetch-request` and `turbo:submit-start` events) bubble up through the document, the `<form>` element is already disconnected. In its absence, the `document.documentElement` serves as the event's target. This commit changes that process to defer the elements removal until _after_ it dispatches a `turbo:submit-end` event. The result is that the element is present throughout all the events, and is accessible through `event.target`.
hotwired · Aug 11, 2022 · 28a5dc4 · 28a5dc4
1 parent b1eed57
commit 28a5dc4
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 12 deletions.
diff --git a/src/observers/form_link_click_observer.ts b/src/observers/form_link_click_observer.ts
@@ -51,7 +51,7 @@ export class FormLinkClickObserver implements LinkClickObserverDelegate {
     this.delegate.submittedFormLinkToLocation(link, location, form)
 
     document.body.appendChild(form)
-    form.requestSubmit()
-    form.remove()
+    form.addEventListener("turbo:submit-end", () => form.remove(), { once: true })
+    requestAnimationFrame(() => form.requestSubmit())
   }
 }
diff --git a/src/tests/fixtures/form.html b/src/tests/fixtures/form.html
@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html id="html">
+<html id="html" data-skip-event-details="turbo:submit-start turbo:submit-end">
   <head>
     <meta charset="utf-8">
     <title>Form</title>

diff --git a/src/tests/fixtures/test.js b/src/tests/fixtures/test.js
@@ -46,6 +46,8 @@
   "turbo:load",
   "turbo:render",
   "turbo:before-fetch-request",
+  "turbo:submit-start",
+  "turbo:submit-end",
   "turbo:before-fetch-response",
   "turbo:visit",
   "turbo:before-frame-render",

diff --git a/src/tests/functional/form_submission_tests.ts b/src/tests/functional/form_submission_tests.ts
@@ -798,6 +798,24 @@ test("test form submission targeting a frame submits the Turbo-Frame header", as
   assert.ok(fetchOptions.headers["Turbo-Frame"], "submits with the Turbo-Frame header")
 })
 
+test("test link method form submission dispatches events from a connected <form> element", async ({ page }) => {
+  await page.evaluate(() =>
+    new MutationObserver(([record]) => {
+      for (const form of record.addedNodes) {
+        if (form instanceof HTMLFormElement) form.id = "a-form-link"
+      }
+    }).observe(document.body, { childList: true })
+  )
+
+  await page.click("#stream-link-method-within-form-outside-frame")
+  await nextEventOnTarget(page, "a-form-link", "turbo:before-fetch-request")
+  await nextEventOnTarget(page, "a-form-link", "turbo:submit-start")
+  await nextEventOnTarget(page, "a-form-link", "turbo:before-fetch-response")
+  await nextEventOnTarget(page, "a-form-link", "turbo:submit-end")
+
+  assert.notOk(await hasSelector(page, "a-form-link"), "the <form> is removed")
+})
+
 test("test link method form submission submits a single request", async ({ page }) => {
   let requestCounter = 0
   page.on("request", () => requestCounter++)

diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts
@@ -331,20 +331,18 @@ test("test does not evaluate data-turbo-eval=false scripts", async ({ page }) =>
 })
 
 test("test redirecting in a form is still navigatable after redirect", async ({ page }) => {
-  await nextBeat()
   await page.click("#navigate-form-redirect")
-  await nextBeat()
-  assert.ok(await hasSelector(page, "#form-redirect"))
+  await nextEventOnTarget(page, "form-redirect", "turbo:frame-load")
+  assert.equal(await page.textContent("turbo-frame#form-redirect h2"), "Form Redirect")
 
-  await nextBeat()
   await page.click("#submit-form")
-  await nextBeat()
-  assert.ok(await hasSelector(page, "#form-redirected-header"))
+  await nextEventOnTarget(page, "form-redirect", "turbo:frame-load")
+  assert.equal(await page.textContent("turbo-frame#form-redirect h2"), "Form Redirected")
 
-  await nextBeat()
   await page.click("#navigate-form-redirect")
-  await nextBeat()
-  assert.ok(await hasSelector(page, "#form-redirect-header"))
+  await nextEventOnTarget(page, "form-redirect", "turbo:frame-load")
+
+  assert.equal(await page.textContent("turbo-frame#form-redirect h2"), "Form Redirect")
 })
 
 test("test 'turbo:frame-render' is triggered after frame has finished rendering", async ({ page }) => {