rails / request.js

MIT License
389 stars 28 forks source link

Waiting for turbo stream actions to finish executing #35

Closed janko closed 2 years ago

janko commented 2 years ago

We have the following desired scenario:

  1. user clicks a button
  2. we turn on loading animation
  3. send a turbo stream request via request.js
  4. remove loading animation when turbo stream actions are executed

The problem is, I couldn't figure out how to wait for turbo stream operations to finish. The most we can do is have an await response.text, which if I understand correctly waits for the response body to be downloaded. But afterwards there are also DOM operations being executed as part of the turbo stream response, and that can take a long time in our case, as we have turbo stream commands with large chunks of HTML.

The response.renderTurboStream() function which request.js runs does seem to finish after turbo stream DOM operations have finished. However, since request.js executes it internally and doesn't expose its promise, there doesn't seem to be a way to wait for it.

One solution might be to assign the promise returned by response.renderTurboStream() to the response object, and provide a response getter, allowing users to wait for it.

marcelolx commented 2 years ago

Couldn't we just await the response in request.js? I mean, put an await in this line https://github.com/rails/request.js/blob/6e32e86fa098f9dbe7041cf1c598da9dcbeb6b78/src/fetch_response.js#L67 and another one here https://github.com/rails/request.js/blob/main/src/fetch_request.js#L29 and just then return the response? @excid3 since you added this functionality, can you see a problem with us doing this? Which scenario wouldn't we expect the turbo stream to be processed before receiving the response object?

excid3 commented 2 years ago

Adding an await here would wait until the elements were added to the DOM only (before they're applied). This would help somewhat at least.

Since Turbo Streams are customElements, there's no way to know when they're finished executing unless we tracked them being removed from the DOM too. I'd consider that out of scope for this library.

I imagine waiting until they're inserted into the DOM will be as close as we can get.

If it still runs too fast with the await, you may need a timeout of 100ms or something afterwards the request returns.

@janko give #36 a try and see if that helps?

excid3 commented 2 years ago

An alternative solution here would be having a turbo stream action that removes the loading animation. That wouldn't require any waiting in request.js and would run at the correct time regardless.

Turbo streams aren't the most surgical, so it would have to be an element that you add / remove from the page probably.

inspire22 commented 1 year ago

This has been causing me a lot of headaches to update any timestamps included in the turbo stream response into a relative "5 mins ago" style note. setTimeout fixes it but it's a pretty big hack. It also seems lacking that the turbo system doesn't have any callbacks associated with the end of the execution, but that's something to complain to them about.

marcelolx commented 1 year ago

@inspire22 not sure how this is causing problems. It is also curious that you need to update timestamps included in a turbo response.

inspire22 commented 1 year ago

In a long-running chat app for example, you can't just render "5 minutes ago" from rails, since that'll be inaccurate after 1 minute. I've gotten a working rendering a function call in the turbo response, but that's pretty ugly. Turbo really should provide a promise/callback for completed rendering.

excid3 commented 1 year ago

The local_time gem/js library is what you'd want for keeping timestamps up-to-date.