Skip to content

Commit

Permalink
Prevent double requests from within turbo-frame
Browse files Browse the repository at this point in the history
Closes hotwired#743

The change in behavior can be traced back to [hotwired#412][].
When the overlap between the `LinkInterceptor` and the
`LinkClickObserver` were unified, the technique used by the
`LinkInterceptor` to prevent duplicate event handlers from responding to
the same `click` event was changed in a subtle way.

In its place, this commit changes the `LinkClickObserver` to monopolize
its handling of `click` events on `<a>` elements that navigate the page
or drive `<turbo-frame>` elements. In tandem with the existing call to
[Event.preventDefault][], this commit adds a call to
[Event.stopImmediatePropagation][].

To exercise this edge case, this commit also adds a `ujs.html` fixture
file along with a `ujs_tests.ts` module to cover situations when
`@rails/ujs` and `@hotwired/turbo` co-exist.

[hotwired#412]: hotwired#412
[Event.preventDefault]: https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault
[Event.stopImmediatePropagation]: https://developer.mozilla.org/en-US/docs/Web/API/Event/stopImmediatePropagation
  • Loading branch information
seanpdoyle committed Sep 28, 2022
1 parent 732db00 commit 1213774
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/observers/link_click_observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export class LinkClickObserver {
const location = this.getLocationForLink(link)
if (this.delegate.willFollowLinkToLocation(link, location, event)) {
event.preventDefault()
event.stopImmediatePropagation()
this.delegate.followedLinkToLocation(link, location)
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/tests/fixtures/ujs.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Frame</title>
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
<script src="/src/tests/fixtures/test.js"></script>
<script type="module">
import Rails from "https://ga.jspm.io/npm:@rails/[email protected]/lib/assets/compiled/rails-ujs.js"

Rails.start()
</script>
</head>
<body>
<turbo-frame id="frame">
<h2>Frames: #frame</h2>

<a data-remote="true" href="/src/tests/fixtures/frames/frame.html">navigate #frame to /src/tests/fixtures/frames/frame.html</a>
</turbo-frame>
</body>
</html>
17 changes: 17 additions & 0 deletions src/tests/functional/ujs_tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { test } from "@playwright/test"
import { assert } from "chai"
import { nextEventOnTarget } from "../helpers/page"

test.beforeEach(async ({ page }) => {
await page.goto("/src/tests/fixtures/ujs.html")
})

test("test clicking a [data-remote=true] anchor within a turbo-frame", async ({ page }) => {
let requestsStarted = 0
await page.on("request", () => requestsStarted++)
await page.click("#frame a[data-remote=true]")
await nextEventOnTarget(page, "frame", "turbo:frame-load")

assert.equal(await page.textContent("#frame h2"), "Frame: Loaded", "navigates the target frame")
assert.equal(requestsStarted, 1, "only submits a single request")
})

0 comments on commit 1213774

Please sign in to comment.