hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.64k stars 421 forks source link

Reconsider adding `turbo:after-stream-render` event #1289

Open searls opened 1 month ago

searls commented 1 month ago

Sam (former maintainer) replied that this wasn't planned, but the linked discussion is no longer available https://github.com/hotwired/turbo/issues/92, so it's not clear what the rationale was.

As a few people have mentioned in the thread, many of us have found valid use cases for being able to hook into this event. It's often possible to get by manually with a MutationObserver, but that can be pretty clunky and in the case I'm dealing with now, also requires avoiding infinite recursion in cases where the purpose of the observer is to "clean up" any changes (for example moving which element has a client-side-managed active class after receiving server-side stream broadcast of a model update)

If maintainers are open to this, I'd be willing to take a stab at implementing

leonvogt commented 1 month ago

Would be really helpful to have an turbo:after-stream-render event and have a way to run JS after the stream has been rendered (without the use of Stimulus or MutationObservers).

Current use case: In the hotwire-dev-tools browser extension we log warnings if the stream targets are not found. This is done in the turbo:before-stream-render event.
Most of the times this works fine, but if a response contains multiple streams and the first stream creates a new DOM element which the second stream targets, we run into problems.
Because while evaluating the second stream, the target is not yet available in the DOM.
Would be handy to have a way to run some JS after the stream has been rendered.

8bit-mate commented 1 month ago

I guess that won't happen, and the following reason was proclaimed:

to encourage you to design your application not to care about where new HTML comes from or how it gets into the document

I'm using this custom event to handle it. Works in my app.

upd

but the linked discussion is no longer available

It is! Apparently the domain has changed. Just replace hotwire.dev with hotwired.dev in the original URL.

searls commented 1 month ago

@8bit-mate thanks for resurrecting that link for us.

While that original opinion is entirely valid, I think it's fair to say that there are imaginable scenarios where that a stream in particular was rendered has meaningful ramifications that wouldn't apply to other mutations (given how many examples were listed in #92 over the years).

It would seem ideologically consistent to include an after event for nearly every before. Especially given that Active Record, Action Pack, and other Rails-world libraries are chock full of granular before & after callbacks. Like Sam, I'd also encourage people to design their application to not care about the vast majority of those hooks either, but Rails is used for so many purposes that there are valid exceptions that justify their inclusion.