Capture intermediate updates when hook rerenders

[alaboudi]

What is your question:

Let's consider the following hook

const useCounter = () => {
    const [counter, setCounter] = React.useState(0);

    React.useEffect(() => {
        setCounter(1)
    }, [])

    return counter;
}

export default useCounter;

when running a test using RHTL, the following passes:

describe('useCounter', () => {
   it('should  provide the initial count', () => {
       const { result } = renderHook(() => useCounter());
       expect(result.current).toBe(1); // ----> this passes 🟢
   });
});

But this test does not capture the actual behavior of how a component consumes a hook during different render cycles

function App() {
  const counter = useCounter();
  console.log('counter', counter);
 return counter; // returns 0 during first render and then returns 1 during second render.
}

Is there a way to test the output of the first render using RHTL? I'm currently in a situation where I just fixed a bug related to variable initialization in a custom hook and it would be nice to be able to add a test to increase my code coverage.

[mpeyper]

It's not the prettiest, but you can do something like this

test('should capture all renders states of hook', () => {
  const results = [];
  const { result } = renderHook(() => {
    const counter = useCounter();
    results.push(counter);
    return counter;
  });

  expect(result.current).toBe(1);
  expect(results[0]).toBe(0);
  expect(results[1]).toBe(1);
  expect(results.length).toBe(2);
})

I wonder how common needing the intermediate states of a hook's render cycle is in general. Part of me feels like in the common case this is an implementation detail and testing for it will lead to brittle tests that break with every change to the hook's implementation, but for cases like this, where you want to write a test to prove an actually observed bug is now fixed, I feel like I want to provide better support for you.

I don't think it would be difficult for us to keep all render results in an array and surface it in the results container as something like result.previous (all but the result.current) or result.all (all including result.current), but I don't want to encourage people to necessarily test for each state transition that occurred while they were awaiting one of the async utils.

Happy to hear feedback from anyone on this.

[mpeyper]

FWIW, I had a bit of a play and it's a relatively small change required to get this test to pass:

import { renderHook } from 'src'

describe('result history tests', () => {
  let count = 0
  function useCounter() {
    const result = count++
    if (result === 2) {
      throw Error('expected')
    }
    return result
  }

  test('should capture all renders states of hook', () => {
    const { result, rerender } = renderHook(() => useCounter())

    expect(result.current).toEqual(0)
    expect(result.previous).toEqual([])
    expect(result.all).toEqual([0])

    rerender()

    expect(result.current).toBe(1)
    expect(result.previous).toEqual([0])
    expect(result.all).toEqual([0, 1])

    rerender()

    expect(result.error).toEqual(Error('expected'))
    expect(result.previous).toEqual([0, 1])
    expect(result.all).toEqual([0, 1, Error('expected')])

    rerender()

    expect(result.current).toBe(3)
    expect(result.previous).toEqual([0, 1, Error('expected')])
    expect(result.all).toEqual([0, 1, Error('expected'), 3])
  })
})

I'm still not sure it's a good idea. I also don't think we would provide both result.previous and result.all, but rather choose one. I'm thinking result.all is probably the more useful, but happy to hear thoughts and alternative names for either, or even alternative approaches.

[sarahatwork]

+1 on this - I often want to test the initial behavior of a hook before any rerendering, especially since this can be used to test SSR behavior.

[mpeyper]

@sarahatwork thanks for your input. I had not thought of this as being a solution to testing SSR behaviour, which is an interesting thought as I just closed #387 tonight because it wasn't really going anywhere.

What are your thoughts in regards to result.all vs. result.previous?

[sarahatwork]

@mpeyper result.all sounds good to me because it might come in handy for other uses too!

[pvandommelen]

result.all would be preferable as this would also allow verifying that a hook only uses a single rerender in situations where that is important by asserting its length.

Alternatively, something like result.allSincePreviousRender. Though I'd suspect that's harder to implement.

Also a +1 on this, for similar reasons as the parent. We have a hook where the output is related to the input in some way, and when it is out of sync the component would cause errors. Below I show a "faulty" implementation which we currently can't write tests for to guard against.

function useHook(key) {
  const [ state, setState ] = useState({[key]: true});
  useEffect(() => setState({[key]: true}, [ key ]);
  return state;
}

function Component() {
  someKey = ..;
  const state = useHook(someKey );
  const value = state[someKey] //key may not exist here
}

[mpeyper]

I've pushed up what I've got so far as a PR if anyone wants to continue it from here. Functionally I think it's fine, but it will need some documentation updates before it can be merged. I'm also happy if anyone want to have a go with their own implementation as well.

[github-actions]

🎉 This issue has been resolved in version 3.6.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