Prevent form links from submitting multiple requests

The `LinkInterceptor` class contains some Frame-specific checks that
were at the root of `<a data-turbo-method="...">` element clicks
submitting multiple HTTP requests.

To resolve the underlying issue, this commit changes the
`FormLinkInterceptor` class to rely on an instance of
`LinkClickObserver` instead of `LinkInterceptor`.

In order to make that possible, this commit also extends the
`LinkClickObserver` to accept a second argument to server as the
[EventTarget][] for the `click` event listeners. As a consumer, the
`FormLinkInterceptor` instances attached to `<turbo-frame>` element
scope their listeners to the element, while the `Session`'s
`FormLinkInterceptor` instance attaches a catch-all listener to the
`window`.

Since this commit changes the underlying event listening mechanism, it
also renames the `FormLinkInterceptor` and delegate to
`FormLinkClickObserver` to match the `LinkClickObserver` naming
patterns.

[EventTarget]: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget

src/core/frames/frame_controller.ts

@@ -24,7 +24,7 @@ import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
 import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/form_submit_observer"
 import { FrameView } from "./frame_view"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
-import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../../observers/form_link_interceptor"
+import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer"
 import { FrameRenderer } from "./frame_renderer"
 import { session } from "../index"
 import { isAction, Action } from "../types"
@@ -38,14 +38,14 @@ export class FrameController
     FormSubmitObserverDelegate,
     FormSubmissionDelegate,
     FrameElementDelegate,
-    FormLinkInterceptorDelegate,
+    FormLinkClickObserverDelegate,
     LinkInterceptorDelegate,
     ViewDelegate<FrameElement, Snapshot<FrameElement>>
 {
   readonly element: FrameElement
   readonly view: FrameView
   readonly appearanceObserver: AppearanceObserver
-  readonly formLinkInterceptor: FormLinkInterceptor
+  readonly formLinkClickObserver: FormLinkClickObserver
   readonly linkInterceptor: LinkInterceptor
   readonly formSubmitObserver: FormSubmitObserver
   formSubmission?: FormSubmission
@@ -64,7 +64,7 @@ export class FrameController
     this.element = element
     this.view = new FrameView(this, this.element)
     this.appearanceObserver = new AppearanceObserver(this, this.element)
-    this.formLinkInterceptor = new FormLinkInterceptor(this, this.element)
+    this.formLinkClickObserver = new FormLinkClickObserver(this, this.element)
     this.linkInterceptor = new LinkInterceptor(this, this.element)
     this.restorationIdentifier = uuid()
     this.formSubmitObserver = new FormSubmitObserver(this, this.element)
@@ -78,7 +78,7 @@ export class FrameController
       } else {
         this.loadSourceURL()
       }
-      this.formLinkInterceptor.start()
+      this.formLinkClickObserver.start()
       this.linkInterceptor.start()
       this.formSubmitObserver.start()
     }
@@ -88,7 +88,7 @@ export class FrameController
     if (this.connected) {
       this.connected = false
       this.appearanceObserver.stop()
-      this.formLinkInterceptor.stop()
+      this.formLinkClickObserver.stop()
       this.linkInterceptor.stop()
       this.formSubmitObserver.stop()
     }
@@ -177,13 +177,13 @@ export class FrameController
     this.loadSourceURL()
   }
 
-  // Form link interceptor delegate
+  // Form link click observer delegate
 
