react-native-testing-library icon indicating copy to clipboard operation
react-native-testing-library copied to clipboard

`renderHook` result values with `waitFor` not working

Open kylebake opened this issue 3 years ago • 8 comments

Describe the bug

Using the new renderHook function included in this library does not behave the same way when it comes to awaiting different return values as the other implementations (referencing specifically react-hooks + testing-library/react).

Trying to waitFor different values to change from the hook directly does not seem to be functioning as expected.

Expected behavior

If https://github.com/testing-library/react-hooks-testing-library is going to be deprecated in favor of this library, I'd expect to retain the same functionality the previous library offered since it was created to test complex hooks without the need for testing the components using these hooks as well. I'd also expect similar functionality to the testing-library/react implementation.

waitFor should be able to handle waiting for the values of the hook to match the expectation passed to it

Steps to Reproduce

Consider the following hook:

const useCount = () => {
  const [count, setCount] = useState(0)

  useEffect(() => {
    const retrieveCount = async () => {
      await new Promise(resolve => {
        setTimeout(() => {
          resolve(undefined)
        }, 500)
      })

      setCount(10)
    }

    retrieveCount()
  }, [])

  return count
}

If we wanted to try and waitFor count to be 10, there doesn't appear to be a way to do this with testing-library/react-native as we can with the others:

  it('waits until count has been set', async () => {
    const { result } = renderHook(() => useCount())
   
    // this line will always fail. `result.current` is always 0, no matter the options/rerendering/etc
    await waitFor(() => expect(result.current).toBe(10))

    // this syntax will also seemingly not do anything, which worked previously in the react-hooks library
    await waitFor(() => result.current === 10)

    expect(true).toBeTrue()
  })

The above example will work just fine in testing-library/react, however.

Screenshots

Versions

kylebake avatar Aug 01 '22 21:08 kylebake

@kylebake RNTL implementation of both renderHook & waitFor closely matches React/Dom Testing Library one (essentially it probably is copy+paste+tweak), with minimal changes due to using React Test Renderer instead of React DOM renderer and not using having Js DOM objects like document.

If you have a bit of spare time and interest, you could try running diff for renderHook and waitFor source files between RNTL & RTL maybe this will shed some light why RTL's version is working and RNTL's version is not.

mdjastrzebski avatar Aug 01 '22 21:08 mdjastrzebski

Happy to help out and dig more into it 👍 I'm in the process of upgrading react-native for our app, which is why I'm uncovering some of this. I'll let you know if I find anything over this week

kylebake avatar Aug 01 '22 21:08 kylebake

A bit more info while I'm still investigating: it definitely appears to be that renderHook isn't rerendering the hook when asynchronous tasks happen. How I tested this theory:

If you take the example hook from my first comment, and test it with this code, result.current is always 0:

    const { result, rerender } = setupHook()

    setInterval(() => {
      console.log(`result: ${result.current}`)
    }, 1000)
    await waitFor(() => expect(result.current).toBe(10), { timeout: 15000 })

However, running the following test, waitFor handles the situation as I would expect and passes after 3s:

    let count = 0

    setTimeout(() => {
      count = 10
    }, 3000)
    await waitFor(() => expect(count).toBe(10), { timeout: 15000 })

I'm going to focus my attention on renderHook for the time being, as it seems to be the root issue

kylebake avatar Aug 03 '22 14:08 kylebake

@kylebake I think the issue here is not with the renderHook implementation but rather that waitFor isn't able to wait for promises using timers, these promises are never resolved (see this other issue #1067). To fix it you can use fake timers, run the pending timer and then use waitFor to resolve the promise. Regarding waitFor not being able to handle this situation I'm not sure what's causing this but I wonder if it's not rather a jest issue

pierrezimmermannbam avatar Aug 19 '22 20:08 pierrezimmermannbam

@pierrezimmermannbam do you mean that setTimeout (and setInterval) never gets called when not real timers? Or that they get called but waitFor is somehow not able to observe the results of them being called. Either way, that sounds like a serious bug.

mdjastrzebski avatar Aug 19 '22 21:08 mdjastrzebski

The promises are resolving in my use cases, I'm able to set breakpoints and see the code get past the async logic. The tests don't seem to be able to see this, however. I'd also push back on it being a jest issue since I'm able to get all of this working without any code/jest changes if I use testing-library/react.

A super hacky workaround that I've found is to continuously update the component/hook in the test and only then can the waitFor eventually see the assertion. My working example right now:

  const hook = renderHook(() => useCount())
  const timer = setInterval(() => {
    hook.rerender()
  }, 500)
 
  // now this waitFor will correctly see the result change to 10
  await waitFor(() => expect(hook.result.current).toBe(10)) 

