Use @testing-library/react-hooks in our unit tests.

[brainkim]

Part of #8419

I started cleaning up the unit tests. There might be a little bit of flake because of testing-library/react-hooks-testing-library#656, but overall the tests look a lot nicer, and don’t spew console errors when they fail. The plan is to continue refactoring the rest of the test suite (at least all the hook tests) to use the react-hooks testing library, but I got tired of rewriting tests so I’m doing some actual refactoring work now.

[hwillson]

This is just AWESOME @brainkim - seeing our dependency on render counts start to disappear is absolutely amazing! 🎉 Thanks for working on this!

[benjamn]

Huge improvement!

[mpeyper]

What you have here is fine, and this is just a suggestion, but this case is what waitForValueToChange was introduced for.

      const { result, waitForValueToChange } = renderHook(
        () => useCreateTodo(),
        { wrapper: ({ children }) => (
          <MockedProvider mocks={mocks}>
            {children}
          </MockedProvider>
        )},
      );

      // TODO: This misses the first update for some reason.
      expect(result.current.loading).toBe(true);
      expect(result.current.data).toBe(undefined);

      await waitForValueToChange(() => result.current.loading);
      // or
      // await waitFor(() => expect(result.current.loading).toBe(false));
      // or
      // await waitFor(() => !result.current.loading);
      
      expect(result.current.data).toEqual(CREATE_TODO_RESULT);

Also, I'm curious what the expected behaviour is for // TODO: This misses the first update for some reason.? Reading the test I would have though you'd expect it to be in a loading state at this point, no?

[brainkim]

Also, I'm curious what the expected behaviour is for // TODO: This misses the first update for some reason.

Seems like an outdated comment! My bad.

[mpeyper]

just calling out that @testing-library/react-hooks has it's own act function that will probably be the same as as the one be being exported by @testing-library/react. You could ensure it's the react-dom/test-utils implementation by importing from @testing-library/react-hooks/dom instead (we support native hooks using react-test-renderer as well and the import tries to autodetect which renderer to use.

[mpeyper]

Sorry if my comment about act was misleading before. What I meant was both @testing-library/react and @testing-library/react-hooks have an export called act that may be compatible with each other as they both will use the react-dom/test-utils version of act for DOM rendering.

Incompatibility can occur when @testing-library/react-hooks detects that the react-test-renderer is available in the environment (i.e. it can be required) and it will use that over react-dom for rendering by default. This may happen without you explicitly listing it as a dependency if one of your dependencies (or on of your dependency's dependencies, etc.) lists it as a dependency.

When this happens neither the act from react-dom/test-utils or the exported version from @testing-library/react will work without warnings for the tests using @testing-library/react-hooks.

The safest way to avoid this would be to use each libraries export, e.g.:

import { render, wait, act as reactAct } from '@testing-library/react';
import { renderHook, act as reactHooksAct } from '@testing-library/react-hooks';

But this can feel cumbersome and detract from the readability of your tests.

Alternatively, you can opt out of the renderer detection and force @testing-library/react-hooks to use react-dom for rendering, and consequently be compatible with act from @testing-library/react (at least so long as we both use react-dom for DOM rendering), e.g.:

import { render, wait, act } from '@testing-library/react';
import { renderHook } from '@testing-library/react-hooks/dom';

Once again. just a suggestion, what you have should work for the forseeable future, but may break in weird and hard to debug ways one day.

[brainkim]

My main reasons for switching to react-dom/test-utils is that 1. it works :) and 2. I was having trouble tracing act warnings when using the testing library equivalents. I’m knee deep in a refactor, and the main struggle with act() is getting it to shut up about “overlapping act calls” or whatever, and being able to Ctrl-P ReactAct in DevTools and adding a breakpoint to figure out who on Earth is causing the warning is probably the most important thing to me right now.

I get what you’re saying: testing-library/react and testing-library/react-hooks both export their own versions of act() because the renderer can be swapped out based on dependencies. This is a case of “I’ll cross that bridge when I need to” insofar as we don’t test against different renderers or even test rendering output at all, and I don’t plan on using any dependency which would cause the react test renderer to be imported. Maybe we can import the non-magic versions of testing-libraries instead sometime.

Again, thanks for the feedback! React testing library has saved me a ton of time already!