hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.73k stars 429 forks source link

data-turbo-method ignored when data-turbo-frame is set #587

Closed botandrose closed 2 years ago

botandrose commented 2 years ago

Hi folks, thanks for Turbo! I'm feeling the joy of server-side webdev like its 2005 again.

However, I've hit a snag with using data-turbo-method and data-turbo-frame at the same time:

Here is a JSFiddle demonstrating the issue:

https://jsfiddle.net/6kvyxLmf/

As you can see, when data-turbo-frame is set, all the fetch methods are made with GET, not the method specified in data-turbo-method. Shall I proceed with a PR?

botandrose commented 2 years ago

It looks like this ALMOST works... the data-turbo-method machinery (convertLinkWithMethodClickToFormSubmission by way of followedLinkToLocation) gets prevented at the very last moment when willFollowLinkToLocation returns false because applicationAllowsFollowingLinkToLocation returns false. Here are the relevant bits: https://github.com/hotwired/turbo/blob/e2d53054438fa16b4babf6a7afeba7a828764772/src/observers/link_click_observer.ts#L41-L44 https://github.com/hotwired/turbo/blob/e2d53054438fa16b4babf6a7afeba7a828764772/src/core/session.ts#L157-L160 https://github.com/hotwired/turbo/blob/e2d53054438fa16b4babf6a7afeba7a828764772/src/core/session.ts#L149-L155 https://github.com/hotwired/turbo/blob/e2d53054438fa16b4babf6a7afeba7a828764772/src/core/session.ts#L291-L294

dhh commented 2 years ago

cc @seanpdoyle

seanpdoyle commented 2 years ago

@botandrose was this resolved by https://github.com/hotwired/turbo/pull/631#issuecomment-1186256417? This test case in particular seems to reproduce your issue:

https://github.com/hotwired/turbo/blob/f8a94c5f761c4f080d68a87459d3916eb4fb6993/src/tests/fixtures/form.html#L302

https://github.com/hotwired/turbo/blob/f8a94c5f761c4f080d68a87459d3916eb4fb6993/src/tests/functional/form_submission_tests.ts#L901-L907

botandrose commented 2 years ago

@seanpdoyle Confirmed! I just switched from my hacked fork to 7.2.0-beta.2, and everything works!

Thanks again for your efforts here, and on Turbo in general.Turbo and import maps reaffirm that my job doesn't have to be Node and Webpack and React, so that means I'll continue to enjoy it :)