📣 RFC: Alternative renderHook APIs

[mpeyper]

I'm working on the improved docs in #19 and I'm struggling a bit with the section on changing the parameters passed to the hooks. Essentially, my issue is that there are 2 ways you can do it.

The first way is to just keep local variables and update them before rerendering:

let someValue = 0
const { result, rerender } = renderHook(() => useSomeHook(someValue))

// update value
someValue = 1
rerender()

The second way is the use the options.initialProps and rerender(newProps) features:

const { result, rerender } = renderHook(({ someValue }) => useSomeHook(someValue), {
  initialProps: { someValue: 0 }
})

// update value
rerender({ someValue: 1 })

The main reason the second way exists was to work around an issue in our own unit test for ensuring useEffect hooks were testable. The problem was that the updated value was updated for the cleanup of the effect too rather than capturing the initial value. In hindsight, I can see that is probably only an issue for that test, as most users would be passing the values into a hook function, capturing the initial value for the cleanup.

I'm not saying that having the callback function scope the variables isn't still a good idea in some cases, but I think it's a non-issue for most users.

Now we could just remove the initialProps/newProps functionality altogether, but I personally find the usage of a bit more ergonomic, especially when there is more than 1 value to update, as I don't have to keep a variable and spend multiple lines to rerender (1 for each update to the values and the another to render), so I'm asking for opinions on people's preferred API for this.

I'm also open to any ideas for a better API than either of these if you can think of one. I've actually been playing with an idea for a "hook-like" API along the lines of:

const { result, updateProps } = renderHook(() => {
  const someValue = useProps(0)
  return useSomeHook(someValue)
})

// update value
updateProps(1)

Where updateProps calls rerender under the hood. I'm not sure if this is a good idea, just a thought I had. Anyway, let me know your thoughts on any of the above.

[hartzis]

I really like where you're headed! Could it be simplified even more so we don't have to explicitly use useProps either?

const { result, updateProps } = renderHook(useSomeHook, initialProps)

// update value
updateProps(1)

Then under the hood renderHook could use your useProps? I also keep seeing the pattern of always passing an anonymous function wrapping the tested hook, () => useSomeHook(), why couldn't renderHook just do that?

I think this might break the current api, but just throwing out to think about it.

[mpeyper]

Hi @hartzis,

Thanks for taking the time to give your thoughts.

The main reason why we don't just accept the initialProps as the second parameter is that there are other options that can be provided, i.e. wrapper. It's impossible to say which is more valid as the second parameter and which would be third, as anyone wanting to update props will need to use initialProps, but anyone wanting to useContext will need to use wrapper, so they both went into the options` giving them the same level of precedence in API. That's not to say we couldn't do that, just that I'd like to find a better way.

Essentially what your asking for with calling the hook with the props is what it does now, so long as your hook accept a single parameter:

function useSomeHook(someValue) {
  // whatever
}

const { result, rerender } = renderHook(useSomeHook, { initialProps: 0 })

// update value
rerender(1)

The anonymous function is often used to take the single parameter and turn it into many for the hook function:

function useSomeHook(someValue1, someValue2) {
  // whatever
}

const { result, rerender } = renderHook(
  ({ someValue1, someValue2 }) => useSomeHook(someValue1, someValue2),
  { initialProps: { someValue1: 0, someValue2: 1 } }
)

// update value
rerender({ someValue1: 3, someValue2: 4 })

Because of this I just use the anonymous function for all cases for consistency.

[mpeyper]

The current spike I have for useProps/updateProps would look like this for the above example

function useSomeHook(someValue1, someValue2) {
  // whatever
}

const { result, updateProps } = renderHook(()  => {
  const [someValue1, someValue2] = useProps(0, 1)
  return useSomeHook(someValue1, someValue2)
})

// update value
updateProps(3, 4)

The hook callback could be onelined as:

const { result, updateProps } = renderHook(() => useSomeHook(...useProps(0, 1)))

I'm not sure that is particularly readible for newcomers though.

There's also a restriction that it can only be called once within renderHook, which is probably fine but may cause difficult to debug issues if misused, so I'd like to either make it jsut work, or at least give a meaningful error if they try.

The other thing I'm noticing as I update the tests to use the new API, is the rerender function is becoming irrelevant. I'm sure there are going to be cases where you want to rerender the hook without props changing so I'm loathed to remove it, but I'm struggling to find one myself. If you have one then please let me know.

[hartzis]

Nice! I too can't currently think of a use for rerender, but there is probably someone that will find a reason, 🤣

Thoughts on a "helper" for the oneliner?

function testHook () {
  const [hook, ...args] = arguments;
  return renderHook(() => hook(...useProps(...args)))
}
...
const { result, updateProps } = testHook(useSomeHook, 0, 1)

Too far? Too much? I may just be having too much fun.

[mpeyper]

I've moved away from the useProps idea as the implmentation was starting to get quite complicated and it was starting to feel like I was make a hooks-like implementation for the "cool" factor more than anything. I still think the concept has legs, but I don't have the time myself to put into an entire rewrite of much of the libraries guts right now.

On a different note, I was given an interesting suggestion from @jpeyper in person about this which I'll put here for consideration.

The idea is to use default values on the parameters for the initial props, and have rerender update the props if they are provided... something like:

const { result, rerender } = renderHook(
  (someValue = 0, someOtherValue = 'test') => useSomeHook(someValue, someOtherValue)
)

// update values
rerender(1, 'updated test') // order would be important here

Note: I think this is technically possible with the current API as long as you use an object for the one and only parameter, e.g.

const { result, rerender } = renderHook(
  ({ someValue = 0, someOtherValue = 'test' }) => useSomeHook(someValue, someOtherValue)
)

// update values
rerender({ someValue: 1, someOtherValue: 'updated test' })

I prefer this to the current initialProps implementation as it's one less time I have to duplicate the names of that parameters. The second example here (using the current props objectalso has some benefits in my opinion in that it doesn't rely on order to line up the parameters and it's perhaps a bit clearer when reading the code to identify the link between thererender` values and the the callback parameters.

