Change props on same component instance

[techniq]

• react-testing-library version: 2.3.0
• node version: 9.9.0
• yarn version: 1.5.1

No way to update props on same component instance like enzyme's

wrapper.setProps({ url: url1 });

What you did:

I'm looking to port react-fetch-component to use react-testing-library mostly due to Enzyme incomplete React 16 support especially for React.createContext() which I plan to use for a feature. As of now they have a merged fixed but has not been released yet.

With that said, I'm having difficulty porting over to react-testing-library. I understand it's goal of encouraging good testing practices by not accessing an instance of a component. Pretty much all of my tests were accessing the promises instance prop to coordinate when a fetch was complete / etc. For most of these cases, I've been able to wait() for the number of mock calls to match my expectations (not exactly my intentions, but it works).

await wait(() => expect(mockChildren.mock.calls.length).toBe(3));

One case where I'm having difficultly working around this issue is not having an equivalent setProps() capability like enzyme. I have a test that renders the <Fetch /> component 3 times with cache enabled which is by default instance level. With enzyme's setProps I can guarantee the same instance is being used, but I don't see a way to do this with react-testing-library.

What happened:

Unable to change props

Suggested solution:

Expose a setProps equivalent helper from render (although this might be against the goal of the library).

[kentcdodds]

Hi @techniq!

I've considered exposing a setProps function and even had an implementation but decided against it because it's generally unadvisable and in situations where it is useful it's actually pretty simple:

const {conatiner} = render(<Foo bar={true} />)

// update the props, re-render to the same container
render(<Foo bar={false} />, {container})

// can still use the same container and all other utilities

// you could also make a little utility
const updateProps = props => render(<Foo {...props} />, {container})
updateProps({bar: false})

So actually the library does have a "setProps" functionality, but it's not straightforward and that's kind of intentional. This is all documented with examples in the FAQ.

If you'd like to petition that we include a setProps method though, I'd be willing to discuss it. I'd love to get other's input as well.

Just to frame our discussion, my biggest concern is that it could lead people to go against the guiding principles. But as I consider it further, the fact that you could only do this for the component that you render, you could consider the props the API of that component (because that's what it is) and in that case, the props are how the user of your software is using your software so it actually does make sense. Thoughts?

[techniq]

I'm off to my boys ball game. Let me play around with this tonight and I'll let you know. Rendering to the container is probably a suffient api for me at initial blush. Any thoughts on the promises access? I feel like it was the best way for me to test my use cases (some can be difficult like out of order promise resolution).

…

On Mon, Apr 30, 2018, 5:10 PM Kent C. Dodds ***@***.***> wrote: Hi @techniq <https://github.com/techniq>! I've considered exposing a setProps function and even had an implementation but decided against it because it's generally unadvisable and in situations where it is useful it's actually pretty simple: const {conatiner} = render(<Foo bar={true} />) // update the props, re-render to the same containerrender(<Foo bar={false} />, {container}) // can still use the same container and all other utilities // you could also make a little utilityconst updateProps = props => render(<Foo {...props} />, {container})updateProps({bar: false}) So actually the library *does* have a "setProps" functionality, but it's not straightforward and that's kind of intentional. This is all documented with examples in the FAQ <https://github.com/kentcdodds/react-testing-library#faq>. If you'd like to petition that we include a setProps method though, I'd be willing to discuss it. I'd love to get other's input as well. Just to frame our discussion, my biggest concern is that it could lead people to go against the guiding principles. But as I consider it further, the fact that you could only do this for the component that you render, you could consider the props the API of that component (because that's what it is) and in that case, the props *are* how the user of your software is using your software so it actually does make sense. Thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#65 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAK1RM7xqHtIHoV_0VWfxWcWJO3X0dKPks5tt33ggaJpZM4TtLqf> .

[alexkrolick]

If you'd like to petition that we include a setProps method though, I'd be willing to discuss it. I'd love to get other's input as well.

How about returning a render method bound to the container? That would simulate a more real-world use-case, that is, the parent re-rendering.

[kentcdodds]

I like that @alexkrolick. What if we call it rerender so we don't have variable name conflicts and to make it more clear that it's different from the render method.

[kentcdodds]

Specifically, I think it should not (need to) return anything.

[alexkrolick]

rerender sounds good. It should also help you test unmount lifecycles.

[kentcdodds]

It should also help you test unmount lifecycles.

We currently do have an unmount method for that 👍

[kentcdodds]

Oh, and with regards to this:

Any thoughts on the promises access? I feel like it was the best way for me to test my use cases (some can be difficult like out of order promise resolution).

In general when I'm trying to ask myself how I get coverage on some code I stop and think: "Rather than coverage, let's consider use cases. What's a use case that I can test that would run this code. And then how would a user manually verify this using the API available."

For components the API available is:

1. Props
2. What is rendered
3. User events based on what's rendered
4. Context subscriptions
5. Data subscriptions/requests in componentDidMount

So based on which use case I'm trying to test for I'll use those APIs. Normally that will be in the form of some sort of user interaction, a mock store updating some state, or a mock http request resolving. Then I'll verify the state and normally that'll be a change in what's rendered or a callback prop was called.

Hopefully that's helpful!

[techniq]

@kentcdodds took longer than hoped, but I have all tests passing using react-testing-library (at least all the ones that were passing under enzyme... I have a few skipped).

I think const { rerender } = render(<Foo />) would be useful, but it's not make or break for me. Thanks again for the tip and all your contributions to open source.

[kentcdodds]

Awesome @techniq! I'd love to hear your feedback on the experience. Why did you decide to migrate? What made the migration the most difficult? What could have made it easier? Are you happy you migrated? Would you do it again?

It'd be super cool if you could write a blog post about this :D

[techniq]

@kentcdodds I'm not much of a blogger and always buried under a deadline, but here's some quick feedback 😄

Why did you decide to migrate?

Support for React 16, especially createContext and the draw of a lighter weight (in cognitive load) library (do I use shallow, or mount, or render, ...). Also nice and concise examples to understand how to use / best practices.

What made the migration the most difficult?

Not having access to the underlying instance :). Currently using wait() for the number of expected children render calls works, but doesn't feel like an equivalent test. I use fetch-mock and should use Promise results more which might help (I already do in places), but a quick port and waiting on those promises were still not working without using wait() and I didn't have time to dig in deeper.

