Skip to content

Add utility for testing custom hooks #261

Closed

Description

@kentcdodds

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

import React, {useState} from 'react'
import {render, cleanup} from 'react-testing-library'

afterEach(cleanup)

function useCounter({initialCount = 0, step = 1} = {}) {
  const [count, setCount] = useState(initialCount)
  const increment = () => setCount(c => c + step)
  const decrement = () => setCount(c => c - step)
  return {count, increment, decrement}
}

function testHook(useHook, props) {
  const RenderProp = ({children, ...rest}) => children(useHook(rest))
  const returnVal = {}
  render(
    <RenderProp {...props}>
      {val => {
        // may need some special treatment if the
        // return value is not an object of values...
        Object.assign(returnVal, val)
        return null
      }}
    </RenderProp>,
  )
  return returnVal
}


// how to use this:
test('renders counter', () => {
  const data = testHook(useCounter)
  data.increment()
  expect(data.count).toBe(1)
  data.decrement()
  expect(data.count).toBe(0)
})

Describe alternatives you've considered:

  • Not providing the utility at all.
  • Calling it renderHook rather than testHook, or just about anything else. I'm not sure I like testHook...

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. With testHook 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 from testHook.

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.

Activity

  1. gnapse commented on Jan 7, 2019

    @gnapse
    Member

    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 useEffect and have no return values? Or even if they return something, they're also using a useEffect and possibly interacting with a ref. For instance, I already have some hooks to spy on scroll (they need a ref to the scroll container) and load more if scroll is down enough. Or custom hooks to imperatively re-apply focus to a given element on certain conditions.

    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 renderHook / testHook to test this use case" can become overwhelming. Unless I'm not seeing from your example how could it be applied to these other scenarios.

  2. kentcdodds commented on Jan 7, 2019

    @kentcdodds
    MemberAuthor

    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.

  3. kentcdodds commented on Jan 7, 2019

    @kentcdodds
    MemberAuthor

    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.

  4. gnapse commented on Jan 7, 2019

    @gnapse
    Member

    I'm sorry but I still do not follow you. What is sideEffect in your code snippet:

    // no need to handle the return value
    testHook(useSideEffect)
    expect(sideEffect).toHaveBeenCalledTimes(1)

    The .toHaveBeenCalledTimes expectation suggests is a function, but where did it come from that it gets called inside useSideEffect and then we can test that it was called indeed?

    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 values change as if on subsequent renders?

    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.

  5. kentcdodds commented on Jan 7, 2019

    @kentcdodds
    MemberAuthor

    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?

  6. gnapse commented on Jan 7, 2019

    @gnapse
    Member

    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 useState does). But also it assumes the hook receives all its arguments as a single object rest. But useEffect does not. And neither does your useLocalStorageState from youtube that I watched the other day.

    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? 🤔

  7. j-f1 commented on Jan 7, 2019

    @j-f1

    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.

  8. alexkrolick commented on Jan 7, 2019

    @alexkrolick
    Collaborator

    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

  9. kentcdodds commented on Jan 16, 2019

    @kentcdodds
    MemberAuthor

    I'm going to close this until we start seeing common enough patterns to warrant a utility.

  10. donavon commented on Jan 21, 2019

    @donavon
    Contributor

    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 react-proxy-hook which initially only supported objects as return values from Hooks. I've modified it to support any return value type.

    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.

    1. Kent implements some form of testHook or proxyHook (whatever the name) in the core library itself.
    2. There is a place in the world for this in a separate package (like mine).

    My package was never meant to last and was really only a POC. I'd much prefer to see this backed into react-testing-library directly, but if not, I'll keep using it.

    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 value (as I implemented it) with other types (i.e. string, numbers, etc).

    const returnedVal = useFooProxy();
    expect(returnedVal[0]).toBe(0);

    Just my 2¢

  11. alexkrolick commented on Jan 21, 2019

    @alexkrolick
    Collaborator

    @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
    }
  12. kentcdodds commented on Jan 21, 2019

    @kentcdodds
    MemberAuthor

    Interesting idea Alex! Hmm... What does everyone else think? I think I like it.

  13. donavon commented on Jan 22, 2019

    @donavon
    Contributor

    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 React as there is no JSX in the test itself.

    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 hookTester

    import 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:

    1. Note that for some reason, I need parens around the destructure statement when destructuring into an object (probably because it looks like a code block) which caused Prettier to enter the ;.

    2. The call to hookTester make it look a lot less like it does when calling the hook in the actual component. We're trading destructing for an odd setup.

    I've taken Kent's sample and ran a single test here on CodeSandbox if you'd like to play with it live.

  14. donavon commented on Jan 22, 2019

    @donavon
    Contributor

    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 hookTester

    import 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.

  15. alexkrolick commented on Jan 22, 2019

    @alexkrolick
    Collaborator

    👍 to the single callback especially for passing arguments to the hook

  16. donavon commented on Jan 22, 2019

    @donavon
    Contributor

    Now... What do we call this thing?

    • hookTester isn't catchy enough. Plus "tester"?
    • renderHook is synonymous with render. (maybe??)
    • testHook (Kent's original above)
    • proxyHook is what I used in my other package.

    Even though I'm not a super fan of using "render", I think renderHook will drive the point across that you use it to render (or use) the hook.

    p.s. Instead of a renderProp, I think we just invented a hookProp. 😆

  17. gnapse commented on Jan 22, 2019

    @gnapse
    Member

    testHook seems pretty straightforward and to the point, you can't get it simpler than that

  18. j-f1 commented on Jan 22, 2019

    @j-f1

    useHook?

  19. donavon commented on Jan 22, 2019

    @donavon
    Contributor

    I was going to put useHook in my list, but didn't want to be the one who said it first. It's just wrong.

  20. donavon commented on Jan 22, 2019

    @donavon
    Contributor

    Moving this along... #274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions