-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add utility for testing custom hooks #261
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
Comments
Sounds good, but I'm still skeptical that is can be advertised as a general solution to test custom hooks. What about custom hooks that only Not saying this invalidates the idea, but advertising this as something to test custom hooks in general may raise expectations too much, and the flood of questions about "how do I use |
Those are great points @gnapse. If the hook does side-effects though, even if you're testing a regular component, you're going to have to mock the side-effects and you could do the same with this thing: // no need to handle the return value
testHook(useSideEffect)
expect(sideEffect).toHaveBeenCalledTimes(1) So I'm pretty sure this will only make those kinds of things easier as well. |
My biggest concern is that people will start testing hooks that are neither complex nor reusable in isolation. That's where docs will need a very big message indicating how this should be used. |
I'm sorry but I still do not follow you. What is // no need to handle the return value
testHook(useSideEffect)
expect(sideEffect).toHaveBeenCalledTimes(1) The Or perhaps let's work with an actual-but-simplified example. Take this hook, for instance: function useRefocus(ref, values) {
useEffect(() => {
if (ref.current && typeof ref.current.focus === 'function') {
ref.current.focus();
}
}, values);
} How do you test not only that it works on the first run, but also provide stimulus to make those Maybe I'm too much into the "create a test-only component to wrap the hook" way of thinking 😄 and I'm not the target user of this feature. |
Good point. That's a good example of a hook that this wouldn't really serve well I think... Can you think of a way that we could simplify testing a hook like that as well? What's the typical way that you test that kind of hook? |
I'll try to think about it, but some additional thoughts. This surfaces yet another problem with the suggested implementation. In it, you already hit the issue of what to do if the returned value from the hook is not an object (like what if it's an array like Actually, I think that one would be a very good basis for a proof-of-concept. It has effects and state, receives more than one arg and returns an array instead of an object. Plus it's as reusable and test-worthy as it can be. If we can make it work with that one, I'd say we're good to go. I'll see if I can carve some time, unless you beat me to it, which you probably will 😄. PS: How does react test the built-in hooks BTW? 🤔 |
How about this API? const counter = testHook(() => useCounter()) For simple hooks like this one, you can just pass the hook function, but it also makes it easy to pass custom parameters. |
Can't find it in the docs so it's probably in the RFC, but the React team said they would provide a way to test hooks in react-test-utils. Without that or a testHook helper here, using jest.spy or jest.fn mocks could work: test('my hook', () => {
const check = jest.fn()
function HookComp() {
const [x, setX] = myHook()
check(x, setX)
return null
}
render(<HookComp />)
expect(check).toHaveBeenCalledWith(y)
// ...
} It seems like there is a lot of variance in use cases and capabilities for hooks |
I'm going to close this until we start seeing common enough patterns to warrant a utility. |
Hello all. Kent pointed me to this issue and I thought I'd join in. I had a discussion with @kentcdodds on some tests that I was writing and he pointed me to his YouTube video on testing hooks. Based on his work, I published a general purpose package called You use it by passing in your Hook. I return a "proxy hook" that you can use—a HOH (Higher Order Hook) if you will. Example: import { proxyHook, cleanup } from 'react-proxy-hook';
import useCounter from '../use-counter';
const useCounterProxy = proxyHook(useCounter);
afterEach(cleanup);
test('useCounter', () => {
const counterData = useCounterProxy();
counterData.increment();
expect(counterData.count).toBe(1);
counterData.decrement();
expect(counterData.count).toBe(0);
}); I've done a lot of refactoring of some tests in this PR and as you can see, this method turned out extremely useful for code readability. The way I see it, this could go one of a few ways.
My package was never meant to last and was really only a POC. I'd much prefer to see this backed into I know there is the "you can't destructure" caveat, but it's not too bad with object and a small price to pay for the massive benefit that it provides. With arrays, it's a little odd, but that too makes sense. const returnedVal = useFooProxy();
expect(returnedVal[0]).toBe(0); And you must use something like const returnedVal = useFooProxy();
expect(returnedVal[0]).toBe(0); Just my 2¢ |
@donavon I like the idea. It's a clean API. My concern is the caveat you mentioned - you can't destructure the result, which looks different than the standard way of consuming the hook so it will likely cause some people to struggle with a strange error. This is especially true for array results, which are "idiomatic" if you mimic the built-in hook APIs. It also does obscure the fact that hooks only work inside component renders 🤔 A more explicit API could look like this: import {HookTester, render} from 'react-testing-library'
import myCustomHook from '../my-custom-hook'
test('my hook', () => {
let count, setCount
render(
<HookTester
hook={myCustomHook}
handleResult={result => ([count, setCount] = result)}
/>
)
expect(count).toBe(0)
setCount(2)
expect(count).toBe(2)
} Yes, you can update values with destructuring% node
> let x, y
undefined
> [x, y] = [1, () => x++]
[ 1, [Function] ]
> y()
1
> x
2 Note: the created [] or {} must be used inline or returned (implicitly is OK like in the REPL) Implementation of HookTester function HookTester({hook, handleResult}) {
handleResult(hook())
return null
} |
Interesting idea Alex! Hmm... What does everyone else think? I think I like it. |
Interesting. I like the idea. But IMO there's too much boilerplate in the test itself. What if we abstracted some of it away like this (below), passing the hook, the args to pass to the hook, and the callback handler. This also eliminates the need to import import { cleanup, hookTester } from 'react-testing-library'
import useCounter from '../use-counter'
afterEach(cleanup)
test('useCounter', () => {
let count, increment
hookTester(useCounter, [{initialCount: 2}], result => {
;({count, increment} = result)
})
expect(count).toBe(2)
increment()
expect(count).toBe(3)
}) Implementation of hookTesterimport React from 'react'
import {render} from 'react-testing-library'
function HookTester({ hook, args, handler }) {
handler(hook(...args));
return null;
}
const hookTester = (hook, args, handler) => {
render<HookTester hook={hook} args={args} handler={handler} />;
};
export default hookTester; This method is not without issues either. The weirdities that I see are:
I've taken Kent's sample and ran a single test here on CodeSandbox if you'd like to play with it live. |
We're over thinking this (also on CodeSandbox) test('useCounter', () => {
let count, increment
hookTester(() => ({count, increment} = useCounter({initialCount: 2})))
expect(count).toBe(2)
increment()
expect(count).toBe(3)
}) Implementation of hookTesterimport React from 'react'
import {render} from 'react-testing-library'
function HookTester({callback}) {
callback()
return null
}
const hookTester = callback => {
render(<HookTester callback={callback} />)
}
export default hookTester Super simple. |
👍 to the single callback especially for passing arguments to the hook |
Now... What do we call this thing?
Even though I'm not a super fan of using "render", I think p.s. Instead of a |
|
|
I was going to put |
Moving this along... #274 |
* docs: update README.md * docs: update .all-contributorsrc Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Describe the feature you'd like:
I think it would be great to add a utility to make testing custom hooks easier. Right now, I recommend making a render prop component out of the hook and testing that component. I think we could do the same from within react-testing-library.
Suggested implementation:
Here's a working implementation/example
Describe alternatives you've considered:
renderHook
rather thantestHook
, or just about anything else. I'm not sure I liketestHook
...Teachability, Documentation, Adoption, Migration Strategy:
One really important consideration is that with
render
we recommend that people destructure just what they need rather than assigning the return value an actual variable name. WithtestHook
however, if you destructure the return value it can have some pretty confusing issues (relating to the way JavaScript closures work, I explain it in the video I linked above). So all docs will need to call this out specially and all examples should not destructure anything fromtestHook
.We should also make sure to document the fact that this should typically only be used for hooks that either have a lot of edge cases that need special unit testing or highly reused hooks. Typically custom hooks should just be covered by testing the components that use them.
The text was updated successfully, but these errors were encountered: