feat: use error boundary to capture useEffect errors (#539)

Fixes #308

Author: mpeyper

Diff for: package.json

@@ -45,7 +45,9 @@
     "@babel/runtime": "^7.12.5",
     "@types/react": ">=16.9.0",
     "@types/react-dom": ">=16.9.0",
-    "@types/react-test-renderer": ">=16.9.0"
+    "@types/react-test-renderer": ">=16.9.0",
+    "filter-console": "^0.1.1",
+    "react-error-boundary": "^3.1.0"
   },
   "devDependencies": {
     "@typescript-eslint/eslint-plugin": "^4.9.1",

Diff for: src/dom/__tests__/errorHook.test.ts

@@ -108,13 +108,7 @@ describe('error hook tests', () => {
     })
   })
 
-  /*
-    These tests capture error cases that are not currently being caught successfully.
-    Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
-    for more details.
-  */
-  // eslint-disable-next-line jest/no-disabled-tests
-  describe.skip('effect', () => {
+  describe('effect', () => {
     test('should raise effect error', () => {
       const { result } = renderHook(() => useEffectError(true))
 

Diff for: src/dom/__tests__/resultHistory.test.ts

@@ -1,34 +1,39 @@
 import { renderHook } from '..'
 
 describe('result history tests', () => {
-  let count = 0
-  function useCounter() {
-    const result = count++
-    if (result === 2) {
+  function useValue(value: number) {
+    if (value === 2) {
       throw Error('expected')
     }
-    return result
+    return value
   }
 
   test('should capture all renders states of hook', () => {
-    const { result, rerender } = renderHook(() => useCounter())
+    const { result, rerender } = renderHook((value) => useValue(value), {
+      initialProps: 0
+    })
 
     expect(result.current).toEqual(0)
     expect(result.all).toEqual([0])
 
-    rerender()
+    rerender(1)
 
     expect(result.current).toBe(1)
     expect(result.all).toEqual([0, 1])
 
-    rerender()
+    rerender(2)
 
     expect(result.error).toEqual(Error('expected'))
     expect(result.all).toEqual([0, 1, Error('expected')])
 
-    rerender()
+    rerender(3)
 
     expect(result.current).toBe(3)
     expect(result.all).toEqual([0, 1, Error('expected'), 3])
+
+    rerender()
+
+    expect(result.current).toBe(3)
+    expect(result.all).toEqual([0, 1, Error('expected'), 3, 3])
   })
 })

Diff for: src/helpers/createTestHarness.tsx

@@ -1,42 +1,65 @@
 import React, { Suspense } from 'react'
+import { ErrorBoundary, FallbackProps } from 'react-error-boundary'
+import filterConsole from 'filter-console'
 
-import { RendererProps, WrapperComponent } from '../types/react'
+import { addCleanup } from '../core'
 
-import { isPromise } from './promises'
+import { RendererProps, WrapperComponent } from '../types/react'
 
-function TestComponent<TProps, TResult>({
-  hookProps,
-  callback,
-  setError,
-  setValue
-}: RendererProps<TProps, TResult> & { hookProps?: TProps }) {
-  try {
-    // coerce undefined into TProps, so it maintains the previous behaviour
-    setValue(callback(hookProps as TProps))
-  } catch (err: unknown) {
-    if (isPromise(err)) {
-      throw err
-    } else {
-      setError(err as Error)
+function suppressErrorOutput() {
+  // The error output from error boundaries is notoriously difficult to suppress.  To save
+  // out users from having to work it out, we crudely suppress the output matching the patterns
+  // below.  For more information, see these issues:
+  //   - https://github.com/testing-library/react-hooks-testing-library/issues/50
+  //   - https://github.com/facebook/react/issues/11098#issuecomment-412682721
+  //   - https://github.com/facebook/react/issues/15520
+  //   - https://github.com/facebook/react/issues/18841
+  const removeConsoleFilter = filterConsole(
+    [
+      /^The above error occurred in the <TestComponent> component:/, // error boundary output
+      /^Error: Uncaught .+/ // jsdom output
+    ],
+    {
+      methods: ['error']
     }
-  }
-  return null
+  )
+  addCleanup(removeConsoleFilter)
 }
 
 function createTestHarness<TProps, TResult>(
-  rendererProps: RendererProps<TProps, TResult>,
+  { callback, setValue, setError }: RendererProps<TProps, TResult>,
   Wrapper?: WrapperComponent<TProps>,
   suspense: boolean = true
 ) {
+  const TestComponent = ({ hookProps }: { hookProps?: TProps }) => {
+    // coerce undefined into TProps, so it maintains the previous behaviour
+    setValue(callback(hookProps as TProps))
+    return null
+  }
+
+  let resetErrorBoundary = () => {}
+  const ErrorFallback = ({ error, resetErrorBoundary: reset }: FallbackProps) => {
+    resetErrorBoundary = () => {
+      resetErrorBoundary = () => {}
+      reset()
+    }
+    setError(error)
+    return null
+  }
+
+  suppressErrorOutput()
+
   const testHarness = (props?: TProps) => {
-    let component = <TestComponent hookProps={props} {...rendererProps} />
+    resetErrorBoundary()
+
+    let component = <TestComponent hookProps={props} />
     if (Wrapper) {
       component = <Wrapper {...(props as TProps)}>{component}</Wrapper>
     }
     if (suspense) {
       component = <Suspense fallback={null}>{component}</Suspense>
     }
-    return component
+    return <ErrorBoundary FallbackComponent={ErrorFallback}>{component}</ErrorBoundary>
   }
 
   return testHarness

Diff for: src/helpers/promises.ts

@@ -2,13 +2,9 @@ function resolveAfter(ms: number) {
   return new Promise<void>((resolve) => setTimeout(resolve, ms))
 }
 
-export async function callAfter(callback: () => void, ms: number) {
+async function callAfter(callback: () => void, ms: number) {
   await resolveAfter(ms)
   callback()
 }
 
-function isPromise<T>(value: unknown): boolean {
-  return typeof (value as PromiseLike<T>).then === 'function'
-}
-
-export { isPromise, resolveAfter }
+export { resolveAfter, callAfter }

Diff for: src/native/__tests__/errorHook.test.ts

@@ -108,13 +108,7 @@ describe('error hook tests', () => {
     })
   })
 
-  /*
-    These tests capture error cases that are not currently being caught successfully.
-    Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
-    for more details.
-  */
-  // eslint-disable-next-line jest/no-disabled-tests
-  describe.skip('effect', () => {
+  describe('effect', () => {
     test('should raise effect error', () => {
       const { result } = renderHook(() => useEffectError(true))
 

Diff for: src/native/__tests__/resultHistory.test.ts

@@ -1,34 +1,39 @@
 import { renderHook } from '..'
 
 describe('result history tests', () => {
-  let count = 0
-  function useCounter() {
-    const result = count++
-    if (result === 2) {
+  function useValue(value: number) {
+    if (value === 2) {
       throw Error('expected')
     }
-    return result
+    return value
   }
 
   test('should capture all renders states of hook', () => {
-    const { result, rerender } = renderHook(() => useCounter())
+    const { result, rerender } = renderHook((value) => useValue(value), {
+      initialProps: 0
+    })
 
     expect(result.current).toEqual(0)
     expect(result.all).toEqual([0])
 
-    rerender()
+    rerender(1)
 
     expect(result.current).toBe(1)
     expect(result.all).toEqual([0, 1])
 
-    rerender()
+    rerender(2)
 
     expect(result.error).toEqual(Error('expected'))
     expect(result.all).toEqual([0, 1, Error('expected')])
 
-    rerender()
+    rerender(3)
 
     expect(result.current).toBe(3)
     expect(result.all).toEqual([0, 1, Error('expected'), 3])
+
+    rerender()
+
+    expect(result.current).toBe(3)
+    expect(result.all).toEqual([0, 1, Error('expected'), 3, 3])
   })
 })

Diff for: src/server/__tests__/errorHook.test.ts

@@ -119,13 +119,7 @@ describe('error hook tests', () => {
     })
   })
 
-  /*
-    These tests capture error cases that are not currently being caught successfully.
-    Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
-    for more details.
-  */
-  // eslint-disable-next-line jest/no-disabled-tests
-  describe.skip('effect', () => {
+  describe('effect', () => {
     test('should raise effect error', () => {
       const { result, hydrate } = renderHook(() => useEffectError(true))
 

Diff for: src/server/__tests__/resultHistory.test.ts

@@ -0,0 +1,44 @@
+import { renderHook } from '..'
+
+describe('result history tests', () => {
+  function useValue(value: number) {
+    if (value === 2) {
+      throw Error('expected')
+    }
+    return value
+  }
+
+  test('should capture all renders states of hook', () => {
+    const { result, hydrate, rerender } = renderHook((value) => useValue(value), {
+      initialProps: 0
+    })
+
+    expect(result.current).toEqual(0)
+    expect(result.all).toEqual([0])
+
+    hydrate()
+
+    expect(result.current).toEqual(0)
+    expect(result.all).toEqual([0, 0])
+
+    rerender(1)
+
+    expect(result.current).toBe(1)
+    expect(result.all).toEqual([0, 0, 1])
+
+    rerender(2)
+
+    expect(result.error).toEqual(Error('expected'))
+    expect(result.all).toEqual([0, 0, 1, Error('expected')])
+
+    rerender(3)
+
+    expect(result.current).toBe(3)
+    expect(result.all).toEqual([0, 0, 1, Error('expected'), 3])
+
+    rerender()
+
+    expect(result.current).toBe(3)
+    expect(result.all).toEqual([0, 0, 1, Error('expected'), 3, 3])
+  })
+})

Diff for: src/server/pure.ts

@@ -20,8 +20,12 @@ function createServerRenderer<TProps, TResult>(
     render(props?: TProps) {
       renderProps = props
       act(() => {
-        const serverOutput = ReactDOMServer.renderToString(testHarness(props))
-        container.innerHTML = serverOutput
+        try {
+          const serverOutput = ReactDOMServer.renderToString(testHarness(props))
+          container.innerHTML = serverOutput
+        } catch (e: unknown) {
+          rendererProps.setError(e as Error)
+        }
       })
     },
     hydrate() {