[v3] allow multiple arguments on mutation function

[stefanprobst]

Describe the bug

i would like to be able to pass multiple arguments (TVariables[]) to a mutate function, which currently fails with a type error. while it is possible to pack all arguments in one object, i think it makes sense to keep some things separate, e.g. for passing custom headers.

To Reproduce

1. create hook for mutation:

function useCreatePost() {
  return useMutation(
    (body: { title: string }, headers?: { [key: string]: string }) => {
      return fetch('https://jsonplaceholder.typicode.com/posts', {
        method: 'POST',
        body: JSON.stringify(body),
        headers: {
          ...headers,
          'Content-type': 'application/json',
        },
      }).then((response) => response.json());
    }
  );
}

2. call hook with two arguments:

mutate({ title: 'A post' }, { Accept: 'application/json' })

which will result in type error: "Argument of type '{ Accept: string; }' is not assignable to parameter of type 'MutateOptions<any, unknown, { title: string; }, unknown>'."

Expected behavior
it would be cool if this would work in v3.

related: #1085

[tannerlinsley]

We may consider this in v4. v3 is going to retain the current API style for now.

[boschni]

This is a request that we are getting more often and I do agree it is somewhat cumbersome to define separate interfaces for mutation functions.

It could be worthwhile to check how an alternative implementation might look like:

function addComment(postId: string, comment: sting) {
  return api.post(`/posts/${postId}/comments`, { comment })
}

const { mutate } = useMutation(addComment)

mutate(1, 'comment')

The additional callback support on the mutate function would need to be dropped, but this might not be that bad as mutateAsync can also be used for those cases:

function addComment(postId: string, comment: Comment) {
  return api.post(`/posts/${postId}/comments`)
}

const { mutateAsync } = useMutation(addComment)

try {
  await mutateAsync(1, 'comment')
  console.log('success')
} catch (error) {
  console.log('error')
} finally {
  console.log('settled')
}

What do you guys think @TkDodo @tannerlinsley ?

[Omelyan]

This is a request that we are getting more often

You get this request a lot because functions can have multiple arguments. it's convenient, it's type separation, in many cases wrapping all into one object is a bad idea.

[TkDodo]

This is a 2.5 year old thread. I haven't heard the request in 18 months 🤷

[Shaddix]

I can gently +1 to the initial request (so that there are 2 requests in 18 months!) :)
I'm autogenerating the mutations, and currently my generated mutateFn accepts the single parameter of BODY.

But when an API has query/URL/header parameters the users want to pass them to mutate function as well. So, multiple parameters would be of a great help here.

Here's a user request thread

[githorse]

+1!

[felipedeboni]

@TkDodo, I also would like to be able to call as mutate(1, 'comment').

Apart from Typescript, it gets crystal clear to whoever is reading the code that 1 is the id that goes into url and the second parameter is the request body.

[TkDodo]

So, I am a bit biased here because I like the callbacks on the mutate function, but I'd like to outline why I like them:

1. co-location

We use it a lot to do things like close a dialog when the mutation is successful, for example:

const dialog = useDialog()
const [mutate] = useMutation(...)

<...lots-of-jsx-here...>

<button onClick={() => {
    mutate(variables, { onSuccess: dialog.close })
}}>Do it!</button>

In this case, I like that I can keep the onSuccess close to the onClick handler, so that I know what happens when I click that button. Yes, if I extract it to a function (e.g. submitForm) it's all the same:

const dialog = useDialog()
const [mutate] = useMutation(...)
const submitForm = () => {
  mutate(variables, { onSuccess: dialog.close })
}

<...lots-of-jsx-here...>

<button onClick={submitForm}>Do it!</button>

so this is a minor point, but it would help me co-locate / inline some occasions.

2. closures

const [mutate] = useMutation(...)

<...lots-of-jsx-here...>

