replace nextAnimationFrame by nextMicroTask #1042

michelson · 2023-10-20T02:04:47Z

DOM operations on turbostream are queued when the user is not in the browser tab or the tab is not active. This fixes that.
the problem resides when we call await nextAnimationFrame() because browsers pause the execution to save resources on the inactive browsers tab. The problem is that for chat applications or other kinds of notifications having a way to update things in the foreground is key.
more context at:
ref hotwired/turbo-rails#338

brunoprietog · 2023-10-20T07:54:01Z

This fixes #920.

Hopefully this will be taken into account in #1019, as mentioned by @seanpdoyle in this comment.

afcapel · 2023-10-24T12:29:59Z

This is subtly changing behavior because nextAnimationFrame and nextMicrotask have some other differences besides how the browser chooses to schedule them when the tab is not active. When you run something after requestAnimationFrame you are guaranteed that the event loop has run its UI rendering part, while putting a callback in the microtask queue executes it within the same event loop tick, before any UI changes have rendered.

To illustrate this point see this code:

<!DOCTYPE html>
<html lang="en">
<head>
  <link rel="icon" href="data:,">
  <style>
    #box {
      width: 100px;
      height: 100px;
      background-color: blue;
      transition: all 1s;
    }
  </style>
  <script type="text/javascript">
    function nextAnimationFrame() {
      return new Promise((resolve) => requestAnimationFrame(() => resolve()))
    }

    function nextEventLoopTick() {
      return new Promise((resolve) => setTimeout(() => resolve(), 0))
    }

    function nextMicrotask() {
      return Promise.resolve()
    }

    document.addEventListener("DOMContentLoaded", async () => {
      const box = document.getElementById("box")

      box.style.transform = "translateX(500px)"
      await nextAnimationFrame()
      box.style.transform = "translateX(0px)"
    })
  </script>
</head>
<body>
  <div id="box"></div>
</body>
</html>

This animates a box moving from right to left.

animation.mov

However replacing nextAnimationFrame with nextMicrotask breaks the animation. The JS code sets translateX(500px) and translateX(0px) before the UI updates render, and so the property doesn't change between rendering steps and there's no animation.

The current uses of nextAnimationFrame are intended to let the browser render UI updates before carrying on with further DOM transformations and I'm wary that changing them can introduce subtle bugs.

However, I'd be interested to see an easy way to reproduce nextAnimationFrame not triggering to see if we can solve the issue in some other way.

michelson · 2023-10-24T15:33:30Z

Hi @afcapel , Thanks for your answer. I've tried your example and put it in codesandbox.
https://codesandbox.io/s/jolly-kare-yq5mmd?file=/index.html , I've also replaced the nextMicroask to nextEventLoopTick in your example, and the animation is executed correctly.

I've implemented this in this PR by replacing the nextMicroTask with nextEventLoopTick.

Unit tests are passing. But note that I had to keep the nextMicroTask on the stream_element.js; not sure why it failed when used with the nextEventLoopTick.

afcapel · 2023-10-30T15:35:03Z

@michelson sorry, I'm a wary of changing this behaviour because I find the possible repercussions very difficult to foresee.

Turbo already works well simplifying most simple use cases, but this one is particular enough that you may have to reach to lower level APIs. I think you're best chance is to reach to ActionCable to deliver the new message notifications.

You can reuse the same WebSocket connection that Turbo opens with the server, and do anything you want when a new message arrives, even if your tab is in the background.

This is how we do it in in one of our apps:

// consumer.js

import { cable } from "@hotwired/turbo-rails"
export default await cable.getConsumer() // this reuses the same websocket that Turbo uses for stream updates

// typing_notifications_channel.js

import consumer from "channels/consumer"

// Subscribe to a custom channel
export function subscribeToTypingNotificationsChannel(roomId, callbacks) {
  const channel = { channel: "TypingNotificationsChannel", room_id: roomId }
  return consumer.subscriptions.create(channel, callbacks)
}

// typing_notifications_controller.js

import { Controller } from "@hotwired/stimulus"
import { subscribeToTypingNotificationsChannel } from "channels/typing_notifications_channel"
import { pageIsTurboPreview } from "helpers/turbo_helpers"

