Fall back to 'setTimeout' when 'requestAnimationFrame' is not called #13091
Conversation
**what is the change?:** Just adding a test to the fixture, where we can easily see whether scheduled callbacks are called after switching away from the fixture tab. **why make this change?:** We are about to fix the schedule module so that it still runs even when the tab is in the backround. **test plan:** Manually tested the fixture, verified that it works as expected and right now callbacks are not called when the tab is in the background. **issue:** Internal task T30754186
**what is the change?:** If 'requestAnimationFrame' is not called for 100ms we fall back to 'setTimeout' to schedule the postmessage. **why make this change?:** When you start loading a page, and then switch tabs, 'requestAnimationFrame' is throttled or not called until you come back to that tab. That means React's rendering, any any other scheduled work, are paused. Users expect the page to continue loading, and rendering is part of the page load in a React app. So we need to continue calling callbacks. **test plan:** Manually tested using the new fixture test, observed that the callbacks were called while switched to another tab. They were called more slowly, but that seems like a reasonable thing. **issue:** Internal task T30754186
|
ReactDOM: size: 🔺+0.1%, gzip: 🔺+0.1% Details of bundled changes.Comparing: da5c87b...d9f6f42 react-dom
react-art
react-scheduler
Generated by 🚫 dangerJS |
There was a problem hiding this comment.
Feels wasteful to always do this but I can't figure out what a better solution would be if we want to stay responsive.
Maybe we could use visibilitychange event to detect backgrounding and toggle the "aggressive" mode, but avoid extra timeouts when we're visible?
Accept to unblock but would be nice to investigate the above solution
| let timeoutID; | ||
| const scheduleAnimationFrameWithFallbackSupport = function(callback) { | ||
| // schedule rAF and also a setTimeout | ||
| rafID = localRequestAnimationFrame(function(...args) { |
There was a problem hiding this comment.
Nit: We expect a single argument, let's keep that explicit? Even if we changed to many, the other if branch already assumes it's just one, so might as well do here for clarity
|
Here's what velocity (a super popular animation library) does: and Kind of fun but likely overkill for us. |
|
That is a fascinating solution - great find! I think it's worth exploring, but will land this first in order to unblock moving forward with further async rendering experiments. My only concerns would be
|
| const counterNode = document.getElementById('test-7'); | ||
| counter++; | ||
| counterNode.innerHTML = counter; | ||
| waitForTimeToPass(100); |
There was a problem hiding this comment.
Why 100? Sorry. I'll keep reading :)
There was a problem hiding this comment.
Good question~! It's arbitrary - seemed like enough time to have lots of numbers increment, but not too fast to see.
There was a problem hiding this comment.
Cool. I didn't know about the scheduling fixtures! This is really neat!
Yes, and it matches what some comparable code in Facebook internals was doing. |
|
Also I'm observing that, in Firefox at least, even The solution @gaearon found could work better, would be nice to continue firing at 60fps when backgrounded if possible. |
|
This fix seems wrong to me. What are the circumstances under which |
"schedule" is not an animation library. 60fps would be overkill. My proposed alternative fix would be to |
This is purely to fix the problem of when the tab is backgrounded.
Is the concern that it's too heavy to schedule a timeout so often? Wouldn't we still do that even if the timeout was 100ms? |
|
Btw browsers throttle |
|
Totally skimmed over the part about tab switching, my bad. Shared my other concerns in person but I'm fine with this as a temporary solution. Using the |
|
My biggest concern is that you switch tabs, the callback times out, and all the async work of the background tab is synchronously flushed without yielding, which is what I suspect might be happening in the current incarnation. Need more tests. Let's follow up before open sourcing this. |
|
Hi, Sorry for burgling in on this conversation. These situations are likely to happen in safari & edge as stated by @jakearchibald in this issue . |
Will you be the hero to review this PR?
Would be great to get the fix landed in Monday's sync. 😁
what is the change?:
If 'requestAnimationFrame' is not called for 100ms we fall back to
'setTimeout' to schedule the postmessage.
why make this change?:
When you start loading a page, and then switch tabs,
'requestAnimationFrame' is throttled or not called until you come back
to that tab. That means React's rendering, any any other scheduled work,
are paused.
Users expect the page to continue loading, and rendering is part of the
page load in a React app. So we need to continue calling callbacks.
test plan:
Manually tested using the new fixture test, observed that the callbacks
were called while switched to another tab. They were called more
slowly, but that seems like a reasonable thing.
issue:
Internal task T30754186