fix: only suppress console.error for non-pure imports (#549)

* fix: only suppress console.error for non-pure imports

* refactor: remove unused promise util

* chore: fix tests for error suppression to

* docs: update docs to with more detail on side effects

* refactor: only add suppression in beforeEach block and move restoration to afterEach

* chore: refactor error suppression tests to require in setup so hooks can actually be registered

* chore: added additional tests to ensure pure imports don't add side effects

* refactor: clean up unnecessary additional types in internal console suppression function

* docs: remove link in API reference docs

Fixes #546

Author: mpeyper

disable-error-filtering.js

@@ -0,0 +1 @@
+process.env.RHTL_DISABLE_ERROR_FILTERING = true

docs/api-reference.md

@@ -153,9 +153,8 @@ module.exports = {
 }
 ```
 
-Alternatively, you can change your test to import from `@testing-library/react-hooks/pure` instead
-of the regular imports. This applys to any of our export methods documented in
-[Rendering](/installation#being-specific).
+Alternatively, you can change your test to import from `@testing-library/react-hooks/pure` (or any
+of the [other non-pure imports](/installation#pure-imports)) instead of the regular imports.
 
 ```diff
 - import { renderHook, cleanup, act } from '@testing-library/react-hooks'
@@ -270,3 +269,49 @@ Interval checking is disabled if `interval` is not provided as a `falsy`.
 _Default: 1000_
 
 The maximum amount of time in milliseconds (ms) to wait.
+
+---
+
+## `console.error`
+
+In order to catch errors that are produced in all parts of the hook's lifecycle, the test harness
+used to wrap the hook call includes an
+[Error Boundary](https://reactjs.org/docs/error-boundaries.html) which causes a
+[significant amount of output noise](https://reactjs.org/docs/error-boundaries.html#component-stack-traces)
+in tests.
+
+To keep test output clean, we patch `console.error` when importing from
+`@testing-library/react-hooks` (or any of the [other non-pure imports](/installation#pure-imports))
+to filter out the unnecessary logging and restore the original version during cleanup. This
+side-effect can affect tests that also patch `console.error` (e.g. to assert a specific error
+message get logged) by replacing their custom implementation as well.
+
+### Disabling `console.error` filtering
+
+Importing `@testing-library/react-hooks/disable-error-filtering.js` in test setup files disable the
+error filtering feature and not patch `console.error` in any way.
+
+For example, in [Jest](https://jestjs.io/) this can be added to your
+[Jest config](https://jestjs.io/docs/configuration):
+
+```js
+module.exports = {
+  setupFilesAfterEnv: [
+    '@testing-library/react-hooks/disable-error-filtering.js'
+    // other setup files
+  ]
+}
+```
+
+Alternatively, you can change your test to import from `@testing-library/react-hooks/pure` (or any
+of the [other non-pure imports](/installation#pure-imports)) instead of the regular imports.
+
+```diff
+- import { renderHook, cleanup, act } from '@testing-library/react-hooks'
++ import { renderHook, cleanup, act } from '@testing-library/react-hooks/pure'
+```
+
+If neither of these approaches are suitable, setting the `RHTL_DISABLE_ERROR_FILTERING` environment
+variable to `true` before importing `@testing-library/react-hooks` will also disable this feature.
+
+> Please note that this may result is a significant amount of additional logging in you test output.

docs/installation.md

@@ -17,7 +17,7 @@ npm install --save-dev @testing-library/react-hooks
 yarn add --dev @testing-library/react-hooks
 ```
 
-### Peer Dependencies
+### Peer dependencies
 
 `react-hooks-testing-library` does not come bundled with a version of
 [`react`](https://www.npmjs.com/package/react) to allow you to install the specific version you want
@@ -92,7 +92,31 @@ import { renderHook, act } from '@testing-library/react-hooks/native' // will us
 import { renderHook, act } from '@testing-library/react-hooks/server' // will use react-dom/server
 ```
 
-## Testing Framework
+## Pure imports
+
+Importing from any of the previously mentioned imports will cause some side effects in the test
+environment:
+
+1. `cleanup` is automatically called in an `afterEach` block
+2. `console.error` is patched to hide some React errors
+
+The specifics of these side effects are discussed in more detail in the
+[API reference](/reference/api).
+
+If you want to ensure the imports are free of side-effects, you can use the `pure` imports instead,
+which can be accessed by appending `/pure` to the end of any of the other imports:
+
+```ts
+import { renderHook, act } from '@testing-library/react-hooks/pure'
+
+import { renderHook, act } from '@testing-library/react-hooks/dom/pure'
+
+import { renderHook, act } from '@testing-library/react-hooks/native/pure'
+
+import { renderHook, act } from '@testing-library/react-hooks/server/pure'
+```
+
+## Testing framework
 
 In order to run tests, you will probably want to be using a test framework. If you have not already
 got one, we recommend using [Jest](https://jestjs.io/), but this library should work without issues

package.json

@@ -18,6 +18,7 @@
     "native",
     "server",
     "pure",
+    "disable-error-filtering.js",
     "dont-cleanup-after-each.js"
   ],
   "author": "Michael Peyper <[email protected]>",

src/core/console.ts

@@ -0,0 +1,28 @@
+import filterConsole from 'filter-console'
+
+function enableErrorOutputSuppression() {
+  // Automatically registers console error suppression and restoration in supported testing frameworks
+  if (
+    typeof beforeEach === 'function' &&
+    typeof afterEach === 'function' &&
+    !process.env.RHTL_DISABLE_ERROR_FILTERING
+  ) {
+    let restoreConsole: () => void
+
+    beforeEach(() => {
+      restoreConsole = filterConsole(
+        [
+          /^The above error occurred in the <TestComponent> component:/, // error boundary output
+          /^Error: Uncaught .+/ // jsdom output
+        ],
+        {
+          methods: ['error']
+        }
+      )
+    })
+
+    afterEach(() => restoreConsole?.())
+  }
+}
+
+export { enableErrorOutputSuppression }

src/dom/__tests__/autoCleanup.disabled.test.ts

@@ -6,11 +6,10 @@ import { ReactHooksRenderer } from '../../types/react'
 // then we DON'T auto-wire up the afterEach for folks
 describe('skip auto cleanup (disabled) tests', () => {
   let cleanupCalled = false
-  let renderHook: (arg0: () => void) => void
+  let renderHook: ReactHooksRenderer['renderHook']
 
   beforeAll(() => {
     process.env.RHTL_SKIP_AUTO_CLEANUP = 'true'
-    // eslint-disable-next-line @typescript-eslint/no-var-requires
     renderHook = (require('..') as ReactHooksRenderer).renderHook
   })
 

src/dom/__tests__/autoCleanup.noAfterEach.test.ts

@@ -2,17 +2,15 @@ import { useEffect } from 'react'
 
 import { ReactHooksRenderer } from '../../types/react'
 
-// This verifies that if RHTL_SKIP_AUTO_CLEANUP is set
+// This verifies that if afterEach is unavailable
 // then we DON'T auto-wire up the afterEach for folks
 describe('skip auto cleanup (no afterEach) tests', () => {
   let cleanupCalled = false
-  let renderHook: (arg0: () => void) => void
+  let renderHook: ReactHooksRenderer['renderHook']
 
   beforeAll(() => {
     // @ts-expect-error Turning off AfterEach -- ignore Jest LifeCycle Type
-    // eslint-disable-next-line no-global-assign
     afterEach = false
-    // eslint-disable-next-line @typescript-eslint/no-var-requires
     renderHook = (require('..') as ReactHooksRenderer).renderHook
   })
 

src/dom/__tests__/autoCleanup.pure.test.ts

@@ -0,0 +1,29 @@
+import { useEffect } from 'react'
+
+import { ReactHooksRenderer } from '../../types/react'
+
+// This verifies that if pure imports are used
+// then we DON'T auto-wire up the afterEach for folks
+describe('skip auto cleanup (pure) tests', () => {
+  let cleanupCalled = false
+  let renderHook: ReactHooksRenderer['renderHook']
+
+  beforeAll(() => {
+    renderHook = (require('../pure') as ReactHooksRenderer).renderHook
+  })
+
+  test('first', () => {
+    const hookWithCleanup = () => {
+      useEffect(() => {
+        return () => {
+          cleanupCalled = true
+        }
+      })
+    }
+    renderHook(() => hookWithCleanup())
+  })
+
+  test('second', () => {
+    expect(cleanupCalled).toBe(false)
+  })
+})

src/dom/__tests__/errorHook.test.ts

@@ -1,5 +1,5 @@
 import { useState, useEffect } from 'react'
-import { renderHook } from '..'
+import { renderHook, act } from '..'
 
 describe('error hook tests', () => {
   function useError(throwError?: boolean) {
@@ -109,15 +109,15 @@ describe('error hook tests', () => {
   })
 
   describe('effect', () => {
-    test('should raise effect error', () => {
+    test('this one - should raise effect error', () => {
       const { result } = renderHook(() => useEffectError(true))
 
       expect(() => {
         expect(result.current).not.toBe(undefined)
       }).toThrow(Error('expected'))
     })
 
-    test('should capture effect error', () => {
+    test('this one - should capture effect error', () => {
       const { result } = renderHook(() => useEffectError(true))
       expect(result.error).toEqual(Error('expected'))
     })
@@ -142,4 +142,51 @@ describe('error hook tests', () => {
       expect(result.error).toBe(undefined)
     })
   })
+
+  describe('error output suppression', () => {
+    test('should allow console.error to be mocked', async () => {
+      const consoleError = console.error
+      console.error = jest.fn()
+
+      try {
+        const { rerender, unmount } = renderHook(
+          (stage) => {
+            useEffect(() => {
+              console.error(`expected in effect`)
+              return () => {
+                console.error(`expected in unmount`)
+              }
+            }, [])
+            console.error(`expected in ${stage}`)
+          },
+          {
+            initialProps: 'render'
+          }
+        )
+
+        act(() => {
+          console.error('expected in act')
+        })
+
+        await act(async () => {
+          await new Promise((resolve) => setTimeout(resolve, 100))
+          console.error('expected in async act')
+        })
+
+        rerender('rerender')
+
+        unmount()
+
+        expect(console.error).toBeCalledWith('expected in render')
+        expect(console.error).toBeCalledWith('expected in effect')
+        expect(console.error).toBeCalledWith('expected in act')
+        expect(console.error).toBeCalledWith('expected in async act')
+        expect(console.error).toBeCalledWith('expected in rerender')
+        expect(console.error).toBeCalledWith('expected in unmount')
+        expect(console.error).toBeCalledTimes(6)
+      } finally {
+        console.error = consoleError
+      }
+    })
+  })
 })

src/dom/__tests__/errorSuppression.disabled.test.ts

@@ -0,0 +1,16 @@
+// This verifies that if RHTL_DISABLE_ERROR_FILTERING is set
+// then we DON'T auto-wire up the afterEach for folks
+describe('error output suppression (disabled) tests', () => {
+  const originalConsoleError = console.error
+
+  beforeAll(() => {
+    process.env.RHTL_DISABLE_ERROR_FILTERING = 'true'
+  })
+
+  test('should not patch console.error', () => {
+    require('..')
+    expect(console.error).toBe(originalConsoleError)
+  })
+})
+
+export {}

src/dom/__tests__/errorSuppression.noAfterEach.test.ts

@@ -0,0 +1,17 @@
+// This verifies that if afterEach is unavailable
+// then we DON'T auto-wire up the afterEach for folks
+describe('error output suppression (noAfterEach) tests', () => {
+  const originalConsoleError = console.error
+
+  beforeAll(() => {
+    // @ts-expect-error Turning off AfterEach -- ignore Jest LifeCycle Type
+    afterEach = false
+    require('..')
+  })
+
+  test('should not patch console.error', () => {
+    expect(console.error).toBe(originalConsoleError)
+  })
+})
+
+export {}

src/dom/__tests__/errorSuppression.noBeforeEach.test.ts

@@ -0,0 +1,17 @@
+// This verifies that if afterEach is unavailable
+// then we DON'T auto-wire up the afterEach for folks
+describe('error output suppression (noBeforeEach) tests', () => {
+  const originalConsoleError = console.error
+
+  beforeAll(() => {
+    // @ts-expect-error Turning off BeforeEach -- ignore Jest LifeCycle Type
+    beforeEach = false
+    require('..')
+  })
+
+  test('should not patch console.error', () => {
+    expect(console.error).toBe(originalConsoleError)
+  })
+})
+
+export {}

src/dom/__tests__/errorSuppression.pure.test.ts

@@ -0,0 +1,15 @@
+// This verifies that if pure imports are used
+// then we DON'T auto-wire up the afterEach for folks
+describe('error output suppression (pure) tests', () => {
+  const originalConsoleError = console.error
+
+  beforeAll(() => {
+    require('../pure')
+  })
+
+  test('should not patch console.error', () => {
+    expect(console.error).toBe(originalConsoleError)
+  })
+})
+
+export {}

src/dom/index.ts

@@ -1,5 +1,7 @@
 import { autoRegisterCleanup } from '../core/cleanup'
+import { enableErrorOutputSuppression } from '../core/console'
 
 autoRegisterCleanup()
+enableErrorOutputSuppression()
 
 export * from './pure'