setInterval with delay 0 runs indefinitely, other scheduled tasks never execute
- FakeTimers version : 14.0.0
- Environment : node (standalone and used as part of jest fake timers)
- Example URL : reproduction repo
- Other libraries you are using: Not necessary to reproduce, but issue is worse when used with jest and react testing library
What did you expect to happen? If an interval and multiple timers are all scheduled to execute at the same time, I would expect all of them to execute once, prior to intervals executing again
What actually happens The interval with a delay of 0 runs repeatedly, and timeouts that are due to run are never executed. This is especially problematic when used in conjunction with jest and react testing library. RTL's waitFor method, when used with fake timers, relies on calling an immediate setTimeout and the advancing timers by 0. If the code under test uses a setInterval with a delay of 0, the test hangs forever. The spec in the reproduction repo demonstrates this without the use of RTL.
How to reproduce
The following code using fake timers
const FakeTimers = require("@sinonjs/fake-timers");
var clock = FakeTimers.install();
let intervalCount = 0;
const intervalId = setInterval(() => {
console.log("interval");
if (++intervalCount > 4) {
clearInterval(intervalId);
}
}, 0);
setTimeout(() => console.log("timeout"), 0);
clock.tick(0);
prints
interval
interval
interval
interval
interval
timeout
An example without fake timers, such as
let intervalCount = 0;
const intervalId = setInterval(() => {
console.log("interval");
if (intervalCount++ > 4) {
clearInterval(intervalId);
}
}, 0);
setTimeout(() => console.log("timeout"), 0);
prints
interval
timeout
interval
interval
interval
interval
interval
Additional Information In debugging this, it seems that the issue is that when intervals are executed, their callAt time is adjusted by the interval, but their position in the list of timers is not adjusted, so the interval continues to be the first timer to run. https://github.com/sinonjs/fake-timers/blob/48f089fdc830e39fcec31dd23099cc360da0bab2/src/fake-timers-src.js#L794
Excellent report! Thanks for taking the time. This seems doable, but I can't do this in the immediate future. Unless you can see a quick fix (which it seemed lik you did) and can supply a PR, I can only say I'll have a look at this in the start of January. Bit busy now just before X-mas :)
No rush at all on my end - it was easy enough to work around in this case since having an delay of 1 in the actual code was functionally equivalent.
I started working on this last night, moving our current hashmap approach into one using lists instead. The idea being that I could then pop a timer of the list for the current time and add it to the end of a (sub)list to avoid the issue above. Before getting to fixing that, I first need to get to feature parity with the current approach - basically making the test suite pass. I got mosts of the internal tests to work, but pretty much all the /tests/issue*.js tests still breaks, so still some more work to do. I could not immediately see what was wrong, so if anyone has some time to have a look, it's here:
https://github.com/sinonjs/fake-timers/tree/522-tasks-not-adjusted-on-settimeout-0