hotwire-django / turbo-django

Unmaintained // An early stage integration of Hotwire Turbo with Django
https://discuss.hotwire.dev/t/django-backend-support-for-hotwire/1570/
Other
385 stars 20 forks source link

How to decide what to implement? #4

Closed davish closed 3 years ago

davish commented 3 years ago

I'm wondering what a good way to move forward is. Should we add utilities as people's projects seem to need them? Should we look at what's offered in hotwired/turbo-rails right now and port it over to Python? The second seems to make sense to me, but I personally don't have any experience with Ruby and have had trouble in the past reading through Rails codebases. If anyone else has experience in that area then I'd love to work with them!

blopker commented 3 years ago

Hey @davish, it looks like some great work has already been done here! I can see this project being very popular with Django devs. I think your proposal of trying to stick as close as possible to the Rails project is spot on. However, as you mentioned there's some significant differences between Rails and Django that will need to be addressed.

Specifically, it seems to me Rails has more mature Websocket support that enables easier integration with Turbo Streams. However, I'm not sure it really matters that much. I've worked with Django for over 7 years now and realtime updating features tend to be pretty rare and will generally require custom JS anyway.

For my projects even just having solid Drive and Frame integration would be huge.

I'm wondering if this project could ship a v1 without Stream support at first? This would allow us to focus on a great Drive and Frame experience and get feedback on that before adding Stream.

I've added a proposal for a turbo_frame tag here (https://github.com/hotwire-django/turbo-django/issues/5) to get started.

Cheers!

davish commented 3 years ago

Hey @blopker!

I agree with you that probably the most important thing is to get good helpers for turbo frames, since that's the most immediately applicable part of Turbo (along with Drive, which I don't think requires much special configuration) to projects. I do think that using turbo streams over HTTP (submit a form to create an item, the form responds with a stream which appends the new item to an existing list) is also a pattern that we should readily support from the get-go.

With regards to websockets, I agree that it's more niche, but the turbo package in the repo actually already contains a prototype Streams integration working over websockets. It definitely had a bit more custom setup required compared to Rails, since ActionCable seems to be considerably higher-level than Django Channels, but I do think we can get there.

I'll take a look at your turbo_frame proposal in #5! Thanks so much for putting it together.

blopker commented 3 years ago

Great! Yes, I didn't mean to imply that we shouldn't have Stream support or that what we currently have is not good. I got the current support working in the example app and it's super cool. I'm just thinking that a) Frames are going to be used the most and b) Stream support may take a bit to get right and we could release without it so it's not blocking.

My other thought is that Stream support could be optional. This is so that people not using ASGI, like many older apps and apps that just don't need async, could still use the other Turbo support. I'd think the only effort here would be to make sure this package still works without channels installed.

davish commented 3 years ago

That all makes sense! It seems like we're in agreement :)

danjac commented 3 years ago

