A number of StrictMode fixes and updates (#99)

* fix(useTimeout): make StrictMode safe

* fix(useAnimationFrame)!: Make StrictMode safe

BREAKING CHANGE: no longer supports `cancelPrevious` this is always true

* fix:(useDebouncedCallback): Clean up timeout logic in strict mode

* chore: deprecate useWillUnmount

This hook is not possible in StrictMode, and can cause bugs

* fix(useForceUpdate): ensure that chained calls produce an update

* Update useCustomEffect.ts

* address feedback

Author: jquense

src/useAnimationFrame.ts

@@ -1,7 +1,5 @@
-import { useRef } from 'react'
+import { useEffect, useState } from 'react'
 import useMounted from './useMounted'
-import useStableMemo from './useStableMemo'
-import useWillUnmount from './useWillUnmount'
 
 export interface UseAnimationFrameReturn {
   cancel(): void
@@ -11,15 +9,12 @@ export interface UseAnimationFrameReturn {
    * Previously registered callbacks will be cancelled
    */
   request(callback: FrameRequestCallback): void
-
-  /**
-   * Request for the provided callback to be called on the next animation frame.
-   * Previously registered callbacks can be cancelled by providing `cancelPrevious`
-   */
-  request(cancelPrevious: boolean, callback: FrameRequestCallback): void
+}
+type AnimationFrameState = {
+  fn: FrameRequestCallback
 }
 /**
- * Returns a controller object for requesting and cancelling an animation freame that is properly cleaned up
+ * Returns a controller object for requesting and cancelling an animation frame that is properly cleaned up
  * once the component unmounts. New requests cancel and replace existing ones.
  *
  * ```ts
@@ -45,32 +40,32 @@ export interface UseAnimationFrameReturn {
  */
 export default function useAnimationFrame(): UseAnimationFrameReturn {
   const isMounted = useMounted()
-  const handle = useRef<number | undefined>()
 
-  const cancel = () => {
-    if (handle.current != null) {
-      cancelAnimationFrame(handle.current)
-    }
-  }
+  const [animationFrame, setAnimationFrameState] =
+    useState<AnimationFrameState | null>(null)
 
-  useWillUnmount(cancel)
+  useEffect(() => {
+    if (!animationFrame) {
+      return
+    }
 
-  return useStableMemo(
-    () => ({
-      request(
-        cancelPrevious: boolean | FrameRequestCallback,
-        fn?: FrameRequestCallback,
-      ) {
-        if (!isMounted()) return
+    const { fn } = animationFrame
+    const handle = requestAnimationFrame(fn)
+    return () => {
+      cancelAnimationFrame(handle)
+    }
+  }, [animationFrame])
 
-        if (cancelPrevious) cancel()
+  const [returnValue] = useState(() => ({
+    request(callback: FrameRequestCallback) {
+      if (!isMounted()) return
+      setAnimationFrameState({ fn: callback })
+    },
+    cancel: () => {
+      if (!isMounted()) return
+      setAnimationFrameState(null)
+    },
+  }))
 
-        handle.current = requestAnimationFrame(
-          fn || (cancelPrevious as FrameRequestCallback),
-        )
-      },
-      cancel,
-    }),
-    [],
-  )
+  return returnValue
 }

src/useCustomEffect.ts

@@ -5,7 +5,6 @@ import {
   useEffect,
   useDebugValue,
 } from 'react'
-import useWillUnmount from './useWillUnmount'
 import useMounted from './useMounted'
 
 export type EffectHook = (effect: EffectCallback, deps?: DependencyList) => void

src/useDebouncedCallback.ts

@@ -1,7 +1,6 @@
 import { useMemo, useRef } from 'react'
 import useTimeout from './useTimeout'
 import useEventCallback from './useEventCallback'
-import useWillUnmount from './useWillUnmount'
 
 export interface UseDebouncedCallbackOptions {
   wait: number
@@ -55,8 +54,6 @@ function useDebouncedCallback<TCallback extends (...args: any[]) => any>(
 
   const isTimerSetRef = useRef(false)
   const lastArgsRef = useRef<unknown[] | null>(null)
-  // Use any to bypass type issue with setTimeout.
-  const timerRef = useRef<any>(0)
 
   const handleCallback = useEventCallback(fn)
 
@@ -71,11 +68,6 @@ function useDebouncedCallback<TCallback extends (...args: any[]) => any>(
 
   const timeout = useTimeout()
 
-  useWillUnmount(() => {
-    clearTimeout(timerRef.current)
-    isTimerSetRef.current = false
-  })
-
   return useMemo(() => {
     const hasMaxWait = !!maxWait
 
@@ -168,14 +160,14 @@ function useDebouncedCallback<TCallback extends (...args: any[]) => any>(
         if (hasMaxWait) {
           // Handle invocations in a tight loop.
           isTimerSetRef.current = true
-          timerRef.current = setTimeout(timerExpired, wait)
+          timeout.set(timerExpired, wait)
           return invokeFunc(lastCallTimeRef.current)
         }
       }
 
       if (!isTimerSetRef.current) {
         isTimerSetRef.current = true
-        timerRef.current = setTimeout(timerExpired, wait)
+        timeout.set(timerExpired, wait)
       }
 
       return returnValueRef.current

src/useForceUpdate.ts

@@ -19,6 +19,6 @@ import { useReducer } from 'react'
 export default function useForceUpdate(): () => void {
   // The toggling state value is designed to defeat React optimizations for skipping
   // updates when they are strictly equal to the last state value
-  const [, dispatch] = useReducer((state: boolean) => !state, false)
+  const [, dispatch] = useReducer((revision) => revision + 1, 0)
   return dispatch as () => void
 }

src/useTimeout.ts

@@ -1,6 +1,5 @@
-import { MutableRefObject, useMemo, useRef } from 'react'
+import { MutableRefObject, useEffect, useRef, useState } from 'react'
 import useMounted from './useMounted'
-import useWillUnmount from './useWillUnmount'
 
 /*
  * Browsers including Internet Explorer, Chrome, Safari, and Firefox store the
@@ -28,12 +27,14 @@ function setChainedTimeout(
         )
 }
 
+type TimeoutState = {
+  fn: () => void
+  delayMs: number
+}
 /**
  * Returns a controller object for setting a timeout that is properly cleaned up
  * once the component unmounts. New timeouts cancel and replace existing ones.
  *
- *
- *
  * ```tsx
  * const { set, clear } = useTimeout();
  * const [hello, showHello] = useState(false);
@@ -47,33 +48,60 @@ function setChainedTimeout(
  * ```
  */
 export default function useTimeout() {
+  const [timeout, setTimeoutState] = useState<TimeoutState | null>(null)
   const isMounted = useMounted()
 
   // types are confused between node and web here IDK
-  const handleRef = useRef<any>()
+  const handleRef = useRef<any>(null)
 
-  useWillUnmount(() => clearTimeout(handleRef.current))
+  useEffect(() => {
+    if (!timeout) {
+      return
+    }
 
-  return useMemo(() => {
-    const clear = () => clearTimeout(handleRef.current)
+    const { fn, delayMs } = timeout
 
-    function set(fn: () => void, delayMs = 0): void {
-      if (!isMounted()) return
+    function task() {
+      if (isMounted()) {
+        setTimeoutState(null)
+      }
+      fn()
+    }
 
-      clear()
+    if (delayMs <= MAX_DELAY_MS) {
+      // For simplicity, if the timeout is short, just set a normal timeout.
+      handleRef.current = setTimeout(task, delayMs)
+    } else {
+      setChainedTimeout(handleRef, task, Date.now() + delayMs)
+    }
+    const handle = handleRef.current
 
-      if (delayMs <= MAX_DELAY_MS) {
-        // For simplicity, if the timeout is short, just set a normal timeout.
-        handleRef.current = setTimeout(fn, delayMs)
-      } else {
-        setChainedTimeout(handleRef, fn, Date.now() + delayMs)
+    return () => {
+      // this should be a no-op since they are either the same or `handle`
+      // already expired but no harm in calling twice
+      if (handleRef.current !== handle) {
+        clearTimeout(handle)
       }
+
+      clearTimeout(handleRef.current)
+      handleRef.current === null
     }
+  }, [timeout])
 
+  const [returnValue] = useState(() => {
     return {
-      set,
-      clear,
+      set(fn: () => void, delayMs = 0): void {
+        if (!isMounted()) return
+
+        setTimeoutState({ fn, delayMs })
+      },
+      clear() {
+        setTimeoutState(null)
+      },
+      isPending: !!timeout,
       handleRef,
     }
-  }, [])
+  })
+
+  return returnValue
 }

src/useWillUnmount.ts

@@ -5,6 +5,7 @@ import { useEffect } from 'react'
  * Attach a callback that fires when a component unmounts
  *
  * @param fn Handler to run when the component unmounts
+ * @deprecated Use `useMounted` and normal effects, this is not StrictMode safe
  * @category effects
  */
 export default function useWillUnmount(fn: () => void) {

test/useDebouncedCallback.test.tsx

@@ -8,6 +8,12 @@ describe('useDebouncedCallback', () => {
     jest.useFakeTimers()
   })
 
+  afterEach(() => {
+    act(() => {
+      jest.runAllTimers()
+    })
+  })
+
   it('should return a function that debounces input callback', () => {
     const callback = jest.fn()
 
@@ -21,7 +27,9 @@ describe('useDebouncedCallback', () => {
 
     expect(callback).not.toHaveBeenCalled()
 
-    jest.runOnlyPendingTimers()
+    act(() => {
+      jest.runOnlyPendingTimers()
+    })
 
     expect(callback).toHaveBeenCalledTimes(1)
     expect(callback).toHaveBeenCalledWith(3)
@@ -47,7 +55,6 @@ describe('useDebouncedCallback', () => {
     act(() => {
       jest.runOnlyPendingTimers()
     })
-
     expect(callback).toHaveBeenCalledTimes(1)
   })
 
@@ -90,16 +97,13 @@ describe('useDebouncedCallback', () => {
       result.current()
       result.current()
       result.current()
-
-      setTimeout(() => {
-        result.current()
-      }, 1001)
     })
 
     expect(callback).toHaveBeenCalledTimes(1)
 
     act(() => {
       jest.advanceTimersByTime(1001)
+      result.current()
     })
 
     expect(callback).toHaveBeenCalledTimes(3)
@@ -137,17 +141,25 @@ describe('useDebouncedCallback', () => {
     const callback = jest.fn(() => 42)
 
     const { result } = renderHook(() => useDebouncedCallback(callback, 1000))
+    let retVal
+
+    act(() => {
+      retVal = result.current()
+    })
 
-    const retVal = result.current()
     expect(callback).toHaveBeenCalledTimes(0)
     expect(retVal).toBeUndefined()
 
     act(() => {
       jest.runAllTimers()
     })
+
     expect(callback).toHaveBeenCalledTimes(1)
 
-    const subsequentResult = result.current()
+    let subsequentResult
+    act(() => {
+      subsequentResult = result.current()
+    })
 
     expect(callback).toHaveBeenCalledTimes(1)
     expect(subsequentResult).toBe(42)
@@ -160,7 +172,10 @@ describe('useDebouncedCallback', () => {
       useDebouncedCallback(callback, { wait: 1000, leading: true }),
     )
 
-    const retVal = result.current()
+    let retVal
+    act(() => {
+      retVal = result.current()
+    })
 
     expect(callback).toHaveBeenCalledTimes(1)
     expect(retVal).toEqual(42)

test/useDebouncedState.test.tsx

@@ -17,11 +17,13 @@ describe('useDebouncedState', () => {
     const wrapper = render(<Wrapper />)
     expect(wrapper.getByText('0')).toBeTruthy()
 
-    outerSetValue((cur: number) => cur + 1)
-    outerSetValue((cur: number) => cur + 1)
-    outerSetValue((cur: number) => cur + 1)
-    outerSetValue((cur: number) => cur + 1)
-    outerSetValue((cur: number) => cur + 1)
+    act(() => {
+      outerSetValue((cur: number) => cur + 1)
+      outerSetValue((cur: number) => cur + 1)
+      outerSetValue((cur: number) => cur + 1)
+      outerSetValue((cur: number) => cur + 1)
+      outerSetValue((cur: number) => cur + 1)
+    })
 
     expect(wrapper.getByText('0')).toBeTruthy()