I'm not sure this does what the comment says it does.

Does the guarantee that all blocks are updated come from the fact that we're waiting for the current cycle of the event loop to end and the next one to begin? If so, you can use a timeout of zero and skip waiting for 500ms. Otherwise, those 500ms offer no guarantee of completion and a different approach will be needed to ensure this always works.

I realise this is just a test, but I could see this turning into a hard-to-debug flaky test in the future :-/

In async mode, each single component is updated in its own cycle. which mean I don't know the exact number of cycles required here. But yeah, maybe there's a better way.

Can we force components to update synchronously in a test environment?

This feels very concerning to me as is.

We could it but I personally prefer not to because it creates a difference in behavior for tests we may not be aware of. For example, I did find a lot of "bugs" when I originally implemented the async mode thanks to the e2e tests which we won't catch if we use sync mode in tests.

In that case, maybe it's worth having a separate test case which could target and specifically verify some expected difference between synchronous and asynchronous modes, or run all / a subset of tests under each, or abstract the problematic interactions under a "safe" variant which handles the expected delay.

What I don't think we should start doing is arbitrarily littering some tests with randomly chosen timeouts which may or may not be sufficient time to complete the asynchronous task (fragile), and may or may not needlessly delay the total runtime of the tests.

I can rewrite the tests slightly to avoid it.

Select the block and then check it's value because selecting a block triggers the sync mode.

I'm not sure a rAF is what you want here; this doesn't look like it should be tied to the screen's refresh rate.

What's the goal behind this wait? If you're just looking to defer things until the next run of the event loop, I'd use a simple lodash defer or the equivalent setTimeout() with a timeout of zero.

Do you instead want to make sure it doesn't all happen in one go and only when the browser is otherwise free to process things? If so, I'd use requestIdleCallback instead, which is only executed when the browser has spare time, with a polyfill or fallback of some sort for browsers that don't support it.

I think React's async mode uses requestAnimationFrame as well. I'm not certain what's the best approach yet but the idea is that when you're typing or moving the mouse or resizing, I want to leave the priority to the browser to do the most critical things.

That sounds like a good use-case for requestIdleCallback, then. Which as far as I can tell is what React's async mode uses as well, but I may have been looking at old code.

The motivation against using requestAnimationFrame is that you'd be doing non-animation work when the browser is getting ready to create a frame. This could lead to one of two things:

• If the work involved in each of these is complex, you could blow the 13ms budget and cause jank.
• If the work involved in each of these is not complex, you could be wasting time by taking numberOfItemsInQueue * 13ms to process the whole queue, when you could potentially be processing it all in much less time.

requestIdleCallback instead schedules things to be done when the browser expects it will have at least 50ms (I believe), which gives you a bigger budget for complex changes and allows for running multiple chunks of work in succession without waiting for another cycle if they're small enough. This could help in keeping changes coherent, too, instead of potentially having one component update per frame.

Looks like requestIdleCallback is not supported by the browsers we need, and I'm not certain it has polyfills.

React uses requestAnimationFrame to provide some kind of polyfill.

https://github.com/facebook/react/blob/653bc582f94e955b175d42836479ee8b9eeeeeaf/packages/scheduler/src/Scheduler.js

I'd be OK using rIC where supported and just having a polyfill as simple as rIC = ( ...args ) => requestAnimationFrame( ...args ) elsewhere, but I'll leave this up to you. As long as the numbers improve, that's what matters :)

While your code is good, I still have more confidence personally in using requestAnimationFrame for this reason:

It seems that in your code the queue will be cleared by batch which mean it will have two consequences:

• While executing a single batch (I guess it's 50ms duration), we can't perform incoming higher-priority work (typing)
• After each batch, we'll wait another 50ms for the next batch.

Anyway I updated using rAF.

No problem, I'll test as-is (using rAF) :) Again, if the improvements are there and it works correctly, it's already a win!

I just want to clarify that higher-priority work isn't an issue in the rIC case; that's the point of checking timeRemaining() after each item, and moving onto a new batch if there's no time left. We're trusting the browser to tell us there's no time left if more important work needs to be done.

As for waiting 50ms, yes, that's true, the wait is longer than the 13ms of a rAF. However, timing is often about waiting for the right time to do something, even if it takes a little longer. That's exactly what rIC tries to do, by using timing knowledge that's internal to the browser. Besides, in the rAF case, you'll incur the 13ms penalty between every item, rather than 50ms between batches in rIC. And since you expect each component's withSelect to be fast (which I assume to mean significantly less than 13ms), that likely means it'll be slower in aggregate, since the average batch would only need to have around 4 items for the rIC case to be faster.

Thanks for the clarifications :) I'm convinced now

Updated :)

Sweet, thank you! :D I should be able to get you some numbers today! :)

React uses requestAnimationFrame to implement polyfill for requestIdleCallback, which would otherwise be exactly the function they want to call.

The goal is to run code as soon as possible after a frame is painted. That's achieved with:

1. Scheduling a callback with requestAnimationFrame. But that alone schedules the callback right before a frame is painted, the opposite of what we want to achieve. That's why this callback just posts a message to the window.
2. Then there is a window.onmessage handler that does the real work. If a message is posted inside requestAnimationFrame, it's guaranteed to be processed ASAP, but not before the frame is painted. It'll be the first thing the browser will do after painting! Mission accomplished.

requestAnimationFrame callback, i.e., running code before frame paint, is useful for writing to DOM effectively, without causing layout trashing.

Running code after paint is useful for effective reading from DOM, without causing layout trashing.

React Hooks introduce new APIs for these purposes. useEffect is ran before paint and is good for DOM writing, and useLayoutEffect is ran after paint and is good for DOM reading. Or is it the other way around? I'm not sure 😄

Calypso has an afterLayoutFlush utility function that follows very closely the React implementation to schedule stuff after paint. But it uses setTimeout(0) instead of postMessage, because I didn't know the hairy details a year ago. setTimeout(0) has the disadvantage that it can skip many frames before running, e.g., when scrolling.

What is this value representing? For clarity and legibility, it'd be deserving of being assigned a variable.

It represents when we enable/disable the async mode, I can add a clarifying comments. The idea is that selected blocks are sync, the others are async.

Well, from how I interpret it, it represents whether the rendered block occurs within the current selection, something akin to a isBlockInSelection variable (or even selector, though I don't think this can help here as written).

I know we had it previously, but I wonder why it matters to have the key be prefixed.

It doesn't

Can we force components to update synchronously in a test environment?

This feels very concerning to me as is.

Well, from how I interpret it, it represents whether the rendered block occurs within the current selection, something akin to a isBlockInSelection variable (or even selector, though I don't think this can help here as written).