TypeScript: mutate() type definition is broken

[mrlubos]

Describe the bug
The mutate() method from useMutation() hook has incorrect type definition allowing us to write potentially buggy code.

To Reproduce
Steps to reproduce the behaviour:

1. Define useMutation variables type as an object with a required key.
2. Call mutate() without any arguments.
3. Watch the world burn.

Expected behaviour
mutate() should not be allowed to be called with undefined (no arguments) when variables type definition is an object (and potentially ever).

Desktop (please complete the following information):

"react-query": "2.15.4"

Additional context
Reproducible error in CodeSandbox

[mrlubos]

Hey @boschni @tannerlinsley, are you able to advise on this issue please? I see all other issues got a reply so now I feel excluded 😪

[boschni]

Hi @mrlubos! Unfortunately I have not been able to fix this issue yet in a backwards compatible way. The variables argument is optional and generic, but could not get the overloads to make a distinction between functions with or without arguments. Suggestions are welcome

[JakeGinnivan]

Could you elaborate on this? Happy to try and help figure a solution, but why can't the optional flag be removed from https://github.com/tannerlinsley/react-query/blob/master/src/react/useMutation.ts#L132 to fix this issue?

[boschni]

If you remove the optional flag then it becomes mandatory to provide variables, even if the mutation function does not accept any variables.

[mrlubos]

For what it's worth @boschni, I have some code where I have to explicitly pass undefined to functions and I don't think it's that much of a pain. I am not aware of a TypeScript solution that would allow us to define optional arguments via union (e.g. string | undefined) and call functions without any arguments. If anyone knows I'd be curious to find out, but otherwise, requiring explicit undefined in mutate(undefined) would work for me.

[mrlubos]

@boschni not sure, I can try to check. Thanks! Are you able to fix this at least as a breaking change? If so, how did you do it? This feels like it can really trip people up, I suppose it will be an issue with any non-primitive parameter.

[tannerlinsley]

For now, we are not tracking type changes as breaking (otherwise, every type change would result in a new major semver). They are instead tracked as patch versions with the expectation that developers should be locking their typescript projects into a specific version and upgrading when convenient.

As for this issue, we're still open to suggestions, but I'm going to move this to a discussion.

[mrlubos]

I am not sure if others are aware of this assumption on your side, even though that is what I'm doing. At the same time, I think this is different than the issue I reported the other day, as the former issue was merely an inconvenience preventing me from writing valid code, but this issue allows me to write invalid code. It's a tricky one!