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

Allow for broadcast methods to accept a renderable #364

Merged
merged 6 commits into from Jun 20, 2023

Conversation

jklina
Copy link
Contributor

@jklina jklina commented Jul 23, 2022

Rails supports rendering of objects such as ViewComponents, but this support doesn't seem to be available in Turbo. This is just a stake in the sand for exploring the option to render a renderable in addition to html and partial.

This should allow for the following API call:

message.broadcast_update_to(:messages, target: "message-count",
  renderable: MessagesCountComponent.new(count: Message.count))

where MessagesCountComponent is a supported Rails rendering object such as a ViewComponent.

It turns out that Rails does accept a long form argument for rendering objects: renderable. The only gotcha is that it renders the layout by default, causing the layout to be duplicated. This change explicitly prevents the layout from being rendered in the call to ApplicationController.render to prevent that duplication from happening.

This is a follow up to: #358

@jklina jklina marked this pull request as ready for review July 23, 2022 01:13
@swanson
Copy link
Contributor

swanson commented Jun 14, 2023

I am currently using the workaround of

message.broadcast_update_to(:messages, target: "message-count",
  renderable: MessagesCountComponent.new(count: Message.count)
  layout: false
)

I added a PR to improve docs for renderable as well: rails/rails#48467

@dhh
Copy link
Member

dhh commented Jun 18, 2023

Needs a rebase. Happy to see this changed.

Joshua Klina and others added 2 commits June 19, 2023 11:55
Rails supports [rendering of objects](https://edgeguides.rubyonrails.org/layouts_and_rendering.html#rendering-objects) such as [ViewComponents](https://github.com/github/view_component), but this support doesn't seem to be available in Turbo. This is just a stake in the sand for exploring the option to render a `renderable` in addition to `html` and `partial`.

This should allow for the following API call:

```ruby
message.broadcast_update_to(:messages, target: "message-count",
  renderable: MessagesCountComponent.new(count: Message.count))
```

where `MessagesCountComponent` is a supported Rails rendering object such as a ViewComponent.

It turns out that Rails does accept a long form argument for rendering objects: `renderable`. The only gotcha is that it [renders the layout by default](rails/rails#39869), causing the layout to be duplicated. This change explicitly prevents the layout from being rendered in the call to `ApplicationController.render` to prevent that duplication from happening.
@jklina jklina force-pushed the add-support-for-renderable-rendering branch from 24de801 to 432b915 Compare June 19, 2023 16:00
@jklina
Copy link
Contributor Author

jklina commented Jun 19, 2023

Should be caught up, thanks!

@@ -83,6 +83,6 @@ def broadcast_stream_to(*streamables, content:)

private
def render_format(format, **rendering)
ApplicationController.render(formats: [ format ], **rendering)
ApplicationController.render(layout: false, formats: [ format ], **rendering)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed if we also set o[:layout] = false in broadcastable#368?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. It doesn't seem like its needed so I nixed it:

58f8427

This isn't necessary since its set within:
app/models/concerns/turbo/broadcastable.rb
@dhh dhh merged commit 865377e into hotwired:main Jun 20, 2023
11 checks passed
@davidalejandroaguilar
Copy link
Contributor

davidalejandroaguilar commented Sep 13, 2023

Might be my setup but this fails when used with the _later version of the Broadcastable methods, right? e.g.

order.broadcast_update_later(
  target: "order_items", 
  renderable: FrontEnd::OrderItemsComponent.new(order:)
)

[ActiveJob] Failed enqueuing Turbo::Streams::ActionBroadcastJob to Sidekiq(default): ActiveJob::SerializationError (Unsupported argument type: FrontEnd::OrderItemsComponent)

I'm using Phlex and ActiveJob with Sidekiq and Sidekiq.strict_args!. However, it looks like ActiveJob is the one complaining, not Sidekiq.

We might need a follow-up PR to render the renderable before enqueueing it so that only a string is serialized? I can take a look later.

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.

None yet

4 participants