React-testing-library: Return value of hook run with testHook?

Created on 12 Feb 2019  路  7Comments  路  Source: testing-library/react-testing-library

Honestly, it got me confused big time. Considering that most of the tests might actually want to validate the output of the custom hook I find it strange we need to do this.

  test('accepts a default initial value for `count`', () => {
    let count
    testHook(() => ({count} = useCounter({})))

    expect(count).toBe(0)
  })

So I would like to propose getting the output directly.

  test('accepts a default initial value for `count`', () => {
    const { result } = testHook(() => useCounter({}))

    expect(result.count).toBe(0)
  })

It can be compared to a render where we are getting container (and bunch of getters) to write our expectations. The testHook is kinda inconsistent in this.

released

Most helpful comment

Ah, I see, that makes sense. Couldn't you do:

  test('accepts a default initial value for `count`', () => {
    const ref = {current: null}
    testHook(() => (ref.current = useCounter({})))

    expect(count).toBe(0)
  })

I guess I can see how that could be annoying. I'd be fine with returning a ref so long as the existing solution still works.

All 7 comments

See https://github.com/kentcdodds/react-testing-library/pull/274#discussion_r250786841 for why this doesn't work - it's due to closures

Ok, in that case, we can do what I already mentioned somewhere else - return a getter.

  test('accepts a default initial value for `count`', () => {
    const { getResult } = testHook(() => useCounter({}))
    const { count } = getResult()
    expect(count).toBe(0)
  })

Alternatively, the useRef approach with a result.current, but personally, I would prefer a getter.

Ok, I have prepared a proof of concept that both approaches work.

https://codesandbox.io/s/209nj999jy?module=%2Fsrc%2FtestHook.test.js&previewwindow=tests

test("using a getter", () => {
  const { getResult } = testHook(() => useCounter({ step: 2 }));
  let { count, increment } = getResult();
  expect(count).toBe(0);
  act(() => {
    increment();
  });
  const { count: count2 } = getResult();
  expect(count2).toBe(2);
});
test("using a result.current", () => {
  const { result } = testHook(() => useCounter({ step: 2 }));
  expect(result.current.count).toBe(0);
  act(() => {
    result.current.increment();
  });
  expect(result.current.count).toBe(2);
});

Looking at it now I might move even more inclined to result.current considering naming clashes when destructuring from the getter.

I'm really struggling to understand how this is better than the current solution/example...

  • It's much easier with TypeScript, because let without an initial value has no clue about the type that will come later (maybe)
  • Imo it feels more natural to work with return value instead of hacking it inside the callback
  • It's closer to API of render
  • It would be optional, anyone can still do it the current way.

Ah, I see, that makes sense. Couldn't you do:

  test('accepts a default initial value for `count`', () => {
    const ref = {current: null}
    testHook(() => (ref.current = useCounter({})))

    expect(count).toBe(0)
  })

I guess I can see how that could be annoying. I'd be fine with returning a ref so long as the existing solution still works.

:tada: This issue has been resolved in version 5.8.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

albert-olive picture albert-olive  路  3Comments

jaredmeakin picture jaredmeakin  路  3Comments

julienw picture julienw  路  4Comments

chasen-bettinger picture chasen-bettinger  路  3Comments

NiGhTTraX picture NiGhTTraX  路  3Comments