fix: defer useInterval until callback resolves (#5096)

* fix: defer useInterval until callback resolves

* fix: avoid refs in useInterval

src/hooks/useMachineTime.ts

 import useInterval from 'lib/hooks/useInterval'
-import { useState } from 'react'
+import { useCallback, useState } from 'react'
 
 const useMachineTimeMs = (updateInterval: number): number => {
   const [now, setNow] = useState(Date.now())
 
-  useInterval(() => {
-    setNow(Date.now())
-  }, updateInterval)
+  useInterval(
+    useCallback(() => {
+      setNow(Date.now())
+    }, []),
+    updateInterval
+  )
   return now
 }
 

src/lib/hooks/useInterval.test.tsx

+import { renderHook } from '@testing-library/react'
+
+import useInterval from './useInterval'
+
+describe('useInterval', () => {
+  const spy = jest.fn()
+
+  it('with no interval it does not run', () => {
+    renderHook(() => useInterval(spy, null))
+    expect(spy).toHaveBeenCalledTimes(0)
+  })
+
+  describe('with a synchronous function', () => {
+    it('it runs on an interval', () => {
+      jest.useFakeTimers()
+
+      renderHook(() => useInterval(spy, 100))
+      expect(spy).toHaveBeenCalledTimes(1)
+
+      jest.runTimersToTime(100)
+      expect(spy).toHaveBeenCalledTimes(2)
+    })
+  })
+
+  describe('with an async funtion', () => {
+    it('it runs on an interval exclusive of fn resolving', async () => {
+      jest.useFakeTimers()
+      spy.mockImplementation(() => Promise.resolve(undefined))
+
+      renderHook(() => useInterval(spy, 100))
+      expect(spy).toHaveBeenCalledTimes(1)
+
+      jest.runTimersToTime(100)
+      expect(spy).toHaveBeenCalledTimes(1)
+
+      await spy.mock.results[0].value
+      jest.runTimersToTime(100)
+      expect(spy).toHaveBeenCalledTimes(2)
+    })
+  })
+})

src/lib/hooks/useInterval.ts

-import { useEffect, useRef } from 'react'
+import { useEffect } from 'react'
 
 /**
  * Invokes callback repeatedly over an interval defined by the delay
+ *
  * @param callback
  * @param delay if null, the callback will not be invoked
- * @param leading if true, the callback will be invoked immediately (on the leading edge); otherwise, it will be invoked after delay
+ * @param leading by default, the callback will be invoked immediately (on the leading edge);
+ *                if false, the callback will not be invoked until a first delay
  */
-export default function useInterval(callback: () => void, delay: null | number, leading = true) {
-  const savedCallback = useRef<() => void>()
-
-  // Remember the latest callback.
+export default function useInterval(callback: () => void | Promise<void>, delay: null | number, leading = true) {
   useEffect(() => {
-    savedCallback.current = callback
-  }, [callback])
+    if (delay === null) {
+      return
+    }
 
-  // Set up the interval.
-  useEffect(() => {
-    function tick() {
-      const { current } = savedCallback
-      current && current()
+    let timeout: ReturnType<typeof setTimeout>
+    tick(delay, /* skip= */ !leading)
+    return () => {
+      if (timeout) {
+        clearInterval(timeout)
+      }
     }
 
-    if (delay !== null) {
-      if (leading) tick()
-      const id = setInterval(tick, delay)
-      return () => clearInterval(id)
+    async function tick(delay: number, skip = false) {
+      if (!skip) {
+        const promise = callback()
+
+        // Defer the next interval until the current callback has resolved.
+        if (promise) await promise
+      }
+
+      timeout = setTimeout(() => tick(delay), delay)
     }
-    return
-  }, [delay, leading])
+  }, [callback, delay, leading])
 }