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

How to wrap a hook with a Provider with prop passing?

Open acrowell1-chwy opened this issue 2 years ago • 16 comments

What is your question:

I have seen the following which was helpful: https://github.com/testing-library/react-hooks-testing-library/issues/23 This question steams further from that with regards to passing initialProps.
I have updated React and DOM ("react": "18.2.0", "react-dom": "18.2.0",) and the React Testing Library ("@testing-library/react": "14.0.0",) to the most recent versions. I have followed that pattern, but notice that the props passed into Provider are only available via children. Please see the example below (borrowed from the tests within the library https://github.com/testing-library/react-hooks-testing-library/blob/1e01273374af4e48a0feb1f2233bf6c76d742167/src/tests/useContext.test.tsx):

 test('should update value in context when props are updated', () => {
    const TestContext = createContext('foo');

    const wrapper = ({
      current,
      children,
    }: {
      current: string;
      children: any;
    }) => {
      console.log({ current });
      console.log({ children });
      console.log('nested', children.props.renderCallbackProps);

      return (
        <TestContext.Provider value={current}>{children}</TestContext.Provider>
      );
    };

    const { result, rerender } = renderHook(() => useContext(TestContext), {
      wrapper,
      initialProps: {
        current: 'bar',
      },
    });

    rerender({ current: 'baz' });

    expect(result.current).toBe('baz');
  });

// console.logs

console.log
{ current: undefined }

console.log
{
  children: {
    '$$typeof': Symbol(react.element),
    type: [Function: TestComponent],
    key: null,
    ref: null,
    props: { renderCallbackProps: [Object] },
    _owner: null,
    _store: {}
  }
}

console.log
nested { current: 'baz' }


Was this the intended change? Are we supposed to only pass children to a wrapper and then desconstruct those props from it? Can we update the tests or docs to represent the recommended pattern?

acrowell1-chwy avatar Aug 16 '23 15:08 acrowell1-chwy

@mpeyper I saw you answer in the previous question. Tagging you here for an extension of it.

acrowell1-chwy avatar Aug 16 '23 15:08 acrowell1-chwy

Hey @acrowell1-chwy, just want to double check that you have stopped using this library (@testing-library/react-hooks and migrated to @testing-library/react), right?

If that’s the case, the wrapper props are handled differently than they were here so our tests and docs do not represent how they intend it to be used. I suggest raising an issue over there instead (or I can move this one if you prefer?).

Unfortunately, documenting the difference of wrapper props behaviour is I got up to in the ā€œnever releasedā€ migration guide.

mpeyper avatar Aug 16 '23 21:08 mpeyper

@mpeyper you are correct, I am using renderHook from @testing-library/react. Yes, please move this to an issue. Thank you for confirming!

acrowell1-chwy avatar Aug 16 '23 21:08 acrowell1-chwy

@mpeyper you are correct, I am using renderHook from @testing-library/react. Yes, please move this to an issue. Thank you for confirming!

Done šŸ™‚

FWIW, I believe their implementation should only pass children to the wrapper and never any props, so you will likely need to manage them manually outside of the wrapper/renderHook callbacks.

mpeyper avatar Aug 16 '23 21:08 mpeyper

FWIW, I discovered that props passed from initialProps were nested within children -> const { current } = children?.props?.renderCallbackProps;. If that helps anyone. Appreciate the prompt response and aid. 😸

Code example:

test("should update value in context when props are updated", () => {
  const TestContext = createContext("foo");

  const wrapper = ({ children }: { children: any }) => {
    const { current } = children?.props?.renderCallbackProps;
    return (
      <TestContext.Provider value={current}>{children}</TestContext.Provider>
    );
  };

  const { result, rerender } = renderHook(() => useContext(TestContext), {
    wrapper,
    initialProps: {
      current: "bar",
    },
  });

  rerender({ current: "baz" });

  expect(result.current).toBe("baz");
});

acrowell1-chwy avatar Aug 16 '23 21:08 acrowell1-chwy

@mpeyper do you know if this const { current } = children?.props?.renderCallbackProps; is intentional for pulling props from initialProps (as shown above)? Happy to contribute/help.

acrowell1-chwy avatar Aug 17 '23 15:08 acrowell1-chwy

I don’t know offhand and am not really involved with this implementation. My understanding was only the TestComponent was passed to the wrapper to be wrapped. Any props you are reading off the component is likely unsafe and not recommended.

mpeyper avatar Aug 18 '23 07:08 mpeyper

This is how it is documented: https://testing-library.com/docs/react-testing-library/api/#renderhook-options

Considering it's not working as documented, this should be tagged as a bug, not as a question. Props should be available alongside the children one.

Anyone trying to test react 18 components following the documentation will face with this problem if they try to add any initialProps. Using the old @testing-library/react-hooks is not an option for anyone using react 18, since it throws errors about a react 18 deprecation in the render method.

Thanks for the workaround @acrowell1-chwy.

Edit: Ok my bad I was reading the old documentation, but the new way to implement it is terrible... having to create a wrapper is not a good solution, because later on when trying to use rerender it simply does not update the props. Using @acrowell1-chwy solution works better for all cases.

elboletaire avatar Aug 21 '23 11:08 elboletaire

For other’s reference, the new docs are here https://testing-library.com/docs/react-testing-library/api/

Whatever workarounds you use, please consider that it is not part of the supported API and could change without notice.

mpeyper avatar Aug 21 '23 11:08 mpeyper

For other’s reference, the new docs are here testing-library.com/docs/react-testing-library/api

I guess you answered before my edits šŸ™ƒ

elboletaire avatar Aug 21 '23 11:08 elboletaire

Whatever workarounds you use, please consider that it is not part of the supported API and could change without notice.

Yeah but the main API looks broken... the documented workaround (creating a createWrapper function) does not work for the rerender props update case. Shouldn't this be considered a bug then?

Edit: BTW just to be clear: the old library works well here. Its only issue is being incompatible with react 18. I understand removing some functionalities and methods when porting the library here, but this initialProps part does not seem to be working as intended now.

elboletaire avatar Aug 21 '23 11:08 elboletaire

The workaround suggested is highlighting that’s it’s not handling anything for you automatically. You could pass an object to the wrapper then update the values in the object and trigger a rerender if you need them to change, but it’s all going to be manual on your part. It’s not a bug, just frustrating.

I’d just like to reiterate though that I’m not associated with the new implementation at all. You’ll need to wait for someone from @testing-library/react to chime in providing better support for this use case. Note though that I imagine part of the reason it’s less supported now is that the new version is piggy backing off the render function used for testing components and they may not want to bleed the logic that far (not something we had to worry about in @testing-library/react-hooks)

mpeyper avatar Aug 21 '23 12:08 mpeyper

Agreed with the comments here, passing the "props" to the hook's render function (instead of the wrapper component) makes rerender much less useful. Perhaps I might recommend adding a second function like 'rerenderWrapper` that works this way? Something like:

const TrackingProvider = (props: { trackingProps: { userId: number }, children: React.ReactNode }) => {
  // .. Context provider that maintains state
}

// useTracking reads state & dispatches tracking events
const { rerenderWrapper } = renderHook(() => useTracking(), {
  wrapper: TrackingProvider,
  initialProps:  {
    trackingProps: { userId: 123 }
  },
})

expect(track).toHaveBeenCalledWith({ userId: 123 })

rerenderWrapper({ trackingProps: { userId: 456 })

expect(track).toHaveBeenCalledWith({ userId: 456 })

davidgoli avatar Dec 19 '23 19:12 davidgoli