Skip to content

Commit b81fd04

Browse files
mpeypermpeyper
authored
feat: use error boundary to capture useEffect errors (#539)
Fixes #308
1 parent 008077c commit b81fd04

File tree

10 files changed

+130
-69
lines changed

10 files changed

+130
-69
lines changed

‎package.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@
4545
"@babel/runtime": "^7.12.5",
4646
"@types/react": ">=16.9.0",
4747
"@types/react-dom": ">=16.9.0",
48-
"@types/react-test-renderer": ">=16.9.0"
48+
"@types/react-test-renderer": ">=16.9.0",
49+
"filter-console": "^0.1.1",
50+
"react-error-boundary": "^3.1.0"
4951
},
5052
"devDependencies": {
5153
"@typescript-eslint/eslint-plugin": "^4.9.1",

‎src/dom/__tests__/errorHook.test.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,7 @@ describe('error hook tests', () => {
108108
})
109109
})
110110

111-
/*
112-
These tests capture error cases that are not currently being caught successfully.
113-
Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
114-
for more details.
115-
*/
116-
// eslint-disable-next-line jest/no-disabled-tests
117-
describe.skip('effect', () => {
111+
describe('effect', () => {
118112
test('should raise effect error', () => {
119113
const { result } = renderHook(() => useEffectError(true))
120114

‎src/dom/__tests__/resultHistory.test.ts

+14-9
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,39 @@
11
import { renderHook } from '..'
22

33
describe('result history tests', () => {
4-
let count = 0
5-
function useCounter() {
6-
const result = count++
7-
if (result === 2) {
4+
function useValue(value: number) {
5+
if (value === 2) {
86
throw Error('expected')
97
}
10-
return result
8+
return value
119
}
1210

1311
test('should capture all renders states of hook', () => {
14-
const { result, rerender } = renderHook(() => useCounter())
12+
const { result, rerender } = renderHook((value) => useValue(value), {
13+
initialProps: 0
14+
})
1515

1616
expect(result.current).toEqual(0)
1717
expect(result.all).toEqual([0])
1818

19-
rerender()
19+
rerender(1)
2020

2121
expect(result.current).toBe(1)
2222
expect(result.all).toEqual([0, 1])
2323

24-
rerender()
24+
rerender(2)
2525

2626
expect(result.error).toEqual(Error('expected'))
2727
expect(result.all).toEqual([0, 1, Error('expected')])
2828

29-
rerender()
29+
rerender(3)
3030

3131
expect(result.current).toBe(3)
3232
expect(result.all).toEqual([0, 1, Error('expected'), 3])
33+
34+
rerender()
35+
36+
expect(result.current).toBe(3)
37+
expect(result.all).toEqual([0, 1, Error('expected'), 3, 3])
3338
})
3439
})

‎src/helpers/createTestHarness.tsx

+44-21
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,65 @@
11
import React, { Suspense } from 'react'
2+
import { ErrorBoundary, FallbackProps } from 'react-error-boundary'
3+
import filterConsole from 'filter-console'
24

3-
import { RendererProps, WrapperComponent } from '../types/react'
5+
import { addCleanup } from '../core'
46

5-
import { isPromise } from './promises'
7+
import { RendererProps, WrapperComponent } from '../types/react'
68

7-
function TestComponent<TProps, TResult>({
8-
hookProps,
9-
callback,
10-
setError,
11-
setValue
12-
}: RendererProps<TProps, TResult> & { hookProps?: TProps }) {
13-
try {
14-
// coerce undefined into TProps, so it maintains the previous behaviour
15-
setValue(callback(hookProps as TProps))
16-
} catch (err: unknown) {
17-
if (isPromise(err)) {
18-
throw err
19-
} else {
20-
setError(err as Error)
9+
function suppressErrorOutput() {
10+
// The error output from error boundaries is notoriously difficult to suppress. To save
11+
// out users from having to work it out, we crudely suppress the output matching the patterns
12+
// below. For more information, see these issues:
13+
// - https://github.com/testing-library/react-hooks-testing-library/issues/50
14+
// - https://github.com/facebook/react/issues/11098#issuecomment-412682721
15+
// - https://github.com/facebook/react/issues/15520
16+
// - https://github.com/facebook/react/issues/18841
17+
const removeConsoleFilter = filterConsole(
18+
[
19+
/^The above error occurred in the <TestComponent> component:/, // error boundary output
20+
/^Error: Uncaught .+/ // jsdom output
21+
],
22+
{
23+
methods: ['error']
2124
}
22-
}
23-
return null
25+
)
26+
addCleanup(removeConsoleFilter)
2427
}
2528

2629
function createTestHarness<TProps, TResult>(
27-
rendererProps: RendererProps<TProps, TResult>,
30+
{ callback, setValue, setError }: RendererProps<TProps, TResult>,
2831
Wrapper?: WrapperComponent<TProps>,
2932
suspense: boolean = true
3033
) {
34+
const TestComponent = ({ hookProps }: { hookProps?: TProps }) => {
35+
// coerce undefined into TProps, so it maintains the previous behaviour
36+
setValue(callback(hookProps as TProps))
37+
return null
38+
}
39+
40+
let resetErrorBoundary = () => {}
41+
const ErrorFallback = ({ error, resetErrorBoundary: reset }: FallbackProps) => {
42+
resetErrorBoundary = () => {
43+
resetErrorBoundary = () => {}
44+
reset()
45+
}
46+
setError(error)
47+
return null
48+
}
49+
50+
suppressErrorOutput()
51+
3152
const testHarness = (props?: TProps) => {
32-
let component = <TestComponent hookProps={props} {...rendererProps} />
53+
resetErrorBoundary()
54+
55+
let component = <TestComponent hookProps={props} />
3356
if (Wrapper) {
3457
component = <Wrapper {...(props as TProps)}>{component}</Wrapper>
3558
}
3659
if (suspense) {
3760
component = <Suspense fallback={null}>{component}</Suspense>
3861
}
39-
return component
62+
return <ErrorBoundary FallbackComponent={ErrorFallback}>{component}</ErrorBoundary>
4063
}
4164

4265
return testHarness

‎src/helpers/promises.ts

+2-6
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,9 @@ function resolveAfter(ms: number) {
22
return new Promise<void>((resolve) => setTimeout(resolve, ms))
33
}
44

5-
export async function callAfter(callback: () => void, ms: number) {
5+
async function callAfter(callback: () => void, ms: number) {
66
await resolveAfter(ms)
77
callback()
88
}
99

10-
function isPromise<T>(value: unknown): boolean {
11-
return typeof (value as PromiseLike<T>).then === 'function'
12-
}
13-
14-
export { isPromise, resolveAfter }
10+
export { resolveAfter, callAfter }

‎src/native/__tests__/errorHook.test.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,7 @@ describe('error hook tests', () => {
108108
})
109109
})
110110

111-
/*
112-
These tests capture error cases that are not currently being caught successfully.
113-
Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
114-
for more details.
115-
*/
116-
// eslint-disable-next-line jest/no-disabled-tests
117-
describe.skip('effect', () => {
111+
describe('effect', () => {
118112
test('should raise effect error', () => {
119113
const { result } = renderHook(() => useEffectError(true))
120114

‎src/native/__tests__/resultHistory.test.ts

+14-9
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,39 @@
11
import { renderHook } from '..'
22

33
describe('result history tests', () => {
4-
let count = 0
5-
function useCounter() {
6-
const result = count++
7-
if (result === 2) {
4+
function useValue(value: number) {
5+
if (value === 2) {
86
throw Error('expected')
97
}
10-
return result
8+
return value
119
}
1210

1311
test('should capture all renders states of hook', () => {
14-
const { result, rerender } = renderHook(() => useCounter())
12+
const { result, rerender } = renderHook((value) => useValue(value), {
13+
initialProps: 0
14+
})
1515

1616
expect(result.current).toEqual(0)
1717
expect(result.all).toEqual([0])
1818

19-
rerender()
19+
rerender(1)
2020

2121
expect(result.current).toBe(1)
2222
expect(result.all).toEqual([0, 1])
2323

24-
rerender()
24+
rerender(2)
2525

2626
expect(result.error).toEqual(Error('expected'))
2727
expect(result.all).toEqual([0, 1, Error('expected')])
2828

29-
rerender()
29+
rerender(3)
3030

3131
expect(result.current).toBe(3)
3232
expect(result.all).toEqual([0, 1, Error('expected'), 3])
33+
34+
rerender()
35+
36+
expect(result.current).toBe(3)
37+
expect(result.all).toEqual([0, 1, Error('expected'), 3, 3])
3338
})
3439
})

‎src/server/__tests__/errorHook.test.ts

+1-7
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,7 @@ describe('error hook tests', () => {
119119
})
120120
})
121121

122-
/*
123-
These tests capture error cases that are not currently being caught successfully.
124-
Refer to https://github.com/testing-library/react-hooks-testing-library/issues/308
125-
for more details.
126-
*/
127-
// eslint-disable-next-line jest/no-disabled-tests
128-
describe.skip('effect', () => {
122+
describe('effect', () => {
129123
test('should raise effect error', () => {
130124
const { result, hydrate } = renderHook(() => useEffectError(true))
131125

‎src/server/__tests__/resultHistory.test.ts

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { renderHook } from '..'
2+
3+
describe('result history tests', () => {
4+
function useValue(value: number) {
5+
if (value === 2) {
6+
throw Error('expected')
7+
}
8+
return value
9+
}
10+
11+
test('should capture all renders states of hook', () => {
12+
const { result, hydrate, rerender } = renderHook((value) => useValue(value), {
13+
initialProps: 0
14+
})
15+
16+
expect(result.current).toEqual(0)
17+
expect(result.all).toEqual([0])
18+
19+
hydrate()
20+
21+
expect(result.current).toEqual(0)
22+
expect(result.all).toEqual([0, 0])
23+
24+
rerender(1)
25+
26+
expect(result.current).toBe(1)
27+
expect(result.all).toEqual([0, 0, 1])
28+
29+
rerender(2)
30+
31+
expect(result.error).toEqual(Error('expected'))
32+
expect(result.all).toEqual([0, 0, 1, Error('expected')])
33+
34+
rerender(3)
35+
36+
expect(result.current).toBe(3)
37+
expect(result.all).toEqual([0, 0, 1, Error('expected'), 3])
38+
39+
rerender()
40+
41+
expect(result.current).toBe(3)
42+
expect(result.all).toEqual([0, 0, 1, Error('expected'), 3, 3])
43+
})
44+
})

‎src/server/pure.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,12 @@ function createServerRenderer<TProps, TResult>(
2020
render(props?: TProps) {
2121
renderProps = props
2222
act(() => {
23-
const serverOutput = ReactDOMServer.renderToString(testHarness(props))
24-
container.innerHTML = serverOutput
23+
try {
24+
const serverOutput = ReactDOMServer.renderToString(testHarness(props))
25+
container.innerHTML = serverOutput
26+
} catch (e: unknown) {
27+
rendererProps.setError(e as Error)
28+
}
2529
})
2630
},
2731
hydrate() {

0 commit comments

Comments
 (0)