Sidenote: I can spend some more time on this issue this weekend, been busy with getting our react native version upgraded to unblock us. I'll get a small repo stood up as well so it's easier for us to test against.

kylebake avatar Aug 23 '22 14:08 kylebake

Please try this with @testing-library/react-hooks

it('test', async () => {
  const { result, waitForNextUpdate } = renderHook(() => useHook());

  act(() => {
    result.current.fetch();
  });
  expect(result.current.state).toBe(undefined);

  await waitForNextUpdate();
  expect(result.current.state).toBe('GOOD'); //or at least "BAD"
});

tinahir avatar Sep 14 '22 15:09 tinahir

@kylebake are you using react 18 ? Promises mixed with useEffect do not work when using react 18, this issue is being discussed in #1093 and #1067 is another duplicate for the same issue. I think this will be fixed but in the meantime I believe your test case should work if you use fake timers

pierrezimmermannbam avatar Sep 14 '22 16:09 pierrezimmermannbam

@kylebake can you confirm if this issue persists with RNTL v11.2.0?

mdjastrzebski avatar Sep 26 '22 12:09 mdjastrzebski

Looks like I'm no longer able to reproduce the issue with v11.2.0, thanks for all the collaboration on this! Apologies for my slow responses, I'll try to help out more moving forward with this repo if I can

kylebake avatar Sep 26 '22 15:09 kylebake

Awesome, thanks for checking that @kylebake

mdjastrzebski avatar Sep 26 '22 16:09 mdjastrzebski

Looks like I'm no longer able to reproduce the issue with v11.2.0, thanks for all the collaboration on this! Apologies for my slow responses, I'll try to help out more moving forward with this repo if I can

I'm still reproducing the problem in v11.2.0!

lubbo avatar Oct 12 '22 13:10 lubbo

@lubbo could you please post a repro using https://github.com/callstack/react-native-testing-library/tree/main/examples/basic please? Without that, we can't figure out what is happening, since the issue has been solved for most (it seems) users.

Are you using react 18 by the way? Because there could still be problem with older versions of react, but I'm not sure we'll be able to prioritise work to fix them.

AugustinLF avatar Oct 12 '22 14:10 AugustinLF

Hi @AugustinLF here is my repro using the basic example: https://github.com/bkdev98/rn-testing-lib-waitfor-issue. The tests/asyncHooks.spec.tsx failed and waitFor does not await for new state to be updated. https://github.com/bkdev98/rn-testing-lib-waitfor-issue/blob/main/tests/asyncHooks.spec.tsx Test result: Screen Shot 2022-11-02 at 13 05 43

bkdev98 avatar Nov 02 '22 06:11 bkdev98

@bkdev98 you should use waitFor with an expectation, i.e. something that throws when the expectation is failed. Returning false does not work, as waitFor considers that a legit return value.

That should be relatively easy to fix:

await waitFor(() => expect(result.current).toBeTruthy());

@bkdev98 if that does not fix the issue for you, please adjust your code to use except as shown above and create a new GH issues for your case.

mdjastrzebski avatar Nov 02 '22 08:11 mdjastrzebski

@bkdev98 I made the following changes in your test and it now works

describe("useProductDetail", () => {
  beforeEach(() => {
    jest.useFakeTimers();
  });

  test("should instantly show initial product data", async () => {
    jest.useFakeTimers();

    const { result } = renderHook(() => useTestAsyncHook());

    await waitFor(() => expect(result.current).toBe(true), { timeout: 2500 });

    expect(result.current).toEqual(true);
  });
});

As @mdjastrzebski mentioned the callback provided to waitFor should throw when the expectation is not met and also since you have a setTimeout of 2000ms, you need to increase the waitFor timeout which is of 1000ms per default so that fake timers are advanced by 2000ms, else it will timeout after 1000s

pierrezimmermannbam avatar Nov 02 '22 08:11 pierrezimmermannbam

We probably should revisit the documentation on waitFor, it says that the default timeout is 4500ms which is false and it does not clearly state that the expectation should throw so that it may work. Also I think it would be nice to mention that it behaves differently when using fake timers

pierrezimmermannbam avatar Nov 02 '22 08:11 pierrezimmermannbam

It works!! Thank you for your help! ❤️

bkdev98 avatar Nov 02 '22 08:11 bkdev98

@pierrezimmermannbam would you have time to update waitFor documentation around timeout and making more explicit how the API works (throwing vs boolean)?

mdjastrzebski avatar Nov 02 '22 09:11 mdjastrzebski

@mdjastrzebski sure I'll try to submit a pr by the end of the week

pierrezimmermannbam avatar Nov 02 '22 10:11 pierrezimmermannbam