From 6ffcc84a8b2acf0d2e179804af754323022267d5 Mon Sep 17 00:00:00 2001 From: Michael Peyper Date: Thu, 14 Jan 2021 00:35:33 +1100 Subject: [PATCH] feat: use error boundary to capture useEffect errors Fixes #308 --- package.json | 4 +- src/dom/__tests__/errorHook.test.ts | 8 +-- src/dom/__tests__/resultHistory.test.ts | 23 +++++--- src/helpers/createTestHarness.tsx | 65 +++++++++++++++------- src/helpers/promises.ts | 8 +-- src/native/__tests__/errorHook.test.ts | 8 +-- src/native/__tests__/resultHistory.test.ts | 23 +++++--- src/server/__tests__/errorHook.test.ts | 8 +-- src/server/__tests__/resultHistory.test.ts | 44 +++++++++++++++ src/server/pure.ts | 8 ++- 10 files changed, 130 insertions(+), 69 deletions(-) create mode 100644 src/server/__tests__/resultHistory.test.ts diff --git a/package.json b/package.json index 729ac612..80d224fb 100644 --- a/package.json +++ b/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 --git a/src/dom/__tests__/errorHook.test.ts b/src/dom/__tests__/errorHook.test.ts index 7fb7a265..c7f21847 100644 --- a/src/dom/__tests__/errorHook.test.ts +++ b/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 --git a/src/dom/__tests__/resultHistory.test.ts b/src/dom/__tests__/resultHistory.test.ts index 1da5e94b..69ce2408 100644 --- a/src/dom/__tests__/resultHistory.test.ts +++ b/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 --git a/src/helpers/createTestHarness.tsx b/src/helpers/createTestHarness.tsx index 2a276107..0d1d4838 100644 --- a/src/helpers/createTestHarness.tsx +++ b/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({ - hookProps, - callback, - setError, - setValue -}: RendererProps & { 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 component:/, // error boundary output + /^Error: Uncaught .+/ // jsdom output + ], + { + methods: ['error'] } - } - return null + ) + addCleanup(removeConsoleFilter) } function createTestHarness( - rendererProps: RendererProps, + { callback, setValue, setError }: RendererProps, Wrapper?: WrapperComponent, 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 = + resetErrorBoundary() + + let component = if (Wrapper) { component = {component} } if (suspense) { component = {component} } - return component + return {component} } return testHarness diff --git a/src/helpers/promises.ts b/src/helpers/promises.ts index 632a513c..2fa89e5f 100644 --- a/src/helpers/promises.ts +++ b/src/helpers/promises.ts @@ -2,13 +2,9 @@ function resolveAfter(ms: number) { return new Promise((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(value: unknown): boolean { - return typeof (value as PromiseLike).then === 'function' -} - -export { isPromise, resolveAfter } +export { resolveAfter, callAfter } diff --git a/src/native/__tests__/errorHook.test.ts b/src/native/__tests__/errorHook.test.ts index 7fb7a265..c7f21847 100644 --- a/src/native/__tests__/errorHook.test.ts +++ b/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 --git a/src/native/__tests__/resultHistory.test.ts b/src/native/__tests__/resultHistory.test.ts index 1da5e94b..69ce2408 100644 --- a/src/native/__tests__/resultHistory.test.ts +++ b/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 --git a/src/server/__tests__/errorHook.test.ts b/src/server/__tests__/errorHook.test.ts index 10a489ab..f7977465 100644 --- a/src/server/__tests__/errorHook.test.ts +++ b/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 --git a/src/server/__tests__/resultHistory.test.ts b/src/server/__tests__/resultHistory.test.ts new file mode 100644 index 00000000..5f2f8b9c --- /dev/null +++ b/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 --git a/src/server/pure.ts b/src/server/pure.ts index b8a40055..3cd25a9d 100644 --- a/src/server/pure.ts +++ b/src/server/pure.ts @@ -20,8 +20,12 @@ function createServerRenderer( 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() {