Replace core code with testHook from react-testing-library

[mpeyper]

So my review found that useHook and testHook offered practically identical features. The testHook implementation is simpler than what I had in useHook so I took that as the base for moving forward.

I made one change to testHook to allow the callback to take props (similar touseHook). In my opinion, this offers a slightly nicer story for updating the values passed to the hook to test how the result changed when inputs changed. It's also backwards compatible with the old testHook so whatever people were doing before will continue to work when migrating across.

I'm also not a big fan of the options.wrapper usage. Having to wrap a context provider in order to pass the value to it seemed clunky in practice. Even the tests in react-testing-library had to turn off lint warnings when using it. I'll sleep on it and see if I can't come up with something a little bit more ergonomic for it but I'll probably keep options.wrapper regardless for backwards compatibility reasons, at least to begin with.

Other than that, I'm pretty happy with this.

Resolves #1

[alexkrolick]

Looks good to me 👍

• Would you mind pulling in the unit tests from RTL for testHook?
• Can you show the usage of the callback argument you mentioned?

[mpeyper]

Dropped those tests in and they worked without any changes (other than formatting to match my prettier config).

For changing the props, I added a new option called initialProps that gets passed into the callback which can be changed when calling rerender.

e.g. given the hook

const sideEffect = { '1': false, '2': false }

function useSideEffect(id) {
  useEffect(() => {
    sideEffect[id] = true
    return () => {
      sideEffect[id] = false
    }
  }, [id])
}

You can change the id in your test like so

test('should update side effect when id changes', () => {
  const { rerender, unmount } = testHook(
    ({ id }) => useSideEffect(id)),
    { initialProps: { id: '1' } }
  )

  expect(sideEffect['1']).toBe(true)
  expect(sideEffect['2']).toBe(false)

  rerender({ id: '2' })

  expect(sideEffect['1']).toBe(false)
  expect(sideEffect['2']).toBe(true)

  unmount()

  expect(sideEffect['1']).toBe(false)
  expect(sideEffect['2']).toBe(false)
})

[Maximilianos]

Hey @mpeyper thanks for putting this together, I started using this branch in a project of mine and all is 👍 at the moment.

Minor naming comment: what do you think of calling the method renderHook (or even render) instead of testHook to be closer to the render method of react-testing-library and be closer to the rerender method that is returned from testHook currently?

[mpeyper]

This branch actually renamed it from useHook which was a fun riff on the hook naming convention.

I also find testHook to feel a bit off, but the intention here was for 1-1 compatibility with react-testing-library.

I would consider an alias export though as I do like your suggestion.

[Maximilianos]

Yeah, I saw that it was useHook before. Great riff 😄! but also suffers from the same issues with testHook regarding the differences in naming and rerender. Given the testHook feature in RTL was never really publicized I assume it was kind of in beta, so the name could still have changed?

Maybe @alexkrolick or @donavon have stronger opinions on this?

I anyway now do something like:

function renderHook(initialProps = {}) {
  return testHook(({ query }) => useMovieSearch(query), { initialProps })
}

so, it's up to you 😄

[mpeyper]

with useHook, there wasn't the naming conflict because it would just rerender on each getCurrentValue() calls. I prefer the new sematics, but agree that renderHook feels better.

I'm going to change the export to be renderHook and create a testHook that looks something like

function testHook(...params) {
  console.warn('testHook is deprecated.  Please use renderHook instead.')
  return renderHook(...params)
}

[mpeyper]

Ok, I'm going to merge and publish this v0.3.0.

I'm still not a fan of the wrapper API, but nothing I do will be backwards compatible, so I'll leave it as is for now and tackle it another day. I'll open an RFC for it and see if anyone else has ideas for it.