From 739afef8a5d6f8648dd21165e3d4e85d8b0ddecd Mon Sep 17 00:00:00 2001
From: "Kent C. Dodds" <me+github@kentcdodds.com>
Date: Tue, 23 Jun 2020 15:09:38 -0600
Subject: [PATCH] fix(waitFor): handle odd timing issue with fake timers

---
 src/__tests__/fake-timers.js | 20 +++++++++++++++++---
 src/wait-for.js              | 15 +++++++++++++--
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/__tests__/fake-timers.js b/src/__tests__/fake-timers.js
index 9f451956..285dfb65 100644
--- a/src/__tests__/fake-timers.js
+++ b/src/__tests__/fake-timers.js
@@ -9,14 +9,14 @@ afterAll(() => {
   jest.useRealTimers()
 })
 
-async function runWaitFor() {
+async function runWaitFor({time = 300} = {}, options) {
   const response = 'data'
   const doAsyncThing = () =>
-    new Promise(r => setTimeout(() => r(response), 300))
+    new Promise(r => setTimeout(() => r(response), time))
   let result
   doAsyncThing().then(r => (result = r))
 
-  await waitFor(() => expect(result).toBe(response))
+  await waitFor(() => expect(result).toBe(response), options)
 }
 
 test('real timers', async () => {
@@ -64,3 +64,17 @@ test('times out after 1000ms by default', async () => {
   // that we're using real timers.
   expect(performance.now() - start).toBeGreaterThanOrEqual(900)
 })
+
+test('recursive timers do not cause issues', async () => {
+  let recurse = true
+  function startTimer() {
+    setTimeout(() => {
+      if (recurse) startTimer()
+    }, 1)
+  }
+
+  startTimer()
+  await runWaitFor({time: 800}, {timeout: 100})
+
+  recurse = false
+})
diff --git a/src/wait-for.js b/src/wait-for.js
index 0a633f3c..a79c23bf 100644
--- a/src/wait-for.js
+++ b/src/wait-for.js
@@ -52,14 +52,25 @@ function waitFor(
       // waiting or when we've timed out.
       // eslint-disable-next-line no-unmodified-loop-condition
       while (!finished) {
+        // we *could* (maybe should?) use `advanceTimersToNextTimer` but it's
+        // possible that could make this loop go on forever if someone is using
+        // third party code that's setting up recursive timers so rapidly that
+        // the user's timer's don't get a chance to resolve. So we'll advance
+        // by an interval instead. (We have a test for this case).
         jest.advanceTimersByTime(interval)
-        // in this rare case, we *need* to wait for in-flight promises
+
+        // It's really important that checkCallback is run *before* we flush
+        // in-flight promises. To be honest, I'm not sure why, and I can't quite
+        // think of a way to reproduce the problem in a test, but I spent
+        // an entire day banging my head against a wall on this.
+        checkCallback()
+
+        // In this rare case, we *need* to wait for in-flight promises
         // to resolve before continuing. We don't need to take advantage
         // of parallelization so we're fine.
         // https://stackoverflow.com/a/59243586/971592
         // eslint-disable-next-line no-await-in-loop
         await new Promise(r => setImmediate(r))
-        checkCallback()
       }
     } else {
       intervalId = setInterval(checkCallback, interval)