`renderHook` `wrapper` function is no longer called with `initialProps`

[louis-young]

• @testing-library/react version: 13.4.0
• Testing Framework and version: Jest version 27.5.1
• DOM Environment: Jest DOM version 5.16.5 with JSDOM version 27.5.1

Relevant code or config:

import { renderHook, act } from "@testing-library/react";
import { TestContextProvider, useTestContext } from "./application";

const Wrapper = ({ children, dependency }) => {
  return (
    <TestContextProvider dependency={dependency}>
      {children}
    </TestContextProvider>
  );
};

describe("useTestContext", () => {
  it("calls `dependency` when `performSomeOperation` is called", async () => {
    const dependency = jest.fn();

    const { result } = renderHook(useTestContext, {
      wrapper: Wrapper,
      initialProps: {
        dependency,
      },
    });

    await act(async () => {
      result.current.performSomeOperation();
    });

    expect(dependency).toHaveBeenCalledTimes(1);
  });
});

What you did:

I replaced @testing-library/react-hooks with the latest major version of @testing-library/react.

What happened:

The test failed.

useTestContext › calls `dependency` when `performSomeOperation` is called

TypeError: dependency is not a function

Reproduction:

https://codesandbox.io/s/github/louis-young/react-testing-library-issue?file=/src/index.test.js

Problem description:

I'm under the impression that the renderHook function that @testing-library/react exports is designed to be a direct replacement for the renderHook function that @testing-library/react-hooks exports. This doesn't appear to be the case.

Suggested solution:

If this is a regression, I suggest that the wrapper option function is called with the initialProps values as it was in @testing-library/react-hooks. If this isn't a regression, I suggest that (if there isn't already, although I can't seem to find it) there's a documented idiomatic way of providing dependencies to render wrapper functions.

Thanks 🙂

[eps1lon]

So they passed the initialProps both to the wrapper component and the hook itself?

I would rather avoid that API choice. render also has no wrapper props so render and renderHook are consistent which is more important than renderHook from this library being consistent with renderHook from another library.

[no-yan]

I would rather avoid that API choice.

Removing wrapper props from API would be okay, since hooks with context can be tested without wrapper props.

test('allows wrapper components2', async () => {
  const ValueContext = React.createContext('default')
  const SetContext = React.createContext('default')
  function Wrapper({children}) {
    const [value, setValue] = React.useState('provided')
    return (
      <ValueContext.Provider value={value}>
        <SetContext.Provider value={setValue}>{children}</SetContext.Provider>
      </ValueContext.Provider>
    )
  }
  const {result} = renderHook(
    () => {
      const setValue = React.useContext(SetContext)
      React.useEffect(() => {
        setValue('updated')
      }, [setValue])

      return React.useContext(ValueContext)
    },
    {
      wrapper: Wrapper,
    },
  )

  expect(result.current).toEqual('updated')
})

However, I’m worried about that this change will require a significant change in testing when we migrate React Testing Library to test React 18.
It is generally anti-pattern, i think, to change the implementation (in this case React) and the tests all at once, but do you think we should @eps1lon ?

[no-yan]

To clarify, I am not concerned about compatibility, I am concerned that the of React itself and of renderHook will make migration harder for large scale development.
(I may be a bit timid, because it took me two weeks in big monorepo)

[louis-young]

So they passed the initialProps both to the wrapper component and the hook itself?

Yeah, that's right 👍

I would rather avoid that API choice. render also has no wrapper props so render and renderHook are consistent which is more important than renderHook from this library being consistent with renderHook from another library.

That sounds reasonable. Is there a suggested upgrade path or an idiomatic/suggested way to retain the same functionality? I can think of writing something like a curried createWrapper function but this feels slightly unnecessary considering renderHook was (supposedly, from what I understand) intended to be a like-for-like replacement.

Curried `createWrapper` function implementation

const Wrapper = ({ children, dependency }) => {
  return (
    <TestContextProvider dependency={dependency}>
      {children}
    </TestContextProvider>
  );
};

const createWrapper =
  ({ dependency }) =>
  ({ children }) => {
    return Wrapper({ children, dependency });
  };

describe("useTestContext", () => {
  it("calls `dependency` when `performSomeOperation` is called", async () => {
    const dependency = jest.fn();

    const { result } = renderHook(useTestContext, {
      wrapper: createWrapper({ dependency }),
    });

    await act(async () => {
      result.current.performSomeOperation();
    });

    expect(dependency).toHaveBeenCalledTimes(1);
  });
});

I've also noticed that renderHook no longer returns a waitFor function. I prefer this API design (using the top level waitFor instead) but is this and any other breaking change(s) documented anywhere? We had a hard time attempting to upgrade due to the differences with the waitFor implementations between the main and hooks testing library and also the issue we're discussing. It'd be nice to help avoid other people running into the same issues so perhaps this could be documented somewhere if it isn't already? I'd be happy to help with the warnings/documentation.

Thanks 🙂

[eps1lon]

considering renderHook was (supposedly, from what I understand) intended to be a like-for-like replacement.

It was not intended to implement the same API since the API itself was problematic in large parts.

I didn't know about this part but I would count this in.

I'd be happy to help with the warnings/documentation.

Documentation would make sense. Ideally with a workaround.

Can't see how runtime warnings would work though.

[louis-young]

It was not intended to implement the same API since the API itself was problematic in large parts.

I see, are the differences between the APIs documented anywhere already?

Documentation would make sense. Ideally with a workaround.

I think so too. What do you think of the proposed workaround I detailed here?

#1161 (comment)

Can't see how runtime warnings would work though.

Yeah, I don't think a runtime warning is suitable; more likely a note/warning under the renderHook documentation 👍

[eps1lon]

I see, are the differences between the APIs documented anywhere already?

We have documentation for what renderHook does: https://testing-library.com/docs/react-testing-library/api/#renderhook

I think so too. What do you think of the proposed workaround I detailed here?

#1161 (comment)

I like the overall approach. Two things: Wrapper shouldn't be called since it's a component. Instead, write <Wrapper />. Otherwise class components or components with hooks would throw.

Also, use a named function expression for the return value instead of an anonymous arrow function expression:

const createWrapper =
  ({ dependency }) =>
  function FinalWrapper({ children }) {
    return <Wrapper dependency={dependency}>{children }</Wrapper>;
  };

That way we get a good display name in error messages.

[louis-young]

We have documentation for what renderHook does: https://testing-library.com/docs/react-testing-library/api/#renderhook

I suppose it'd be an entirely different thing to document the migration from @testing-library/react-hooks to @testing-library/react, fair enough 👍

I like the overall approach. Two things: Wrapper shouldn't be called since it's a component. Instead, write <Wrapper />. Otherwise class components or components with hooks would throw.

Also, use a named function expression for the return value instead of an anonymous arrow function expression:

const createWrapper =
  ({ dependency }) =>
  function FinalWrapper({ children }) {
    return <Wrapper dependency={dependency}>{children }</Wrapper>;
  };

That way we get a good display name in error messages.

Well spotted! Thanks, I'll test the workaround in a few applications and then raise a pull request to add a note to the documentation.

I'll close this issue as it's an intended change 👍