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

Dependency injection in hooks possibly causing `act` warnings and `waitFor` possibly not working as intended

Open louis-young opened this issue 3 years ago • 2 comments

I've used Testing Library for a while and absolutely love it; I think it's a fantastic library so firstly a big thank you. I should preface this by saying that I'm not entirely convinced that this is a "bug" as such. I think there's a possibility that something has changed under the hood, requiring changes to our specific tests. I'd be interested to find out either way 🙂

  • @testing-library/react version: 13.3.0

  • Testing Framework and version: Jest version 27.5.1

  • DOM Environment: Jest DOM version 5.16.5 with JSDOM version 27.5.1

  • React version: 18.2.0

  • React DOM version: 18.2.0

(these dependency versions are from the latest Create React App with the TypeScript template)

Relevant code or config:

Please see the reproductions.

What you did:

We are attempting to upgrade to React version 18 and React Testing Library version 13.

What happened:

It seems that with the latest dependency versions, some unit tests (specifically it seems to be context consumer hooks and custom hooks with dependency injection that perform a state update in an effect) fail for one of two reasons:

  1. An act warning is reported in the console, even if no action is explicitly taking place in the test. We can make this disappear by wrapping waitFor in act (please see the reproductions).

  2. The test fails, expecting the value to be x but receiving y, even though there's a waitFor before the assertion ensuring the value is x (the waitFor is successfully awaited and the test fails on the expect assertion).

Reproduction:

I've created two basic examples for each dependency version. One is a simple context provider that receives an asynchronous function as a prop that is being tested through a consumer hook (useWhateverContext). The other is a simple custom hook that also receives an asynchronous function as a parameter.

There are effects in both of these examples that cause a state update to be made shortly after rendering.

I've added comments to the implementations of both the context provider and hook documenting the behaviour (and an effect that uses the dependency injected function and one that doesn't) and have commented out an act wrapped version of the waitFor in the tests. Hopefully this is helpful 😅

Not working as expected

React version 18 with React Testing Library version 13: https://codesandbox.io/s/react-18-testing-library-investigation-fs0vn1?file=/src/context/test/index.tsx

Working as expected

React version 17 with React Testing Library version 12: https://codesandbox.io/s/react-17-testing-library-investigation-chch96?file=/src/context/test/index.tsx

Problem description:

None of this happened with the older dependency versions (please see the reproductions) so I'm unsure if we're doing something wrong or if there's some change we need to make. Please note that I'm aware that these are contrived examples but this is because they're minimal reproducible examples that have been taken out of the context of our application. I'm aware that there are often better ways to test context consumer hooks and other custom React hooks (such as exercising them through the component(s) that render them) but there are some valid cases for doing this. I'm interested in solving the problem here and hopefully not having to circumvent it by changing the testing approach.

Suggested solution:

I wonder if this has something to do with the recent changes around act with the waitFor utility, or possibly the waitFor utility being decoupled from the renderHook API (as it's no longer returned from renderHook). I don't have any real context of the Testing Library internals and I'm not particularly up-to-date with the changes that have been made so these are just some guesses really.

This is possibly just something that we're doing wrong, but I'd be interested to understand what has changed in the latest versions to cause this behaviour.

Thank you for your time 🙂

louis-young avatar Aug 25 '22 08:08 louis-young

We are struggling with this as well when trying to upgrade to those versions.

amy-mac avatar Sep 09 '22 18:09 amy-mac

Hi @kentcdodds, @eps1lon, @alexkrolick, @MatanBobi, @MichaelDeBoey

Apologies for the tags, I wasn't sure who actively maintains React Testing Library so I've tagged the first few contributors I could see on GitHub.

This issue is approaching a month old now and I was wondering if there was a process in place for triaging/investigating issues? I can see that newer issues have been triaged/answered/resolved which made me wonder if this one had slipped through the net. I appreciate that this is an (amazing) free library and that you maintain it in your spare time voluntarily so firstly thank you for that; this isn't intended to come across as an entitled "why isn't this fixed yet" but more of a question as to whether this will be looked at and possibly fixed, or whether we should explore other options to resolve/work around it.

Thanks for your time and contributions to this fantastic library 🙂

louis-young avatar Sep 20 '22 16:09 louis-young

I'm not personally maintaining Testing Library very actively, but I just thought I'd say part of the reason you're probably not getting a response here is because it looks like your question is harder to answer.

One thing you could do to help is by narrowing down your reproduction to the simplest form. You have quite a few dependencies and files in your reproduction. Try to remove as much as possible. See if you can determine what part of testing library caused the issue.

Sorry I can't be of more help right now.

kentcdodds avatar Sep 25 '22 05:09 kentcdodds

Thanks, that makes sense and is understandable. I'll try and narrow it down even further and raise smaller issues. Thanks again 🙂

louis-young avatar Sep 26 '22 09:09 louis-young

but I just thought I'd say part of the reason you're probably not getting a response here is because it looks like your question is harder to answer.

That is why I didn't respond yet. The reproduction includes a lot of what I would classify as noise. Try to keep in mind that a reproduction isn't like your production code. It doesn't need to be DRY or follow clean coding practices. Ideally it's just a single file with no dependencies at all. I saw you're using TypeScript. If the types are not relevant you can just remove them.

Put yourself in our shoes: We're given a repro and in order to understand it I have to open 3 different files which just consists of <20 lines half of which are just types. The less code I have to keep in my head the easier it is to understand and faster I can begin to understand where assumptions are being broken.

eps1lon avatar Oct 08 '22 09:10 eps1lon

Thanks for getting back to me. I've raised a much simpler, smaller issue here https://github.com/testing-library/react-testing-library/issues/1140. Hopefully that's helpful. Thanks 🙂

louis-young avatar Oct 12 '22 15:10 louis-young

This was resolved in https://github.com/testing-library/react-testing-library/issues/1140 🎉

-    await waitFor(() => value === ...);
+    await waitFor(() => expect(value).toEqual(...));

louis-young avatar Oct 12 '22 17:10 louis-young