Shaddix / react-query-swagger

Generates react-query hooks based on Swagger API definitions
MIT License
118 stars 4 forks source link

Subsequent mutations #28

Open danielterwiel opened 1 year ago

danielterwiel commented 1 year ago

I'm running into an issue with the way useMutation is instantiated.

I have a form where I need to make two separate calls, but the last call depends on the response of the former call.

Example:

API:

POST /user
POST /api/{user_id}/foo

Pseudo implementation:

const { mutateAsync: createUser } = useUserMutation()
// NOTE: user_id is required in the generated API client. But we do not have it yet here
const { mutateAsync: createFoo } = useFooMutation(user_id)

const { data: user } = await createUser({ name: 'f00' });
// at this point we should have a user id in `data` and this is the right place to pass it to createFoo()
const { data } = await createFoo(user.id)

Because both of the useMutation hooks are called at the rendering of the component, it shouldn't require the {user_id} argument in the call of the hook. In stead it should be passed to the mutateAsync

Shaddix commented 1 year ago

Hi! Thanks for filing an issue with clear and concise explanation! You are not the first one facing this issue (see #22 which is somewhat similar). So, seems like this scenario should be supported somehow.

I'll take a look into it, maybe something could be done here

Shaddix commented 1 year ago

Turns out it's a bit complex to create a nice API for that.

Let's take your example. Currently, useFooMutation is implemented like that:

function useFooMutation() {
  return useMutation((body) => ApiClient.foo(**user_id**, body);
}

And later, mutation.mutate() takes the same body parameter as the function passed to useMutation.

So, we want to pass not only the body (as it is now), but some additional arguments (user_id) to mutation.mutate().

Unfortunately, the mutation function accepts a SINGLE parameter only, so we can't call mutation.mutate(body, user_id).

The advice from tanstack is to wrap the body and user_id in an object and pass it to mutate: mutation.mutate({body: body, query: { user_id: user_id /* there could be more query parameters here*/ }}).

I don't like this, because it breaks the current API (backwards compatibility), and also if you don't need to pass user_id (arguably the majority of cases), you'd still have to wrap it in the object: mutation.mutate({body: body})

So, we could define another overload: useFooMutationWithParams (ideas about the name postfix are welcome :)). So you could use more or less like you described:

const { mutateAsync: createFoo } = useFooMutationWithParams()
const { data } = await createFoo({ body: /* BODY here (if method requires the body) */, query: { userId: user.id }})

Having 2 methods per mutation will increase the size of generated file, but tree-shaking should eliminate not used 'overloads', so shouldn't be a problem.

Do you think that would work for your case?

danielterwiel commented 1 year ago

Thanks a lot for looking into this so quickly!

I can see how React Query's API design is the constraint here. And the proposed solution to pass the URL Path Parameters upon calling the mutate function would be suffice for me. I don't see any better option either.

I think the most descriptive naming would be something along the lines of useMutationWithUrlPathParams or just useMutationWithParams

SMenigat commented 3 months ago

Thank you so much for implementing this and for generating the ...WithParameters mutation variants ❤️