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

Ensure turbo-stream[action="remove"] does not render a view partial by default #680

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

seanpdoyle
Copy link
Contributor

Closes #679

Prior to this change, calls to
Turbo::Streams::Broadcasts#broadcast_remove_to would render a view partial that corresponds to the model regardless of whether or not the contents of that render would be included in the broadcast <turbo-stream> element.

Not only is it wasteful to render HTML and then do nothing with it, it also poses the risk of failing due to errors raised during the rendering process.

This commit restructures the contents of the
Turbo::Streams::Broadcasts#broadcast_action_to method to skip extraneous rendering if render: false is passed. In addition, it ensures that calls to broadcast_remove_to pass render: false by default.

Closes [hotwired#679][]

Prior to this change, calls to
`Turbo::Streams::Broadcasts#broadcast_remove_to` would render a
view partial that corresponds to the model *regardless* of whether or
not the contents of that render would be included in the broadcast
`<turbo-stream>` element.

Not only is it wasteful to render HTML and then do nothing with it, it
also poses the risk of failing due to errors raised during the rendering
process.

This commit restructures the contents of the
`Turbo::Streams::Broadcasts#broadcast_action_to` method to skip
extraneous rendering if `render: false` is passed. In addition, it
ensures that calls to `broadcast_remove_to` pass `render: false` by
default.

[hotwired#679]: hotwired#679
@seanpdoyle seanpdoyle changed the title Ensure turbo-stream[action="remove"] does not render Ensure turbo-stream[action="remove"] does not render a view partial Sep 18, 2024
@seanpdoyle seanpdoyle changed the title Ensure turbo-stream[action="remove"] does not render a view partial Ensure turbo-stream[action="remove"] does not render a view partial by default Sep 18, 2024
@seanpdoyle
Copy link
Contributor Author

@jorgemanrubia @brunoprietog are either of you available to review this?

Copy link
Contributor

@brunoprietog brunoprietog left a comment

Choose a reason for hiding this comment

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

Thanks @seanpdoyle!

@brunoprietog brunoprietog merged commit 9562a65 into hotwired:main Sep 18, 2024
15 checks passed
@seanpdoyle seanpdoyle deleted the issue-679 branch September 19, 2024 02:20
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.

v2.0.8 introduced issue where broadcast_remove_to requires render: false
2 participants