Add wrapper option to render/testHook

[alexkrolick]

See #283 (comment)

• testHook
• render

[danielkcz]

I am thinking if we really need it as a wrapper option. I mean @kentcdodds is against making that utility more complicated and using options object would kinda imply to be open for further customization later. Besides, it would make things slightly more readable imo.

// define
const testHookWithNameContext = (callback, wrapperProps) => {
  let result
  testHook(
    () => {
      result = callback()
    },
    props => <NameContext.Provider {...wrapperProps} {...props} />,
  )
  return result
}

// use
const [a, b] = testHookWithNameContext(myCustomHook, {value: "ABC"})

[alexkrolick]

I think I still prefer the option object and here's why:

Positional arguments can be confusing for uncommon options. Someone new to a codebase can figure out what {wrapper} means but would have to look in the docs for the meaning of the second argument and might think it's somehow required.

For render, wrapper will fit in with the other options that already exist. You can say "the second argument is passed as the options to the render function" and you only need to document the usage one time (plus examples as needed).

Edit:

Also, you may need some of the other render options to test hooks in Karma or something.

[danielkcz]

Ok, I agree with your reasoning. I will take a stab at this probably today as I need it in my codebase anyway.

[danielkcz]

One thing might be rather confusing in case of more complex wrapper. What to do with passed props user might wonder.

const testHookWithContexts = (callback, { nameValue, otherValue }) => {
  let result
  testHook(
    () => {
      result = callback()
    },
    props => (
      <NameContext.Provider value={nameValue}>
        <OtherContext.Provider value={otherValue} {...props} />
      </NameContext.Provider>
    )
  )
  return result
}

In this case, the props are really just a { children } because testHook is not passing any other props to render call. We would need to clearly document that these props need to be spread on the lowest element in the wrapper tree 😕

I was thinking for a moment to utilize React.cloneElement here, but that won't work either for these complex scenarios.

Alternative approach could be passing array of nodes and let the testHook to build a tree from it and apply children to last of them.

[danielkcz]

@alexkrolick Feel free to close, forgot to use a magic word in the PR :)

[alexkrolick]

@FredyC good point that figuring out where to put ...props is confusing. Better to destructure ({children}) in the docs I think.