kentcdodds / testing-react-apps

A workshop for testing react applications
https://www.epicreact.dev/modules/testing-react-apps-v1/testing-react-apps-welcome
Other
1.07k stars 719 forks source link

Pass props to useCounter hook #72

Closed som-sm closed 2 years ago

som-sm commented 2 years ago

Accept props in the container component and pass it down to the useCounter hook.

kentcdodds commented 2 years ago

Thanks for the contribution. In this part of the exercise we don't need that feature and I prefer to keep the code the same way as it is in the videos so I'm not going to merge. But thanks anyway :)

som-sm commented 2 years ago

Hello @kentcdodds!

Actually in the NEXT extra credit there's already some difference in the video and the code in the repository.

In the video the prop is directly passed to the useCounter hook, as shown in the screenshot below: image

But in the repo, the prop is passed via TestComponent (as shown below), and my PR just adds the same thing in other exercises as well to make it more consistent.

function setup({initialProps} = {}) {
  const result = {}
  function TestComponent(props) {
    result.current = useCounter(props)
    return null
  }
  render(<TestComponent {...initialProps} />)
  return result
}

Thanks!

kentcdodds commented 2 years ago

The file you changed is associated to this video: https://epicreact.dev/modules/testing-react-apps/testing-custom-hooks-extra-credit-solution-1 which ends like this:

image

This is how the code is in the repo currently.

som-sm commented 2 years ago

Yeah yeah, the files I changed currently matches with video. But I made the change so that its similar to the second extra credit (which does not match with the video) and also in case someone is trying to add more tests then the props would come in handy

kentcdodds commented 2 years ago

I'm pretty sure that this video: https://epicreact.dev/modules/testing-react-apps/testing-custom-hooks-extra-credit-solution-2 matches this file: https://github.com/kentcdodds/testing-react-apps/blob/main/src/__tests__/final/08.extra-2.js

Anyone who's watching the videos should know when the props addition is useful. I want to keep the videos and the code in the repo in sync.

Thanks anyway :)

som-sm commented 2 years ago

No, this video https://epicreact.dev/modules/testing-react-apps/testing-custom-hooks-extra-credit-solution-2 does NOT match this file: https://github.com/kentcdodds/testing-react-apps/blob/main/src/__tests__/final/08.extra-2.js

Code in video can be seen right at 03:46 mark: image

Code in src/__tests__/final/08.extra-2.js file (changes highlighted): image

And the thought behind my PR was:

  1. To make it uniform with the above mentioned test.
  2. If suppose someone wants to add a new test (as shown below) in the src/__tests__/final/08.extra-2.js file, then they can simply add the test without having to make any changes in the UseCounterHookExample component itself.
test('allows customization of the initial count', () => {
  render(<UseCounterHookExample initialCount={3} />)
  const message = screen.getByText(/current count/i)

  expect(message).toHaveTextContent('Current count: 3')
})

Hope, that makes sense. Thanks!

kentcdodds commented 2 years ago

Gotcha. I would much rather update 08.extra-2.js to match the video. There are countless ways to accomplish the goals here and people can extrapolate themselves if they feel like they want to try a different approach and that's fine. The priority here is to reduce confusion by matching the videos :)