// Reacts to new messages on the `TypingNotificationsChannel`
export default class extends Controller {
  connect() {
    if (!pageIsTurboPreview()) {
      this.channel = subscribeToTypingNotificationsChannel(this.roomIdValue, { received: this.#received.bind(this) })
    }
  }

  ...

  #received(data) {
    ... // handle received data
  }
}

Hope that helps and you can migrate your app away from React.

michelson · 2023-10-30T20:03:07Z

This are certain use cases might be affected or limited by this approach:

Immediate Feedback: In scenarios where immediate feedback is crucial, even a slight delay introduced by nextAnimationFrame can be noticeable. For instance, real-time collaborative tools or games might need instantaneous reactions.
Rapid Successive Updates: If there are rapid successive updates to the DOM, nextAnimationFrame might cause them to be bundled together. While this is often an optimization, there are scenarios where you might want updates to render as they come in, rather than waiting.

Chat applications have unique challenges and requirements that might be influenced by the use of nextAnimationFrame for rendering updates. Here are some points specific to chat applications:

Instant Message Rendering: In real-time chat applications, messages from other users need to appear immediately once they're received. Any perceptible delay, even if it's minor, can detract from the "real-time" feel of the chat. nextAnimationFrame could potentially introduce such a delay.
Message Order: If multiple messages are received in quick succession, it's crucial that they render in the correct order. The delay introduced by nextAnimationFrame could potentially cause issues if messages are processed in rapid succession but render out of order.
Read Receipts: Many chat applications show when a message has been read by the other user. If the rendering of this status is delayed, it might confuse users or make the application feel less responsive.
Typing Indicators: "User is typing..." indicators need to be real-time to be effective. If there's a delay in showing or hiding these indicators due to nextAnimationFrame, it can lead to miscommunication or confusion.
Notifications and Alerts: For unread messages or mentions in a chat, instant notifications are crucial. Any delay in rendering these notifications could mean users might miss out on timely information or updates.

Considering these points, while nextAnimationFrame can optimize the rendering process for many web applications, chat applications have specific real-time needs that might be affected by such delays.

what if this could be an opt in feature? would you consider a new PR that implements that?

This is an idea to provide an optional configuration setting for Hotwire's Turbo to control whether it uses nextAnimationFrame for rendering updates. If the setting is enabled, Turbo would bypass nextAnimationFrame for more immediate rendering, especially useful for applications like real-time chats.

Here's a high-level overview of how we could implement this:

Configuration Setting:
Add a configuration option in Turbo to control the use of nextAnimationFrame.

Turbo.config = {
    ...,
    useNextAnimationFrame: true // default is true to maintain current behavior
};

Update Turbo's Rendering Logic:
In the part of the Turbo code that handles rendering, conditionally bypass nextAnimationFrame based on the configuration.

if (Turbo.config.useNextAnimationFrame) {
    requestAnimationFrame(() => {
        // current rendering logic
    });
} else {
    // immediate rendering logic
}

michelson · 2023-10-31T18:44:23Z

Doing more tests, I believe that the only change needed is in the render function of StreamElement, so I'm guessing if the <turbo-stream action=replace target=id><template> element could have some indicator to change how it handles its render, like an attribute like: <turbo-stream render-timing="event-loop"> is that something that you would accept as a PR, @afcapel ?

afcapel · 2023-11-01T10:20:31Z

Not interested in configuration options, but I'm open to a more surgical change. We can try limiting the change to the StreamElement render function and using a new timing function that only skips nextAnimationFrame if the tab is hidden:

// util.js

export function nextRepaint() {
  if (document.visibilityState === "hidden") {
    return nextEventLoopTick()
  } else {
    return nextAnimationFrame()
  }
}

michelson · 2023-11-01T14:39:24Z

Great solution, @afcapel . On it.

seanpdoyle · 2023-11-01T15:07:19Z

src/elements/stream_element.js

@@ -43,7 +43,7 @@ export class StreamElement extends HTMLElement {
      const event = this.beforeRenderEvent

      if (this.dispatchEvent(event)) {
-        await nextEventLoopTick()
+        nextRepaint()


Do we need to await this? Should nextRepaint() return the next-prefixed Promise utilities instead of await-ing them?

src/tests/unit/stream_element_tests.js

src/core/index.js

src/util.js

michelson · 2023-11-01T15:36:00Z

Hi @seanpdoyle, thanks for the suggestions. I've applied those already. Also, CI passing at https://github.com/michelson/turbo/actions/runs/6721616946/job/18267725557

afcapel · 2023-11-01T16:17:47Z

Thanks @michelson & @seanpdoyle!

replace nextAnimationFrame by nextMicroTask

ad14576

fix linter warnings

49d2e01

michelson added 2 commits October 24, 2023 10:58

use nextEventLoopTick

3cf31d7

use nextMicroTask in stream element

6259958

michelson mentioned this pull request Oct 24, 2023

How to run browser tests on local machine? #1045

Closed

event loop tick

52d41bf

afcapel closed this Oct 30, 2023

Merge branch 'hotwired:main' into main

8edb92e

michelson added 7 commits October 31, 2023 01:42

useNextAnimationFrame setting on session

b4b2bad

Merge branch 'main' of https://github.com/hotwired/turbo

1412c40

Merge branch 'main' of https://github.com/michelson/turbo

98efdaa

revert to initial

91b543c

conditioned nexTAnimationFrame

5a296ed

use nextEventTick on stream_message_renderer only

3c77b82

update stream element tests

223aade

afcapel reopened this Nov 1, 2023

michelson added 3 commits November 1, 2023 11:51

nextRepaint util function with browsert tab awareness

ac7f5a8

integrate nextRepaint into stream element test

3015eec

Merge remote-tracking branch 'origin/main' into main

68f194e

seanpdoyle reviewed Nov 1, 2023

View reviewed changes

src/tests/unit/stream_element_tests.js Outdated Show resolved Hide resolved

seanpdoyle reviewed Nov 1, 2023

View reviewed changes

src/core/index.js Outdated Show resolved Hide resolved

seanpdoyle reviewed Nov 1, 2023

View reviewed changes

src/util.js Outdated Show resolved Hide resolved

michelson added 3 commits November 1, 2023 12:12

await on nextRepaint on stream element

f12e3a3

restore session cache & nextRepaint returns sync

5469283

restore nextAnimationFrame on strea element test

d689a98

seanpdoyle approved these changes Nov 1, 2023

View reviewed changes

afcapel merged commit 3d48c86 into hotwired:main Nov 1, 2023
1 check passed

brunoprietog mentioned this pull request Nov 1, 2023

Replace call to requestAnimationFrame in render function to update page content even if tab has lost focus #920

Closed

brunoprietog mentioned this pull request Dec 6, 2023

Page refreshes: Replace remaining calls to nextAnimationFrame by nextRepaint #1102

Merged

YoutacRandS-VA mentioned this pull request Jun 19, 2024

[Snyk] Upgrade @hotwired/turbo from 7.3.0 to 8.0.4 YoutacRandS-VA/osv.dev#4

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

replace nextAnimationFrame by nextMicroTask #1042

replace nextAnimationFrame by nextMicroTask #1042

michelson commented Oct 20, 2023 •

edited

Loading

brunoprietog commented Oct 20, 2023

afcapel commented Oct 24, 2023

michelson commented Oct 24, 2023 •

edited

Loading

afcapel commented Oct 30, 2023 •

edited

Loading

michelson commented Oct 30, 2023 •

edited

Loading

michelson commented Oct 31, 2023

afcapel commented Nov 1, 2023

michelson commented Nov 1, 2023

seanpdoyle Nov 1, 2023

michelson commented Nov 1, 2023

afcapel commented Nov 1, 2023

replace nextAnimationFrame by nextMicroTask #1042

replace nextAnimationFrame by nextMicroTask #1042

Conversation

michelson commented Oct 20, 2023 • edited Loading

brunoprietog commented Oct 20, 2023

afcapel commented Oct 24, 2023

michelson commented Oct 24, 2023 • edited Loading

afcapel commented Oct 30, 2023 • edited Loading

michelson commented Oct 30, 2023 • edited Loading

michelson commented Oct 31, 2023

afcapel commented Nov 1, 2023

michelson commented Nov 1, 2023

seanpdoyle Nov 1, 2023

Choose a reason for hiding this comment

michelson commented Nov 1, 2023

afcapel commented Nov 1, 2023

michelson commented Oct 20, 2023 •

edited

Loading

michelson commented Oct 24, 2023 •

edited

Loading

afcapel commented Oct 30, 2023 •

edited

Loading

michelson commented Oct 30, 2023 •

edited

Loading