-
Notifications
You must be signed in to change notification settings - Fork 232
Added error handling in hook callback #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
import { useState, useEffect } from 'react' | ||
import { renderHook } from 'src' | ||
|
||
describe('error hook tests', () => { | ||
function useError(throwError) { | ||
if (throwError) { | ||
throw new Error('expected') | ||
} | ||
return true | ||
} | ||
|
||
const somePromise = () => Promise.resolve() | ||
|
||
function useAsyncError(throwError) { | ||
const [value, setValue] = useState() | ||
useEffect(() => { | ||
somePromise().then(() => { | ||
setValue(throwError) | ||
}) | ||
}, [throwError]) | ||
return useError(value) | ||
} | ||
|
||
describe('synchronous', () => { | ||
test('should raise error', () => { | ||
const { result } = renderHook(() => useError(true)) | ||
|
||
expect(() => { | ||
expect(result.current).not.toBe(undefined) | ||
}).toThrow(Error('expected')) | ||
}) | ||
|
||
test('should capture error', () => { | ||
const { result } = renderHook(() => useError(true)) | ||
|
||
expect(result.error).toEqual(Error('expected')) | ||
}) | ||
|
||
test('should not capture error', () => { | ||
const { result } = renderHook(() => useError(false)) | ||
|
||
expect(result.current).not.toBe(undefined) | ||
expect(result.error).toBe(undefined) | ||
}) | ||
|
||
test('should reset error', () => { | ||
const { result, rerender } = renderHook((throwError) => useError(throwError), { | ||
initialProps: true | ||
}) | ||
|
||
expect(result.error).not.toBe(undefined) | ||
|
||
rerender(false) | ||
|
||
expect(result.current).not.toBe(undefined) | ||
expect(result.error).toBe(undefined) | ||
}) | ||
}) | ||
|
||
describe('asynchronous', () => { | ||
test('should raise async error', async () => { | ||
const { result, waitForNextUpdate } = renderHook(() => useAsyncError(true)) | ||
|
||
await waitForNextUpdate() | ||
|
||
expect(() => { | ||
expect(result.current).not.toBe(undefined) | ||
}).toThrow(Error('expected')) | ||
}) | ||
|
||
test('should capture async error', async () => { | ||
const { result, waitForNextUpdate } = renderHook(() => useAsyncError(true)) | ||
|
||
await waitForNextUpdate() | ||
|
||
expect(result.error).toEqual(Error('expected')) | ||
}) | ||
|
||
test('should not capture async error', async () => { | ||
const { result, waitForNextUpdate } = renderHook(() => useAsyncError(false)) | ||
|
||
await waitForNextUpdate() | ||
|
||
expect(result.current).not.toBe(undefined) | ||
expect(result.error).toBe(undefined) | ||
}) | ||
|
||
test('should reset async error', async () => { | ||
const { result, waitForNextUpdate, rerender } = renderHook( | ||
(throwError) => useAsyncError(throwError), | ||
{ | ||
initialProps: true | ||
} | ||
) | ||
|
||
await waitForNextUpdate() | ||
|
||
expect(result.error).not.toBe(undefined) | ||
|
||
rerender(false) | ||
|
||
await waitForNextUpdate() | ||
|
||
expect(result.current).not.toBe(undefined) | ||
expect(result.error).toBe(undefined) | ||
}) | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any other test cases or usage patterns we'd want to to see being tested? |
||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,8 @@ export function renderHook<P, R>( | |
} & RenderOptions | ||
): { | ||
readonly result: { | ||
current: R | ||
readonly current: R, | ||
readonly error: Error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, technically you can throw anything, so maybe this should be a bit looser? Also, typescript isn't really my thing, so not sure if |
||
} | ||
readonly waitForNextUpdate: () => Promise<void> | ||
readonly unmount: RenderResult['unmount'] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically you can throw anything (not just an
Error
instance), so I'm not sure if this should beany
as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not knowledgeable enough with typescript, but I think even if someone would throw, it would be an instance of
Error
as they would extendError
to create a custom error in case they wanted to. So, I think this should be fine unless someone with knowledge of typescript differs in opinion.