{
   items.map(item => (
       <button
           key={item.id}
           onClick={() => {
               mutate(item, { onSuccess: () => doSomethingWithId(item.id) })
           }}
         >
            Update {item.name}
       </button>
   )
}

I think the only solution to use the callback directly on useMutation here would be to extract the <button> to it's own component, pass the item and each instance would do it's own useMutation. Possible, but it seems like an unnecessary overhead just to be able to pass arguments to useMutation which I have in the closure

3. custom hooks

We often move mutations into custom hooks, to provide good namings and keep the actual references to our api out of the view:

// custom hook
const useUpdateItem = () => useMutation(({ id, ...payload }) => axios.post(`/item/${id}`, payload))

//in the view:

const dialog = useDialog()
const [updateItem] = useUpdateItem(...)

<button onClick={() => {
    updateItem(variables, { onSuccess: dialog.close })
}}>Do it!</button>

Without the callback, I would need to pass the dialog to the useUpdateItem hook just to be able to use the callback :/

4. same mutation, different callbacks

the obvious one, but has actually never happened to me:

const [updateItem] = useUpdateItem(...)

<button onClick={() => {
    updateItem(variables, { onSuccess: doSomething })
}}>Do it!</button>

<button onClick={() => {
    updateItem(variables, { onSuccess: doSomethingElse })
}}>Do something else!</button>

----------------

Can I do all of this with the async variant? Absolutely. I just need to wrap everything in try/catch, which some people just don't prefer, me included :)

So, for me, this are all bigger issues than the "we need multiple variables to useMutation" problem.

I think people request it for two reasons:

1. They have two variables, one being e.g. the id that goes to the url and one is the payload, and that should be separated.

This is also possible right now, like I hinted above:

const [mutate] = useMutation(({ id, ...payload }) => axios.post(`/item/${id}`, payload))

mutate({ id: 1, foo: "bar", baz: true })
// vs
mutate(1, { foo: "bar", baz: true })

I don't see this as a huge issue, because mostly, you will have id in a variable anyways, so it comes down to:

mutate({ id, foo: "bar", baz: true })
// vs
mutate(id, { foo: "bar", baz: true })

is that such a big deal, or am I missing something?

Imo, named parameters are even a good thing for readability, and I like that this is enforced by useMutation:

mutate(id, "foo", true, bar, 5)

yes, just don't do this if you don't like it. 😅

2. keeping things separate, like in the above example (body and headers):

mutate({ title: 'A post' }, { Accept: 'application/json' })

an overhead, I agree:

mutate({ body: { title: 'A post' }, headers: { Accept: 'application/json' } })

and I also agree that typing individual parameters is usually easier in TypeScript than typing an object:

(body: { title: string }, headers?: Record<string, string>) => {

vs.

({ body, headers }: { body: { title: string }, headers?: Record<string, string> }) => {

Yuk, that is really ugly. Extracting the type obviously helps, and we are used to do that for React Components all day.

----------------

If callbacks-on-mutate and multiple-variables are mutually exclusive, I would prefer to keep the current notation.

Hope this wasn't too long and helps everyone to come to a conclusion :)

[tannerlinsley]

Sounds like we should keep the current syntax then and keep discussing.

…

On Nov 6, 2020, 12:23 PM -0700, Dominik Dorfmeister ***@***.***>, wrote: So, I am a bit biased here because I like the callbacks on the mutate function, but I'd like to outline why I like them: 1. co-location We use it a lot to do things like close a dialog when the mutation is successful, for example: const dialog = useDialog() const [mutate] = useMutation(...) <...lots-of-jsx-here...> <button onClick={() => { mutate(variables, { onSuccess: dialog.close }) }}>Do it!</button> In this case, I like that I can keep the onSuccess close to the onClick handler, so that I know what happens when I click that button. Yes, if I extract it to a function (e.g. submitForm) it's all the same: const dialog = useDialog() const [mutate] = useMutation(...) const submitForm = () => { mutate(variables, { onSuccess: dialog.close }) } <...lots-of-jsx-here...> <button onClick={submitForm}>Do it!</button> so this is a minor point, but it would help me co-locate / inline some occasions. 2. closures const [mutate] = useMutation(...) <...lots-of-jsx-here...> { items.map(item => ( <button key={item.id} onClick={() => { mutate(item, { onSuccess: () => doSomethingWithId(item.id) }) }} > Update {item.name} </button> ) } I think the only solution to use the callback directly on useMutation here would be to extract the <button> to it's own component, pass the item and each instance would do it's own useMutation. Possible, but it seems like an unnecessary overhead just to be able to pass arguments to useMutation which I have in the closure 3. custom hooks We often move mutations into custom hooks, to provide good namings and keep the actual references to our api out of the view: // custom hook const useUpdateItem = () => useMutation(({ id, ...payload }) => axios.post(`/item/${id}`, payload)) //in the view: const dialog = useDialog() const [updateItem] = useUpdateItem(...) <button onClick={() => { updateItem(variables, { onSuccess: dialog.close }) }}>Do it!</button> Without the callback, I would need to pass the dialog to the useUpdateItem hook just to be able to use the callback :/ 4. same mutation, different callbacks the obvious one, but has actually never happened to me: const [updateItem] = useUpdateItem(...) <button onClick={() => { updateItem(variables, { onSuccess: doSomething }) }}>Do it!</button> <button onClick={() => { updateItem(variables, { onSuccess: doSomethingElse }) }}>Do something else!</button> ---------------- Can I do all of this with the async variant? Absolutely. I just need to wrap everything in try/catch, which some people just don't prefer, me included :) So, for me, this are all bigger issues than the "we need multiple variables to useMutation" problem. I think people request it for two reasons: 1. They have two variables, one being e.g. the id that goes to the url and one is the payload, and that should be separated. This is also possible right now, like I hinted above: const [mutate] = useMutation(({ id, ...payload }) => axios.post(`/item/${id}`, payload)) mutate({ id: 1, foo: "bar", baz: true }) // vs mutate(1, { foo: "bar", baz: true }) I don't see this as a huge issue, because mostly, you will have id in a variable anyways, so it comes down to: mutate({ id, foo: "bar", baz: true }) // vs mutate(id, { foo: "bar", baz: true }) is that such a big deal, or am I missing something? Imo, named parameters are even a good thing for readability, and I like that this is enforced by useMutation: mutate(id, "foo", true, bar, 5) yes, just don't do this if you don't like it. 😅 2. keeping things separate, like in the above example (body and headers): mutate({ title: 'A post' }, { Accept: 'application/json' }) an overhead, I agree: mutate({ body: { title: 'A post' }, headers: { Accept: 'application/json' } }) and I also agree that typing individual parameters is usually easier in TypeScript than typing an object: (body: { title: string }, headers?: Record<string, string>) => { vs. ({ body, headers }: { body: { title: string }, headers?: Record<string, string> }) => { Yuk, that is really ugly. Extracting the type obviously helps, and we are used to do that for React Components all day. ---------------- If callbacks-on-mutate and multiple-variables are mutually exclusive, I would prefer to keep the current notation. Hope this wasn't too long and helps everyone to come to a conclusion :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[aliakbarazizi]

Another great solution that I found in blog is

const useUpdateTitle = (id) => {

  const queryClient = useQueryClient()


  return useMutation(

    (newTitle) => axios.patch(`/posts/${id}`, { title: newTitle }),

    {

      // 💡 response of the mutation is passed to onSuccess

      onSuccess: (newPost) => {

        // ✅ update detail view directly

        queryClient.setQueryData(['posts', id], newPost)

      },

    }

  )

}

You can put id in the useUpdateTitle argument instead of mutation and its look like good.

But he only issue with this approach is you can't use same mutation in one component for different id, you kind of need to create component for each item and call this hook with id.

[TkDodo]

The important part is to type your MutationFunction - the rest can be inferred from there. Assuming that your input is the type DataSchema and the output is Promise<DataSchemaResponse>, I would do:

useMutation({
  mutationFn: (input: DataSchema): Promise<DataSchemaResponse> => ...,
  onSuccess (responseData) => {
    // responseData will be of type `DataSchemaResponse`
  }
})

this becomes obvious if you extract the mutationFn into a separate function:

function updateDataSchema(input: DataSchema): Promise<DataSchemaResponse> {
 ...
}

useMutation({
  mutationFn: updateDataSchema
})

using the notation that passes generics via <> is almost always wrong because it takes away type inference. I go into this topic (albeit about useQuery, but same principles apply) here: https://tkdodo.eu/blog/type-safe-react-query

[Omelyan]

mutationFn: (input: DataSchema): Promise => ...,

yeap, that's right. but the topic is about multiple arguments, so only this first example will work, and DataSchema will be mentioned twice: for mutationFn and updateDataSchema.

[TkDodo]

please show an example that doesn't work with typescript, I don't get it.

[palmmymumu]

To be clear, which mutation hook is better in your opinion?

export const useUpdateAccount = id => {
  const queryClient = useQueryClient()
  return useMutation(data => AccountService.update(id, data), {
    onSuccess: () => {
      queryClient.invalidateQueries(['accounts'])
      queryClient.invalidateQueries(['account', id])
    }
  })
}

And

export const useUpdateAccount = () => {
  const queryClient = useQueryClient()
  return useMutation(({ id, ...data }) => AccountService.update(id, data), {
    onSuccess: (_, params) => {
      queryClient.invalidateQueries(['accounts'])
      queryClient.invalidateQueries(['account', params.id])
    }
  })
}

The second one seems better for me as it can reuse with different id in same component.
But I'm not sure about other downsides.

[TkDodo]

To be clear, which mutation hook is better in your opinion?

depends, and mostly doesn't matter. You'll have access to id wherever you call useMutation and likely wherever you call mutate.