Using wait() and making a big breaking change can cause the entire test suite to take a long time to return as I have to wait for all the tests to timeout. When I ran into this, I just used it.only one by one as I was making big breaking changes to reduce the iteration time.

What could have made it easier?

See wait() and promises above. If I ultimately get promises working and await them again instead of wait and failing assertions it should help, but will require more tests refactoring.

Re-rendering/setProps wasn't as straight forward but works once you led me in the right direction (I later found the expandable FAQ that mentioned this). I think exposing a rerender sounds like a good idea. It will still be a little painful to have to pass all props again unlike enzyme's setProps and would likely require your own test helper regardless.

Are you happy you migrated?

Yes :). Allowed me to implement and test using React.createContext for react-fetch-component

Would you do it again?

Already have - techniq/react-odata@4aac207

[kentcdodds]

Thanks @techniq! That's awesome!

So I do think that I want to add rerender. Here's what we'd need:

1. Add a rerender section in the docs (probably put it after the unmount section) to explain how it works and provide a simple example.
2. Remove the FAQ about updating props because now it's part of the API
3. Add a new test file here called rerender.js with a single test for this new functionality
4. Add a rerender property below this line.

The implementation can be as simple as:

rerender: ui => {
  render(ui, {container})
  // Intentionally do not return anything to avoid unnecessarily complicating the API.
  // folks can use all the same utilities we return in the first place that are bound to the container
}

I'm pretty sure that'd work great.

I'd like to wait and see if @suhailnaw would like to make this contribution as he asked on my AMA if there's something he can do to help (kentcdodds/ama#389).

Cheers!

[kentcdodds]

Ok, I think we've waited long enough. I'm going to open this up to anyone else who would like to contribute.

[johann-sonntagbauer]

I could give it a try

[kentcdodds]

Awesome. Go for it 👍

[ral51]

@kentcdodds

While @johann-sonntagbauer still working on it, I wanted to give a try but I'm having hard time to run tests. If I understood the issue correctly, is the test below looking good?

it('rerenders button into document', () => {
  const ref = React.createRef()
  const {container, rerender} = renderIntoDocument(<div ref={ref} />)
  expect(container.firstChild).toBe(ref.current)
  const updatedRef = React.createRef()
  rerender(<div ref={updatedRef} />)
  expect(container.firstChild).toBe(updatedRef.current)
})

[vivek12345]

I would like to volunteer for this as well.

[johann-sonntagbauer]

@ral51 @vivek12345 have not started yet, was on train with no internet connection :) so if you want to takle that issue you are welcome. let me know if I can help.

[ral51]

@johann-sonntagbauer thanks for the offer. Since you wanted to fix first, go ahead and do it. I just wanted to see myself how the fix should be.

[kentcdodds]

Here's the kind of test I think would work well:

test('rerender will re-render the element', () => {
  const Greeting = props => <div>{props.message}</div>
  const {container, rerender} = render(<Greeting message="hi" />)
  expect(container.firstChild).toHaveTextContent('hi')
  rerender(<Greeting message="hey" />)
  expect(container.firstChild).toHaveTextContent('hey')
})

To get the toHaveTextContent assertion, you may need to import jest-dom/extend-expect into the test file.

[kentcdodds]

🎉 This issue has been resolved in version 2.5.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[techniq]

👍 thanks all