A few thoughts after spending a few days converting some fairly big projects over to Hotwire:

  1. You don't need to use streams with ASGI. In fact the most common use was with form validation: the most common paradigm with traditional server-side Django is "post a form, re-render HTML page with validation errors, if ok then redirect". You can't actually do this with Turbo: if you try to do anything other than return a redirect, it throws an error. The only way to handle "traditional" server side validation is to wrap the returned HTML in a stream and insert/update the relevant element with the matching ID. In fact I needed to do subclass a whole lot of allauth views to be able to do this (a big omission in the Turbo beta, hopefully to be addressed ASAP, is that you can't use data-turbo="false" on forms, which means there's no escape hatch and all your legacy forms will break). There's no magic here: just return an HttpResponse that wraps your HTML in turbo-stream tags in a plain Django view.
  2. You don't even need ASGI to do partial updates: you just use a StreamingHttpResponse. Here's the example I pasted on the forum:

    def update_cart(request):
    # handle update in session / DB and recalculate "total_amount"...
    
    def render():
    for target in ["total-cart-nav", "total-cart-top", "total-cart-bottom"]:
      yield f'<turbo-stream id="{target}" action="update">{total_amount}</turbo-stream>'
    
    # remember the correct content type!
    return StreamingHttpResponse(render(), content_type="text/html; turbo-stream;")

    I've not looked at lazy loading yet; at first glance this should just work with plain responses, but it might not. Still, you don't need to set up channels for common use cases.

  3. I found frames to be fine for very simple AJAX updates but actually less useful than streams. They are limited somewhat by the lack of a lower-level API; there is no equivalent to visit() where you can fetch a stream or frame and have it just work (you can certainly fetch the response but Turbo will ignore it). Hopefully this will be addressed (depending on the Hotwire/Basecamp team's priorities), but right now if you want to do anything slightly more complicated with frames than "click/submit, replace this bit of HTML" you're going to have to drop down to Stimulus/fetch.
  4. I played around with template tags, but found them to be of very limited benefit. Ultimately most of the work was building response classes that wrapped the necessary bits of markup for streams and frames. This is what I have so far: https://github.com/danjac/radiofeed/tree/master/radiofeed/common/turbo. There's a middleware which just checks the "Accept" header for cases where I need to return either a full page/redirect or frame.
  5. I rewrote a simple Slack clone with channels and streams: https://github.com/danjac/chatter/. This was quite straightforward, again just wrapping the responses in turbo-stream syntax and wiring up a WebSocket in the controller with Turbo. connectStreamSource.

tl;dr; most of the work was in writing helpers/response classes for views, template tags didn't do much, streams are more useful and used than frames, and Hotwire/Turbo is still very early beta and there's quite a few missing features and lack of docs/examples that hopefully will be addressed going forward.

blopker commented 3 years ago

Good stuff! Regarding 1, I just tried to return a form with errors in a Turbo Frame and the errors shows as expected. Maybe this only works when the form is in a Frame? It's nice that Streams are still useful outside of ASGI though. Would be nice to focus on the non-ASGI stuff first.

For 2, I haven't tested this, but from the Turbo documentation I'd assume just rendering all those turbo-stream tags in a normal response (not a streaming response) would work as well? I only ask since streaming responses are harder to gzip.

danjac commented 3 years ago
  1. You return a stream not a frame.
  2. Yes. A normal HttpResponse is fine.

Here's a concrete example (in production, you can go to https://audiotrails.io/account/signup/ and try it out):

  1. See https://github.com/danjac/radiofeed/blob/master/templates/account/_signup.html. This is a plain Django form (I use widget_tweaks but doesn't matter, {{ form_as_p }} or whatever is fine). Notice the "id" in the form. Note that the top level <form>tag must be self contained (like a React or Vue component); you can't have two top level tags in a turbo-stream response. It's included in https://github.com/danjac/radiofeed/blob/master/templates/account/signup.html. This is a plain simple Django include: we don't need any additional tags beyond what Django provides.
  2. This is subclassed from allauth SignupView: https://github.com/danjac/radiofeed/blob/master/radiofeed/users/views/account.py. It uses a form mixin https://github.com/danjac/radiofeed/blob/master/radiofeed/common/turbo/mixins.py. This mixin just returns a stream response if the form is invalid. The mixin does a "replace" action by default, you can use "update" for example if you want to insert the form into a specific place instead.
  3. The response classes are here: https://github.com/danjac/radiofeed/blob/master/radiofeed/common/turbo/response.py. As you can see, the class is just a subclass of plain TemplateResponse, that wraps the output in the turbo-stream tags and sets the correct content type. That's it: nothing magic needed.
davish commented 3 years ago

With regards to the issue @danjac's running into around Turbo Frames and form responses, I also agree with @blopker that this works for me without issue. I replaced the default ModelForm generated by CreateView in the chat example in this repository with this form:

class MessageForm(ModelForm):
    class Meta:
        model = Message
        fields = ["text", "confirm"]

    def clean_confirm(self):
        if not self.cleaned_data["confirm"]:
            raise ValidationError(["You must confirm message"], code="invlaid")
        return self.cleaned_data["confirm"]

Along with adding confirm = models.BooleanField() to the Message model and generating a migration. The frame properly replaced the content of the send-message frame with the server-side error:

image

The issue might have been that the form template did not include the turbo-frame with a matching ID to the frame on your page, so the response generated by Django and sent to the frontend wasn't recognized by Turbo as the replacement content. You can see in the message_form.html template in the demo that the partial contains the turbo-frame with the same ID as appears on the room page itself.

I need to do some more investigation here, but I'd guess that you can return a response that contains both a Stream update along with a new Frame to reset the form after submission

danjac commented 3 years ago

With regards to the issue @danjac's running into around Turbo Frames and form responses

What issue is this? I just mentioned that the right approach is to return a stream rather than a frame with a form response. This is the right practice as per the Turbo docs. The problem is when you want to trigger a frame/stream from inside JS (e.g. from a fetch()) where you need to do something slightly more complicated than a simple form submit/link.

davish commented 3 years ago

Closing this issue, since discussion has moved to Slack (join here for anyone reading this!)