I also think that if the rerender behavior was changed to merge with the current props, this could be quite a straight forward API to use:

const { result, rerender } = renderHook(
  ({ someValue = 0, someOtherValue = 'test' }) => useSomeHook(someValue, someOtherValue)
)

// update someValue
rerender({ someValue: 1 })

// update someOtherValue
rerender({ someOtherValue: 'updated test' })

// resets both
rerender({ someValue: undefined, someOtherValue: undefined })

Happy to hear thoughts 👂

[hartzis]

@mpeyper I personally really like the idea of using default values.

[mpeyper]

Thanks for the feedback @hartzis! Just for my reference, which of the 3 shown example do you prefer?

[hartzis]

@mpeyper Yeah, I guess I kind of saw them all as essentially the same. The defaults are in the user's control and not the library.

rerender could just pass all arguments to the function passed to renderHook?

Then both the object destructuring and the order of params just work?

Your examples are easy to follow, read, and feel natural with what I think one would "expect". Please let me know if I'm misunderstanding, but they really feel straightforward and clean.

[hartzis]

@mpeyper made a quick attempt at a possible solution 😺

master...hartzis:patch-1

[mpeyper]

Yep, that's the basic idea for

const { result, rerender } = renderHook(
  (someValue = 0, someOtherValue = 'test') => useSomeHook(someValue, someOtherValue)
)

// update values
rerender(1, 'updated test') // order would be important here

It still relies on them passing all the arguments each time, regarless of whether they are changing or not. It also has the mental just of having the unnamed arguments of the rerender function lining up with the named parmeters of the hook callback.

It would be nice if the new api was backwards compatible initialProps but with a deprecated message, which I think you would get with something like

function renderHook(callback, { initialProps, wrapper } = {}) {
  // ...
  if (initialProps) {
    console.warn("some deprecated message")
  }
  const hookProps = { current: [initialProps] }
  // ...
}

The second example (which I think is technically already possible) improves the mental jump issue a bit by naming the arguments

const { result, rerender } = renderHook(
  ({ someValue = 0, someOtherValue = 'test' }) => useSomeHook(someValue, someOtherValue)
)

// update values
rerender({ someValue: 1, someOtherValue: 'updated test' })

The other advantage here is that argument order is not important, but all arguments are still required.

We would swill want to add the deprecated message for anyone passing initialProps, but I think that is the only change we would need to make.

The final example solves the need to pass all arguments each time by merging the new props into the old props

const { result, rerender } = renderHook(
  ({ someValue = 0, someOtherValue = 'test' }) => useSomeHook(someValue, someOtherValue)
)

// update someValue
rerender({ someValue: 1 })

// update someOtherValue
rerender({ someOtherValue: 'updated test' })

// resets both
rerender({ someValue: undefined, someOtherValue: undefined })

The change here would be something like

rerender: (newProps = {}) => {
  hookProps.current = { ... hookProps.current, ...newProps }
    // ...
}

I don't think there is a way to make the changes for option 3 without being a breaking change as the behaviour of the rerender argument would be different to now, so cases where they legitimately wanted to pass an object with different keys to a previous render would fail. Consequently would probaably would bother with a deprecated message and just go for the breaking change.

