Unblock Preact tests that are executed against fake clock

[dvoytenko]

This pull request reverts #30181.

The description of the problem 1:

Preact schedules handler only once for a queue, when queue.length === 1. We had a situation where the fake test clock was used to schedule the update for queue.length === 1, but then it was discarded. All followup queue pushes were not scheduled because queue.length > 1. But the fake timer was never executed, so the queue was stuck - more items were added to it, but handler was never called. I was actually surprised by this a bit - I though clock.uninstall would run all pending timers, but apparently not.

Not yet clear how to defend from this in depth. Debugging this problem is extremely difficult - there's no way to determine a "hanged" queue on Preact's side. One thing I tried is setting Preact.options.requestAnimationFrame, which is similar to wrapping in act(). However, even with this option, Preact still relies on setTimeout.

The description of the problem 2:

Preact's test-utils used to break the queue when the render function fails. This has been fixed in preactjs/preact@4efcd75. However, we're currently blocked on our Preact upgrade due to unrelated Closure bug. See #30043.

In general, a failing render function is not a good idea and that should be fixed separately. However, noting this here since this is also very hard to debug.

[kristoferbaxter]

LGTM, future would be smart to add some checks to prohibit this from happening in other ported components.

[kristoferbaxter]

Makes sense, perhaps we should add some additional enforcement to ensure this happens for all tests with a preact implementation?

[dvoytenko]

Yeah. I'm looking for defense in depth right now. But no good ideas yet.

[samouri]

I actually ran into a very similar problem when making a rAF polyfill in worker-dom.

Are we reusing the same queue / outdated fake timer in between the test files, or was it the same instance of Preact that had grabbed/held-on to the rAF? A defense could be a new instance of Preact and of fake-timers for each test file

[dvoytenko]

@samouri The specific problem is that the Preact's global queue is shared between all tests. When the first element is added to the queue (and only when the first element added) the queue is scheduled with the setTimeout currently available in globals. If that setTimeout was a fake one - that's when the problem happens: it might never complete and Preact will not try to schedule anymore using the "good" setTimeout because queue is not empty. Preact actually protects itself from a similar problem with requestAnimationFrame. You can see it here. It checks whether the instance of rAF has changed since the last scheduling. Unfortunately the same is not done for setTimeout.

[marvinhagemeister]

@dvoytenko I think the options hook you were looking for is options.debounceRendering. By setting that you can override our internal scheduling function which is by default a microtask. options.requestAnimationFrame is only used in preact/hooks for effect scheduling. The full list of available option hooks is listed in the official Preact documentation.

Note: that we don't recommend changing scheduling functions. It can lead to unexpected consequences when libraries rely on specific scheduling semantics. Your choice to go the fake timer route is much more foolproof and reliable in the long run. It's indeed a gotcha with fake timers, that one has to run those to completion manually. Not sure how we can hint at this pitfall from Preact's side. We certainly can add a section for that in our docs.

[dvoytenko]

@marvinhagemeister, thanks a lot for the advice. We are actually running into an issue with hooks specifically. So I think per your note, we should try to work this around via options.requestAnimationFrame. I'll try it.

[dvoytenko]

Also discovered (and skipped) the following problem (updated in the description):

The description of the problem 2:
Preact's test-utils used to break the queue when the render function fails. This has been fixed in preactjs/preact@4efcd75. However, we're currently blocked on our Preact upgrade due to unrelated Closure bug. See #30043.

In general, a failing render function is not a good idea and that should be fixed separately. However, noting this here since this is also very hard to debug.