Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

13.x: testing error states with renderHook does not throw as expected #1733

Open
fastfrwrd opened this issue Jan 13, 2025 · 8 comments
Open

Comments

@fastfrwrd
Copy link

Describe the bug

We found that, upon upgrading to @testing-library/react-native 13.x, a number of existing tests which tested failure cases for hooks began to fail. A simplified reproduction looks like this:

import { renderHook } from "@testing-library/react-native";

const service = jest.fn().mockReturnValue(42);

function useCustom(): number {
  const serviceResult = service();
  if (!serviceResult) {
    throw new Error("service did not return a result");
  }
  return serviceResult;
}

describe("demonstrate the problem", () => {
  it("should throw if service does not return a result", () => {
    service.mockReturnValueOnce(undefined);
    expect(() => {
      renderHook(() => useCustom());
    }).toThrowErrorMatchingInlineSnapshot(`"service did not return a result"`);
  });
});

This test passes with 12.x and fails with 13.x. Specifically, the failure here is:

Received function did not throw

The following change fixes the test:

   it("should throw if service does not return a result", () => {
-    service.mockReturnValueOnce(undefined);
+    service.mockReturnValueOnce(undefined).mockReturnValueOnce(undefined);
     expect(() => {
       renderHook(() => useCustom());
     }).toThrowErrorMatchingInlineSnapshot(`"service did not return a result"`);

...which smells like a StrictMode hook issue, but I don't see anything that leads me to believe that was turned on by default, and also, this only seems to impact hooks which throw errors, so that doesn't make sense.

I've tested this with both Jest and Vitest, so it seems like some kind of issue within @testing-library/react-native for sure.

Expected behavior

When a hook that throws an error is rendered, the first failed rendered output ought to cause the renderHook call to throw, but only the second call to the hook can actually cause renderHook to throw.

Steps to Reproduce

See the above example. Let me know if you want me to include any other config files in this report or set up some sort of code sandbox.

Screenshots

n/a

Versions

Exclusive to 13.x.

@fastfrwrd
Copy link
Author

The only thing that really jumps out in diffing 12.9 and 13.0 is concurrentRoot - I wonder if renderHooks should be rendering into a component with concurrentRoot set to false, although that's not really possible with the current API.

@mdjastrzebski
Copy link
Member

Could you try running again with configure({ concurrentRoot: false }) call before renderHook call?

@fastfrwrd
Copy link
Author

Yep, confirmed, that fixes it. What do you think - should this just be documented in the migration guide, or is this really a bug?

@mdjastrzebski
Copy link
Member

Is that issue occurring only when your hook throws? Are there any scenarios when the hook does not throw and the test fails?

@fastfrwrd
Copy link
Author

fastfrwrd commented Jan 14, 2025

it's only thrown errors from hooks - all other tests work as-is. Rendering a component which throws as part of its rendering also works fine with concurrentRoot turned on:

function Component({ shouldThrow }: { shouldThrow?: boolean }) {
  if (shouldThrow) {
    throw new Error("the error is thrown");
  }
  return <Text>hello</Text>;
}

it("should throw if render component throws", () => {
  service.mockReturnValueOnce(undefined);
  expect(() => {
    render(<Component />);
  }).not.toThrow();
});

it("should throw if render component throws", () => {
  expect(() => {
    render(<Component shouldThrow />);
  }).toThrowErrorMatchingInlineSnapshot(`"the error is thrown"`);
});

@mdjastrzebski
Copy link
Member

Hmm, interesting observation.

BTW it's not guaranteed that render/renderHook will re-throw errors happening during rendering, as these might happen async with concurrent rendering and render/renderHook are sync. The safest way would be to wrap the component with an error boundary and put assertions about it's state.

https://react.dev/reference/react/Component#catching-rendering-errors-with-an-error-boundary

@mdjastrzebski
Copy link
Member

@fastfrwrd I've done some checks and it seems that the service is called twice. So if you swap mockReturnValueOnce to mockReturnValue the test passes correctly. I am not sure what causes hook to be called twice.

The TestComponent used to wrap user's hook in renderHook seems to also called twice, which could indicate strict mode being used but:

  1. RNTL does not enable it by itself, it would have to be react or react-test-renderer
  2. useEffect in test component is called only once, while in strict mode it should be called twice.

Hence, the most probable answer is that Concurrent Mode renders test component (+ your hook) twice simply as a matter of regular operation.

@fastfrwrd
Copy link
Author

fastfrwrd commented Jan 15, 2025

Spent a couple minutes testing what you said, and it definitely seems true! Has nothing to do with hooks, but rather any thrown error within a concurrent root leads to exactly one re-render, which is a difference from how things work when you turn the concurrent root off. Here's some tests which demonstrate that.

import { Text } from "react-native";
import { render } from "@testing-library/react-native";

const callTracker = jest.fn();

describe("demonstrate post-throw re-rendering", () => {
  function Component2({ throwOnMount }: { throwOnMount?: boolean }) {
    callTracker();

    if (throwOnMount) {
      throw new Error(`throw on mount`);
    }

    return <Text>Some Component</Text>;
  }

  beforeEach(() => {
    callTracker.mockClear();
  });

  it("should render once", () => {
    expect(callTracker).not.toHaveBeenCalled();
    render(<Component2 />);
    expect(callTracker).toHaveBeenCalledTimes(1);
  });

  it("should throw, but then it re-renders", () => {
    expect(callTracker).not.toHaveBeenCalled();
    expect(() => render(<Component2 throwOnMount />)).toThrow();
    expect(callTracker).toHaveBeenCalledTimes(2);
  });

  describe("non-concurrent", () => {
    it("does not re-render following throw", () => {
      expect(callTracker).not.toHaveBeenCalled();
      expect(() =>
        render(<Component2 throwOnMount />, { concurrentRoot: false })
      ).toThrow();
      expect(callTracker).toHaveBeenCalledTimes(1);
    });
  });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants