Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

7.2 data-remote inside turbo-frame produces double requests #743

Closed
acetinick opened this issue Sep 28, 2022 · 1 comment · Fixed by #744
Closed

7.2 data-remote inside turbo-frame produces double requests #743

acetinick opened this issue Sep 28, 2022 · 1 comment · Fixed by #744

Comments

@acetinick
Copy link

acetinick commented Sep 28, 2022 •
edited
Loading

Since upgrading to turbo 7.2 which we use extensively throughout our project, we have some introduced issues when we have some legacy data-remote="true" UJS being triggered within a turbo-frame.

When we have a situation where we have a data-remote inside a turbo-frame, things break down. Example:

<turbo-frame id="example">
   <a data-remote="true" href="/customers/new">click</a>
</turbo-frame>

What ends up happening is that the remote gets triggered, but for some reason a double request occurs issueing the turbo as well along with the UJS call to the URL. Attached example from console (from a real situation not above example)

image

  • the 406 is the turbo request, which is NOT expected resulting in an UnkownFormat exception which trying to render a .html format, but we only have a .js.erb view
  • the 200 is the UJS request, which is expected to return JS to open a modal window

I can conifrm that when we downgrade to 7.1 everything works fine, but as soon as we upgrade issue is reproduced. VERY strange tho, for some reason it only happens in non-local environments. Locally it works, but in staging it breaks.

It also doesn't seem to matter if its data-method="post" or data-method="get", it occurs always with data-remote when inside a turbo-frame.

My gut feel it must be related to #612 or #645, which is a huge benefit we already making use of this, but somehow its conflicting with data-remote somehow. @kevinmcconnell @seanpdoyle perhaps you got some ideas?
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Sep 28, 2022
@seanpdoyle
Prevent double requests from within turbo-frame
1213774 
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
@seanpdoyle seanpdoyle mentioned this issue Sep 28, 2022
Merged
@seanpdoyle
Copy link
Contributor

Thank you for opening this issue @acetinick. I've opened #744 to try and resolve it.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Sep 30, 2022
@seanpdoyle
Prevent double requests from within turbo-frame
3db3efd 
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` and
`FormSubmitObserver` to ignore `<a>` clicks and `<form>` submissions if
they're annotated with `[data-remote="true"]`.

To exercise these edge cases, 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
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Sep 30, 2022
@seanpdoyle
Prevent double requests from within turbo-frame
5871173 
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` and
`FormSubmitObserver` to ignore `<a>` clicks and `<form>` submissions if
they're annotated with `[data-remote="true"]`.

To exercise these edge cases, 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
@dhh dhh closed this as completed in #744 Oct 8, 2022
dhh pushed a commit that referenced this issue Oct 8, 2022
@seanpdoyle
Ignore UJS <a> clicks and <form> submissions (#744)
2345af9 
* Prevent double requests from within `turbo-frame`

Closes #743

The change in behavior can be traced back to [#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` and
`FormSubmitObserver` to ignore `<a>` clicks and `<form>` submissions if
they're annotated with `[data-remote="true"]`.

To exercise these edge cases, 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.

[#412]: #412

* Revert "Replace LinkInterceptor with LinkClickObserver"

This reverts commit 9bef653.

* adjust after reverting #412

* Restore `<form data-remote="true">`+`<turbo-frame>` behavior from 7.1.0

[comment]: #744 (comment)
[FormInterceptor]: https://github.com/hotwired/turbo/blob/v7.1.0/src/core/frames/form_interceptor.ts#L31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ignore UJS <a> clicks and <form> submissions seanpdoyle/turbo
2 participants
@acetinick @seanpdoyle