Skip to content

Rerender with new wrapper #322

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

Closed
josejulio opened this issue Mar 26, 2020 · 7 comments
Closed

Rerender with new wrapper #322

josejulio opened this issue Mar 26, 2020 · 7 comments
Labels
enhancement New feature or request

Comments

@josejulio
Copy link

josejulio commented Mar 26, 2020

Describe the feature you'd like:

I would like to be able to test a re-render with a different wrapper.
The use case is to ensure that returned callbacks of the tested hook are changed when a new wrapper is used.

Suggested implementation:

--

Describe alternatives you've considered:

I have considered using the renderHook again, but it doesn't keep the previous state and the test is not really relevant this way.

Teachability, Documentation, Adoption, Migration Strategy:

--

@josejulio josejulio added the enhancement New feature or request label Mar 26, 2020
@mpeyper
Copy link
Member

mpeyper commented Mar 29, 2020

It's not super elegant, but you can do something like this

  const TestContext1 = createContext()
  const TestContext2 = createContext()

  function useTestContext() {
    const context1Value = useContext(TestContext1)
    const context2Value = useContext(TestContext2)

    if (context1Value) {
      return 1
    } else if (context2Value) {
      return 2
    } else {
      return 0
    }
  }

  test('should change context used', () => {
    let Wrapper = TestContext1.Provider

    const wrapper = ({ children }) => <Wrapper value={true}>{children}</Wrapper>

    const { result, rerender } = renderHook(() => useTestContext(), { wrapper })

    expect(result.current).toBe(1)

    Wrapper = TestContext2.Provider

    rerender()

    expect(result.current).toBe(2)
  })

or if it's super important that it be a new wrapper (for example, if both the Provider and props needed to change for some reason), you could do something like this

  const TestContext1 = createContext()
  const TestContext2 = createContext()

  function useTestContext() {
    const context1Value = useContext(TestContext1)
    const context2Value = useContext(TestContext2)

    return context1Value || context2Value
  }

  test('should change context used', () => {
    let testWrapper = ({ children }) => (
      <TestContext1.Provider value="foo">{children}</TestContext1.Provider>
    )

    const wrapper = (props) => testWrapper(props)

    const { result, rerender } = renderHook(() => useTestContext(), { wrapper })

    expect(result.current).toBe('foo')

    testWrapper = ({ children }) => (
      <TestContext2.Provider value="bar">{children}</TestContext2.Provider>
    )

    rerender()

    expect(result.current).toBe('bar')
  })

@mpeyper
Copy link
Member

mpeyper commented Mar 29, 2020

Currently the wrapper does not receive any props other than children (the TesHook component that renders the hook), but a one line change would make this possible:

  const TestContext1 = createContext()
  const TestContext2 = createContext()

  function useTestContext() {
    const context1Value = useContext(TestContext1)
    const context2Value = useContext(TestContext2)

    return context1Value || context2Value
  }

  test('should change context used', () => {
    const wrapper = ({ newWrapper = false, children }) => {
      if (newWrapper) {
        return <TestContext2.Provider value="bar">{children}</TestContext2.Provider>
      } else {
        return <TestContext1.Provider value="foo">{children}</TestContext1.Provider>
      }
    }

    const { result, rerender } = renderHook(() => useTestContext(), { wrapper })

    expect(result.current).toBe('foo')

    rerender({ newWrapper: true })

    expect(result.current).toBe('bar')
  })

Obviously you would need to pass props (or initialProps) for the sole reason of changing the wrapper, but it could also be useful for changing the value passed to a Provider which in my opinion is a more realistic test case than the whole wrapper changing.

Let me know if you think any of the above would satisfy your use case. I'm thinking about making the change to pass additional props to the wrapper anyway as I cannot see a reason not to, it's a very small change and it might be useful to someone at some stage.

@josejulio
Copy link
Author

This worked for my use case, thank you!

@adamseckel
Copy link
Contributor

it could also be useful for changing the value passed to a Provider which in my opinion is a more realistic test case than the whole wrapper changing.

I've just ran into this scenario. Your proposed changes would be really useful here @mpeyper. +1 from me!

@mpeyper
Copy link
Member

mpeyper commented Jun 4, 2020

Thanks for the feedback @Hemlok. Honestly, I forgot all about that idea. I'll move it up my todo list (unless you or someone else wants to take a run at it?).

@adamseckel
Copy link
Contributor

I'll put a pr together rn 😊

@adamseckel
Copy link
Contributor

PR here: #381 🎉

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

No branches or pull requests

3 participants