Skip to content

MutationFunction optional parameters #4264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
logan272 opened this issue Oct 5, 2022 · 8 comments
Closed

MutationFunction optional parameters #4264

logan272 opened this issue Oct 5, 2022 · 8 comments
Labels
help wanted Extra attention is needed types

Comments

@logan272
Copy link

logan272 commented Oct 5, 2022

Describe the bug

useMutation does't not persist the signature of MutationFunction. is it make sense and possible to persist MutationFunction optional parameters in muration.murate?

import { useMutation } from '@tanstack/react-query';

const updateRecord = async (arg1?: number) => {
  return arg1;
};

const mutation = useMutation(updateRecord);

mutation.mutate(); // type error
mutation.mutate(undefined); // ok

feel redundant that i have to write mutation.mutate(undefined); instead of just mutation.mutate().

Screen Shot 2022-10-05 at 15 21 31

Screen Shot 2022-10-05 at 15 21 38

See also

Treating undefined parameters as optional

Your minimal, reproducible example

https://codesandbox.io/s/zen-fire-9iu1er?file=/src/App.tsx

Steps to reproduce

--

Expected behavior

See above

How often does this bug happen?

No response

Screenshots or Videos

No response

Platform

  • MacOS

react-query version

4.9.0

TypeScript version

4.7.4

Additional context

No response

@logan272 logan272 changed the title Persist MutationFunction optional parameters (Treating undefined parameters as optional) Persist MutationFunction optional parameters Oct 5, 2022
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 5, 2022

plase make sure to save the sandbox. It's empty when I open it!

@TkDodo TkDodo added the types label Oct 5, 2022
@TkDodo TkDodo changed the title Persist MutationFunction optional parameters MutationFunction optional parameters Oct 5, 2022
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 5, 2022

so this works:

arg1?: number | void

https://codesandbox.io/s/thirsty-andras-iw67ud?file=/src/App.tsx:105-125

@logan272
Copy link
Author

logan272 commented Oct 5, 2022

Thanks for your reply, yeah, arg1?: number | void works. But change the type of updateRecord may not always be possible(or i have to wrap the original mutation function). for example, if updateRecord lives in a third party library.

@TkDodo TkDodo added the help wanted Extra attention is needed label Oct 5, 2022
@logan272
Copy link
Author

logan272 commented Oct 5, 2022

honestly, at this point i'm not sure what's the difference between declaring a function parameter as optional and declaring a function parameter as void

function fa(a?: number) {
  return a;
}

function fb(a: number | void) {
  return a;
}

fa(); // ok
fa(undefined); // ok
fb(); // ok
fb(undefined); // ok

@TkDodo
Copy link
Collaborator

TkDodo commented Oct 5, 2022

yeah I get that. I think right now, variables need to be required and we work around this with the void type as the default for the TVariables generic:

TVariables = void,

The function is then constructed like so:

> = (
...args: Parameters<MutateFunction<TData, TError, TVariables, TContext>>
) => void

I think mutations without variables are the exception - a DELETE request comes to mind. I haven't seen one with only one optional parameter though.

What you can do is wrap and unwrap it in an object if you can't change the type of the function that make the request itself:

const mutation = useMutation((params: { arg1?: number } | void) =>
  updateRecord(params?.arg1)
);

mutation.mutate(); // ok
mutation.mutate(undefined); // ok
mutation.mutate({ arg1: 5 }) // ok

see: https://codesandbox.io/s/summer-monad-w3isyh?file=/src/App.tsx

it takes away the option to just call .mutate(5) but I think it's a good tradeoff.

Otherwise, if you or someone else wants to take a stab at improving the types, PRs are welcome.

@logan272
Copy link
Author

logan272 commented Oct 5, 2022

I tried something like this:

export type MutateFunction<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> = TVariables extends undefined ? {
  (
    variables?: TVariables,
    options?: MutateOptions<TData, TError, void, TContext>,
  ): Promise<TData>
} : {
  (
    variables: TVariables,
    options?: MutateOptions<TData, TError, TVariables, TContext>,
  ): Promise<TData>
}

But got few type errors in other files. i'm just get started with react-query and don't know how to resolve those type errors.

@TkDodo thanks for you quick reply ❤️

@logan272 logan272 closed this as completed Oct 5, 2022
@logan272
Copy link
Author

logan272 commented Oct 5, 2022

intuitively, TVariables should be an optional parameter in MutateFunction. If TVariables is optional, then we can write mutation.mutate(options) instead of mutation.mutate(undefined, options). you just mention that

mutations without variables are the exception.

the statement may not be true when building a library that uses react-query as the underling layer for async state management.

@logan272 logan272 reopened this Oct 5, 2022
@TkDodo
Copy link
Collaborator

TkDodo commented Oct 28, 2022

If TVariables is optional, then we can write mutation.mutate(options)

This would need overloads and runtime checks that we cannot do because we are not in control of the variables.

What would a call to:

mutate({ onSuccess })

be at runtime? Is this no variables + options, or is it just variables, no options? It's ambiguous.

For useQuery, the overloads only work because we know that the key is always an array, and that the queryFn is always a function, and that options are always an object (at runtime). And still, we want to move away from overloads.

So no, I don't see a good way to make that work.

@TkDodo TkDodo closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed types
Projects
None yet
Development

No branches or pull requests

2 participants