There might be some more work to only update the props if the values provided actually change (e.g. shallow compare the keys and use the old props if nothing actually changes so it keeps the reference), but I'm not sure how important that is.

---

As you said, if you implement option 1, you haven't actually prevented anyone from using option 2 if they want to. The harder part for us is documenting the behavour in a way that is clear, so if we went for option 1, we would likely be unable to document option 2 without confusing people.

Similarly, option 3 does not prevent people from using option 2 either (the merge would jsut override every key). I think this combination is easier to document the option 3 behaviour and have it cover both variations anyway.

I don't think it makes sense to support options 1 and 3 at the same time either as the mismatch of functionality would likely cause confusing behaviour to anyone that used an object as their first argument.

I think I prefer option 2 or 3 over option 1 for the mental leap reasons. Part of me kind of likes the explicitnes of option 2, but the convenience of option 3 is appealing.

[hartzis]

I too like 2 and/or 3. in addition to the deprecated message

[mpeyper]

From #160 (@PolarbearDK)

---

Describe the feature you'd like:

I think the renderHook interface is a bit clumbersome, especially when working with custom hooks with arguments.

Suggested implementation:

I have created a Typescript wrapper to simplify the unit testing code

// Test utilty for custom hooks with effects.
// Usage:
//       const hook = renderCustomHook(useMyHook, TypedArd1, TypeArg2, TypedArg3...);
//
// assert results from: hook.result
// use hook.rerender(typed arguments) to update hook.
export function renderCustomHook<T extends any[], R>(
  theHook: (...args: T) => R,
  ...args: T
): {
  readonly result: HookResult<R>;
  readonly waitForNextUpdate: () => Promise<void>;
  readonly unmount: () => boolean;
  readonly rerender: (...args: T) => void;
};
export function renderCustomHook(theHook: (...args: any[]) => any, ...args: any[]) {
  const hook = renderHook((param: { props: any[] }) => theHook(...param.props), {
    initialProps: { props: args },
  });
  return { ...hook, rerender: (...args: any[]) => hook.rerender({ props: args }) };
}

Describe alternatives you've considered:

Would it be possible to include something like this in the library?

[mpeyper]

My response to #160

---

I'm not particularly fond of the const hook = renderHook(someHook, arg1, arg2, arg3) interface. One of my design goals with this library is to use the hook like you would in your component. This means calling it yourself with the arguments you want ti to have.

An option I have considered would be do something like:

const testSomeHook = wrapHook(someHook)

const hook = testSomeHook(arg1, arg2, arg3)

Which is a similar concept, but feels more like just calling the function than your example.

[mpeyper]

From #407 (@cincospenguinos)

---

Describe the feature you'd like:

I find usage of this library hard to grok. If I understand correctly, the current interface for rendering and rerendering a hook is something to this effect:

const { rerender } = renderHook((props) => useMyHook(props.arg1, props.arg2), { initialProps: { arg1: 'foo', arg2: 'bar' } });
rerender({ arg1: 'bar', arg2: 'foo' });

A downside to this is that as arguments grow, so too does the initial props object grow too.

Suggested implementation:

Another way to interact with rendering a hook would simply be switching over to an array of arguments. This would
allow handing the hook to render directly rather than providing an anonymous function:

const { rerender } = renderHook(useMyHook, ['foo', 'bar']);
rerender(['bar', 'foo']);

Describe alternatives you've considered:

I have no real alternatives--I just found myself struggling to get renderHook to work simply because I found the usage API to be unintuitive. I think my proposed solution would simplify interaction with the library, as hooks will always be functions and you will always provide arguments to a function, making an array a more expressive representation of function arguments than a javascript object.

Teachability, Documentation, Adoption, Migration Strategy:

NOTE

I would be more than happy to implement this change to the API, given the fact that what I'm asking for is a decent amount of work.

[mpeyper]

Thanks @cincospenguinos,

Similarly to previous suggestions, this one goes against my design goals of have the use of the hook function be as similar to how you use them in a function component as possible.

My other big concern with anything array based instead of using a props object is it's harder to trace the connection between the arguments used in renderHook and those of rerender. For example:

const { result, rerender } = renderHook(useSomeHook, ['value1', 'value2']);

act(() => {
  result.current.someUpdate()
})

rerender(['value3', 'value4']);

expect(result.current.someValue).toBe('whatever')

There is very little information to draw a connection between those when reading this test. With an object, you at least get the matching keys to help you (or someone reading the test for the first time) out a bit.

Also consider the mental hoops someone would need to jump through if the arrays had different number of arguments or changing types:

const { result, rerender } = renderHook(useSomeHook, ['value1', true, { key: 5 }]);
rerender(['value2', { key: 10 }]);

There's a lot here that can make spotting the issue with the above more difficult. if the second argument only uses the boolean as truthy, the object will still work. If the third argument has a default value it might not give an undefined error when used. It might even be that the way the hook is written that this is a completely valid way to use this hook and everything is fine.

I'd be interested to hear what you found unintuitive about the current API. Would if help if the docs presented the usage as:

const { rerender } = renderHook(({ arg1 = 'foo', arg2 = 'bar') => useMyHook(arg1, arg2));
rerender({ arg1: 'bar', arg2: 'foo' });

Now that you know my thoughts and design goals, are there any of the suggestions above that appeal to you more or do you have any other suggestions on how the API might be improved while providing the readability and debugability I'm hoping to retain?

[goce-cz]

I currently use local "mutable" variable way (without initialProps) in a multi-prop friendly flavor:

const props = {
  propA: 1,
  propB: 2
}

const { rerender } = renderHook(() => useMyHook(props.propA, props.propB))

props.propA = 3
rerender()

Why I prefer this solution (ordered by importance):

1. no advanced API knowledge needed (easy to understand for newcomers)
2. all props clearly kept together
3. resembles a declarative approach to props
4. no let keyword

Both rerender({...}) and updateProps({...}) are clearly imperative mutations which can become a bit hard to track - you can't simply see the state in the middle of your test.

I like the updateProps() approach as it kinda leverages hook mechanism by itself. However, I will still be using my simple way just because it's that simple.

[mpeyper]

Thanks for the input @goce-cz, you bring up some good points. In hindsight, we probably never would have implemented initialProps and rerender(newProps) and they seem to be the source of some confusion for people starting out.

I've been thinking more about this idea recently where the hooks is wrapped up from (instead of rendered) and used like a normal function

const testMyHook = wrapHook(useMyHook)
let hookValue = testMyHook(1, 2)
hookValue = testMyHook(3, 2)

The part I keep falling down on is how to handle things like the async utils (waitForNextUpdate, waitFor, etc.) and unmount nicely.

The only have two ideas for this right now. First is to decorate the test harness with utility functions:

const testMyHook = wrapHook(useMyHook)
const { triggerAsyncUpdate } = testMyHook(1, 2)

act(() => {
  triggerAsyncUpdate(3)
})

const { someValue } = await testMyHook.waitForNextUpdate()

expect(someValue).toBe('whatever')

I'm not thrilled with this though.

The second idea is to make have the utilities wrap the action, something like:

const testMyHook = wrapHook(useMyHook)
const { triggerAsyncUpdate } = testMyHook(1, 2)

const { someValue } = await waitForNextUpdate(() => {
  triggerAsyncUpdate(3)
})

expect(someValue).toBe('whatever')

I like the look of that one better, but I'm not sure how difficult it will be to implement it as the utility won't have much knowledge of which wrapped hook is being executed to resolve the promise when the update occurs.

[wistoft]

I appreciate the work you do on this project, and that you reconsider improving the API.

I think you are on the right track with wrapHook. Instead of only returning the wrapped hook. It can return the same things as renderHook does. Like this:

Solution 1

const {result, hook, unmount} = wrapHook(useMyHook, 1, 2)

hook(3, 4) //rerender

Suggested implementation

I have implemented this, and find it simple to use. It's implemented as just a new function in the API. So it's opt-in. One can use both the old and the new API. It's built on top of existing API, so is completely compatible. No breaking changes.

Solution 1.1

If wrapHook needs arguments, it can take an object as the first parameter.

const {result, hook, unmount } = wrapHook({hook: useMyHook, wrapper: MyWrapper}, 1, 2)

Solution 1.2

It's also possible to take initial arguments in the options object. It's different from initialProps in the current API. It's the ...args for the hook.

const {result, hook, unmount } = wrapHook({hook: useMyHook, wrapper: MyWrapper, initialArgs: [1, 2]})

Solution 2

If you wish to retain the design goal, that wrapHook doesn't take arguments for useMyHook, like:

const {hook, unmount, rerender} = wrapHook(useMyHook)

unmount() //mustn't be called yet
rerender() //mustn't be called yet

hook(1, 2) //first render

Then unmount, rerender will not be working right away. The returned hook need to be called first in order for something to be unmountable or rerenderable.

I can't tell what is best. But doing first render in wrapHook is more in line with current API, because that's how renderHook works now.

[wistoft]

I've recreated a npm package with a suggestion for a new api