-  shouldInterceptFormLinkClick(link: Element): boolean {
+  willSubmitFormLinkToLocation(link: Element): boolean {
     return this.shouldInterceptNavigation(link)
   }
 
-  formLinkClickIntercepted(link: Element, form: HTMLFormElement): void {
+  submittedFormLinkToLocation(link: Element, _location: URL, form: HTMLFormElement): void {
     const frame = this.findFrameElement(link)
     if (frame) form.setAttribute("data-turbo-frame", frame.id)
   }
@@ -198,7 +198,7 @@ export class FrameController
     this.navigateFrame(element, url)
   }
 
-  // Form interceptor delegate
+  // Form submit observer delegate
 
   willSubmitForm(element: HTMLFormElement, submitter?: HTMLElement) {
     return element.closest("turbo-frame") == this.element && this.shouldInterceptNavigation(element, submitter)

src/core/session.ts

@@ -5,7 +5,7 @@ import { FormSubmitObserver, FormSubmitObserverDelegate } from "../observers/for
 import { FrameRedirector } from "./frames/frame_redirector"
 import { History, HistoryDelegate } from "./drive/history"
 import { LinkClickObserver, LinkClickObserverDelegate } from "../observers/link_click_observer"
-import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../observers/form_link_interceptor"
+import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../observers/form_link_click_observer"
 import { getAction, expandURL, locationIsVisitable, Locatable } from "./url"
 import { Navigator, NavigatorDelegate } from "./drive/navigator"
 import { PageObserver, PageObserverDelegate } from "../observers/page_observer"
@@ -38,7 +38,7 @@ export class Session
   implements
     FormSubmitObserverDelegate,
     HistoryDelegate,
-    FormLinkInterceptorDelegate,
+    FormLinkClickObserverDelegate,
     LinkClickObserverDelegate,
     NavigatorDelegate,
     PageObserverDelegate,
@@ -53,11 +53,11 @@ export class Session
 
   readonly pageObserver = new PageObserver(this)
   readonly cacheObserver = new CacheObserver()
-  readonly linkClickObserver = new LinkClickObserver(this)
+  readonly linkClickObserver = new LinkClickObserver(this, window)
   readonly formSubmitObserver = new FormSubmitObserver(this, document)
   readonly scrollObserver = new ScrollObserver(this)
   readonly streamObserver = new StreamObserver(this)
-  readonly formLinkInterceptor = new FormLinkInterceptor(this, document.documentElement)
+  readonly formLinkClickObserver = new FormLinkClickObserver(this, document.documentElement)
   readonly frameRedirector = new FrameRedirector(document.documentElement)
 
   drive = true
@@ -70,7 +70,7 @@ export class Session
     if (!this.started) {
       this.pageObserver.start()
       this.cacheObserver.start()
-      this.formLinkInterceptor.start()
+      this.formLinkClickObserver.start()
       this.linkClickObserver.start()
       this.formSubmitObserver.start()
       this.scrollObserver.start()
@@ -91,7 +91,7 @@ export class Session
     if (this.started) {
       this.pageObserver.stop()
       this.cacheObserver.stop()
-      this.formLinkInterceptor.stop()
+      this.formLinkClickObserver.stop()
       this.linkClickObserver.stop()
       this.formSubmitObserver.stop()
       this.scrollObserver.stop()
@@ -163,13 +163,13 @@ export class Session
     this.history.updateRestorationData({ scrollPosition: position })
   }
 
-  // Form link interceptor delegate
+  // Form click observer delegate
 
-  shouldInterceptFormLinkClick(_link: Element): boolean {
-    return true
+  willSubmitFormLinkToLocation(link: Element, location: URL): boolean {
+    return this.elementDriveEnabled(link) && locationIsVisitable(location, this.snapshot.rootLocation)
   }
 
-  formLinkClickIntercepted(_link: Element, _form: HTMLFormElement) {}
+  submittedFormLinkToLocation() {}
 
   // Link click observer delegate
 

src/observers/form_link_interceptor.ts → src/observers/form_link_click_observer.ts

@@ -1,17 +1,17 @@
-import { LinkInterceptor, LinkInterceptorDelegate } from "../core/frames/link_interceptor"
+import { LinkClickObserver, LinkClickObserverDelegate } from "./link_click_observer"
 
-export type FormLinkInterceptorDelegate = {
-  shouldInterceptFormLinkClick(link: Element): boolean
-  formLinkClickIntercepted(link: Element, form: HTMLFormElement): void
+export type FormLinkClickObserverDelegate = {
+  willSubmitFormLinkToLocation(link: Element, location: URL, event: MouseEvent): boolean
+  submittedFormLinkToLocation(link: Element, location: URL, form: HTMLFormElement): void
 }
 
-export class FormLinkInterceptor implements LinkInterceptorDelegate {
-  readonly linkInterceptor: LinkInterceptor
-  readonly delegate: FormLinkInterceptorDelegate
+export class FormLinkClickObserver implements LinkClickObserverDelegate {
+  readonly linkInterceptor: LinkClickObserver
+  readonly delegate: FormLinkClickObserverDelegate
 
-  constructor(delegate: FormLinkInterceptorDelegate, element: HTMLElement) {
+  constructor(delegate: FormLinkClickObserverDelegate, element: HTMLElement) {
     this.delegate = delegate
-    this.linkInterceptor = new LinkInterceptor(this, element)
+    this.linkInterceptor = new LinkClickObserver(this, element)
   }
 
   start() {
@@ -22,14 +22,15 @@ export class FormLinkInterceptor implements LinkInterceptorDelegate {
     this.linkInterceptor.stop()
   }
 
-  shouldInterceptLinkClick(link: Element): boolean {
+  willFollowLinkToLocation(link: Element, location: URL, originalEvent: MouseEvent): boolean {
     return (
-      this.delegate.shouldInterceptFormLinkClick(link) &&
+      this.delegate.willSubmitFormLinkToLocation(link, location, originalEvent) &&
       (link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream"))
     )
   }
 
-  linkClickIntercepted(link: Element, action: string): void {
+  followedLinkToLocation(link: Element, location: URL): void {
+    const action = location.href
     const form = document.createElement("form")
     form.setAttribute("data-turbo", "true")
     form.setAttribute("action", action)
@@ -47,7 +48,7 @@ export class FormLinkInterceptor implements LinkInterceptorDelegate {
     const turboStream = link.hasAttribute("data-turbo-stream")
     if (turboStream) form.setAttribute("data-turbo-stream", "")
 
-    this.delegate.formLinkClickIntercepted(link, form)
+    this.delegate.submittedFormLinkToLocation(link, location, form)
 
     document.body.appendChild(form)
     form.requestSubmit()

src/observers/link_click_observer.ts

@@ -7,33 +7,35 @@ export interface LinkClickObserverDelegate {
 
 export class LinkClickObserver {
   readonly delegate: LinkClickObserverDelegate
+  readonly eventTarget: EventTarget
   started = false
 
-  constructor(delegate: LinkClickObserverDelegate) {
+  constructor(delegate: LinkClickObserverDelegate, eventTarget: EventTarget) {
     this.delegate = delegate
+    this.eventTarget = eventTarget
   }
 
   start() {
     if (!this.started) {
-      addEventListener("click", this.clickCaptured, true)
+      this.eventTarget.addEventListener("click", this.clickCaptured, true)
       this.started = true
     }
   }
 
   stop() {
     if (this.started) {
-      removeEventListener("click", this.clickCaptured, true)
+      this.eventTarget.removeEventListener("click", this.clickCaptured, true)
       this.started = false
     }
   }
 
   clickCaptured = () => {
-    removeEventListener("click", this.clickBubbled, false)
-    addEventListener("click", this.clickBubbled, false)
+    this.eventTarget.removeEventListener("click", this.clickBubbled, false)
+    this.eventTarget.addEventListener("click", this.clickBubbled, false)
   }
 
-  clickBubbled = (event: MouseEvent) => {
-    if (this.clickEventIsSignificant(event)) {
+  clickBubbled = (event: Event) => {
+    if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) {
       const target = (event.composedPath && event.composedPath()[0]) || event.target
       const link = this.findLinkFromClickTarget(target)
       if (link && doesNotTargetIFrame(link)) {

src/tests/functional/form_submission_tests.ts

@@ -798,6 +798,49 @@ 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 submits a single request", async ({ page }) => {
+  let requestCounter = 0
+  page.on("request", () => requestCounter++)
+
+  await page.click("#stream-link-method-within-form-outside-frame")
+  await nextBeat()
+
+  const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
+
+  await noNextEventNamed(page, "turbo:before-fetch-request")
+
+  assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method")
+  assert.equal(requestCounter, 1, "submits a single HTTP request")
+})
+
+test("test link method form submission inside frame submits a single request", async ({ page }) => {
+  let requestCounter = 0
+  page.on("request", () => requestCounter++)
+
+  await page.click("#stream-link-method-inside-frame")
+  await nextBeat()
+
+  const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
+  await noNextEventNamed(page, "turbo:before-fetch-request")
+
+  assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method")
+  assert.equal(requestCounter, 1, "submits a single HTTP request")
+})
+
+test("test link method form submission targetting frame submits a single request", async ({ page }) => {
+  let requestCounter = 0
+  page.on("request", () => requestCounter++)
+
+  await page.click("#turbo-method-post-to-targeted-frame")
+  await nextBeat()
+
+  const { fetchOptions } = await nextEventNamed(page, "turbo:before-fetch-request")
+  await noNextEventNamed(page, "turbo:before-fetch-request")
+
+  assert.equal(fetchOptions.method, "POST", "[data-turbo-method] overrides the GET method")
+  assert.equal(requestCounter, 2, "submits a single HTTP request then follows a redirect")
+})
+
 test("test link method form submission inside frame", async ({ page }) => {
   await page.click("#link-method-inside-frame")
   await nextBeat()