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

broadcast_remove_later_to doesn't exist 🤔 #506

Closed
jon-sully opened this issue Oct 23, 2023 · 2 comments
Closed

broadcast_remove_later_to doesn't exist 🤔 #506

jon-sully opened this issue Oct 23, 2023 · 2 comments

Comments

@jon-sully
Copy link

I realize that the typical / default usage of broadcasting remove is that a record has been destroyed and its partial should be removed from users' UIs, and that trying to run a _later version of that would fail since the background job would then try to lookup a record that doesn't exist anymore, blow up, and nobody would get the UI update. That makes sense. But doesn't that couple model state to broadcast action?

I have cases where I definitely want to broadcast_remove_later_to even though the model still persists (tasks assigned to specific users that get reassigned to other users) but that method doesn't exist. To that end, I understand why the defaults work this way — that broadcasts and broadcasts_to use a synchronous remove instead of _later. It seems reasonable that the defaults should couple with default model lifecycles (and hence they're run after_destroy_commit), I think I'm just advocating that broadcast_remove_later_to should exist even if it's not part of the defaults 😛

Happy to PR this if that's a reasonable API request 👍

@afcapel
Copy link
Contributor

afcapel commented Oct 23, 2023

@jon-sully the broadcast_*_later actions exist so you don't slow down an HTTP request processing a turbo update that may need expensive HTML rendering. In the case of remove there's no such risk because you only need the element id to render a remove stream action and the rendering time is not a concern in those cases.

See this comment for details.

@jon-sully
Copy link
Author

Oof 🤦‍♂️ not just a very cogent point but one that's already documented in a comment! Big derp.

That makes great sense. Thank you for the pointer. I'll close this out and hopefully future searchers can find the same! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants