Use same interval handler in all supported clocks

Author: eps1lon

src/__tests__/wait-for.js

@@ -207,7 +207,7 @@ test('if you switch from fake timers to real timers during the wait period you g
   const error = await waitForError
 
   expect(error.message).toMatchInlineSnapshot(
-    `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
+    `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
   )
   // stack trace has this file in it
   expect(error.stack).toMatch(__dirname)
@@ -223,7 +223,7 @@ test('if you switch from real timers to fake timers during the wait period you g
   const error = await waitForError
 
   expect(error.message).toMatchInlineSnapshot(
-    `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
+    `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
   )
   // stack trace has this file in it
   expect(error.stack).toMatch(__dirname)

src/wait-for.js

@@ -43,26 +43,26 @@ function waitFor(
   }
 
   return new Promise(async (resolve, reject) => {
-    let lastError, intervalId, observer
+    let lastError, observer
     let finished = false
     let promiseStatus = 'idle'
 
-    const overallTimeoutTimer = setTimeout(handleTimeout, timeout)
+    const overallTimeout = setTimeout(handleTimeout, timeout)
+    const intervalId = setInterval(handleInterval, interval)
 
-    const usingJestFakeTimers = jestFakeTimersAreEnabled()
-    if (usingJestFakeTimers) {
+    const wasUsingJestFakeTimers = jestFakeTimersAreEnabled()
+    if (wasUsingJestFakeTimers) {
       checkCallback()
       // this is a dangerous rule to disable because it could lead to an
       // infinite loop. However, eslint isn't smart enough to know that we're
       // setting finished inside `onDone` which will be called when we're done
       // waiting or when we've timed out.
       // eslint-disable-next-line no-unmodified-loop-condition
       while (!finished) {
-        if (!jestFakeTimersAreEnabled()) {
-          const error = new Error(
-            `Changed from using fake timers to real timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to real timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
-          )
-          if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError)
+        try {
+          // jest.advanceTimersByTime will not throw if
+          ensureInvariantTimers()
+        } catch (error) {
           reject(error)
           return
         }
@@ -73,12 +73,6 @@ function waitFor(
         // by an interval instead. (We have a test for this case).
         jest.advanceTimersByTime(interval)
 
-        // 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.
@@ -96,19 +90,18 @@ function waitFor(
         reject(e)
         return
       }
-      intervalId = setInterval(checkRealTimersCallback, interval)
       const {MutationObserver} = getWindowFromNode(container)
-      observer = new MutationObserver(checkRealTimersCallback)
+      observer = new MutationObserver(handleInterval)
       observer.observe(container, mutationObserverOptions)
       checkCallback()
     }
 
     function onDone(error, result) {
       finished = true
-      clearTimeout(overallTimeoutTimer)
+      clearTimeout(overallTimeout)
+      clearInterval(intervalId)
 
-      if (!usingJestFakeTimers) {
-        clearInterval(intervalId)
+      if (observer !== undefined) {
         observer.disconnect()
       }
 
@@ -119,15 +112,27 @@ function waitFor(
       }
     }
 
-    function checkRealTimersCallback() {
-      if (jestFakeTimersAreEnabled()) {
+    function ensureInvariantTimers() {
+      const isUsingJestFakeTimers = jestFakeTimersAreEnabled()
+      if (wasUsingJestFakeTimers !== isUsingJestFakeTimers) {
         const error = new Error(
-          `Changed from using real timers to fake timers while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing to fake timers. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
+          `Changed from using ${
+            wasUsingJestFakeTimers ? 'fake timers' : 'real timers'
+          } to ${
+            isUsingJestFakeTimers ? 'fake timers' : 'real timers'
+          } while using waitFor. This is not allowed and will result in very strange behavior. Please ensure you're awaiting all async things your test is doing before changing what timers your test is using. For more info, please go to https://github.com/testing-library/dom-testing-library/issues/830`,
         )
         if (!showOriginalStackTrace) copyStackTrace(error, stackTraceError)
-        return reject(error)
-      } else {
+        throw error
+      }
+    }
+
+    function handleInterval() {
+      try {
+        ensureInvariantTimers()
         return checkCallback()
+      } catch (error) {
+        return reject(error)
       }
     }