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.

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

Document data-turbo-stream attribute #103

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Document data-turbo-stream attribute #103

merged 1 commit into from
Sep 22, 2022

Conversation

kevinmcconnell
Copy link
Contributor

Documents the data-turbo-stream attribute introduced in hotwired/turbo#612.

@netlify
Copy link

netlify bot commented Jul 15, 2022

Deploy Preview for epic-fermat-9d85e9 ready!

Name Link
🔨 Latest commit bb08da7
🔍 Latest deploy log https://app.netlify.com/sites/epic-fermat-9d85e9/deploys/62d516a802d4760009dd96e1
😎 Deploy Preview https://deploy-preview-103--epic-fermat-9d85e9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -19,6 +19,7 @@ The following data attributes can be applied to elements to customize Turbo's be
* `data-turbo-cache="false"` removes the element before the document is cached, preventing it from reappearing on restoration Visits.
* `data-turbo-eval="false"` prevents inline `script` elements from being re-evaluated on Visits.
* `data-turbo-method` changes the link request type from the default `GET`. Ideally, non-`GET` requests should be triggered with forms, but `data-turbo-method` might be useful where a form is not possible.
* `data-turbo-stream="true"` specifies that a link or form should request a Turbo Streams response. Turbo [automatically requests stream responses](/handbook/streams#streaming-from-http-responses) for form submissions with non-`GET` methods; `data-turbo-stream` allows Turbo Streams to be used with `GET` requests as well.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that wasn't clear to me in hotwired/turbo#612... does it still also accept a HTML response? If yes, maybe this could be more clear:

Suggested change
* `data-turbo-stream="true"` specifies that a link or form should request a Turbo Streams response. Turbo [automatically requests stream responses](/handbook/streams#streaming-from-http-responses) for form submissions with non-`GET` methods; `data-turbo-stream` allows Turbo Streams to be used with `GET` requests as well.
* `data-turbo-stream="true"` specifies that a link or form can also accept a Turbo Streams response. Turbo [automatically requests stream responses](/handbook/streams#streaming-from-http-responses) for form submissions with non-`GET` methods; `data-turbo-stream` allows Turbo Streams to be used with `GET` requests as well. If the server returns a HTML response then navigation proceeds as normal; if the server returns a Turbo Stream response then it is processed as a Turbo Stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ghiculescu thanks, that's a good point. I think your wording of "can accept" makes this clearer. I've updated the description to use that.

I've also added a note in the Handbook section that it references to explain in a bit more detail, which also should make this option easier to discover.

What do you think?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinmcconnell thanks a ton for this. It's extremely useful.

Just a bit confused by why an HTML request gets sent after the turbo stream request. Is there a way to prevent that if all we want is a turbo stream response?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jusko there should still only be one request. Adding data-turbo-stream means that request can accept either a stream response, or a regular HTML response.

Copy link

@jusko jusko Jul 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Forgive my ignorance @kevinmcconnell... I only glanced at the turbo code quickly, but using it in my client code produces the following:

image

The last two roundtrips relate to:
2. the turbo stream response after clicking a link with data-turbo-stream (working as expected)
3. a 406 response to a separate request (sent with Content-Type: text/html in the headers) also sent when clicking the link (the controller only responds to the turbo stream format).

I understand this isn't really the place to discuss this, but on the off chance anyone else experiences this, hopefully it's worth it. I will look into it more when there's time and update.

Thanks again for the new functionality. Extremely useful!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jusko oh interesting! That definitely sounds like a bug. I hadn’t seen that myself but I will also try to look into it when I get a chance. The intended behaviour is that a single request is sent. Thanks for bringing this up!

I wonder if this is related to the issue that’s being addressed by hotwired/turbo#645.

Also worth mentioning, we’ve discussed changing how data-turbo-stream works under the hood (see hotwired/turbo#641 (comment)), and I believe @seanpdoyle is planning to open a PR for that change. So once that’s ready it could be worth testing against that version as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be related to hotwired/turbo#645.

Documents the `data-turbo-stream` attribute introduced in
hotwired/turbo#612.
@dhh dhh merged commit 9adc47d into hotwired:main Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants