useMutation/useLazyQuery execution functions always requires full variables when using TypedDocumentNode

[maciesielka]

Issue Description

The documentation for hooks like useMutation and useLazyQuery mention that there are two distinct ways to provide variables to the actual request: (1) as an option to the hook itself and (2) as an option to the query function returned by the hook. These allow variable merging such that the query function used at the call-site can override any values provided in the hook, and the docs linked above advertise this as a good way to provide defaults.

The issue I'm running into is using this with Typescript and specifically TypedDocumentNodes that strongly type the variables to the operation, because the hooks always require the fully formed Variables rather than something than might support the merging / default structure you'd expect. Some possible solutions might be something straightforward like allowing a Partial at both sites or something more involved that makes any fields provided in the hook optional in the query function.

Check the quick sketch below as well as the provided Codesandbox to see this issue in more detail:

Imagine we'd like to use the following mutation

mutation AddPerson($name: String, $title: String) {
  addPerson(name: $name, title: $title) {
    node {
      id
      name
      title
    }
  }
}

Usage of useMutation might look something like this, which creates errors in both the hook declaration and the execution usage, despite functionally working properly if you erase those errors with as any.

const [addEngineer] = useMutation(TYPED_DOCUMENT_NODE, {
  // ❌ Property 'name' is missing in type '{ title: string; }' but required in type '{ name: string; title: string; }'.
  variables: {
    title: 'Engineer'
  }
});

// ❌ Property 'title' is missing in type '{ name: string; }' but required in type '{ name: string; title: string; }'.
useEffect(() => addEngineer({ variables: { name: 'Alvin' } }), []);

The same sort of issue occurs for useLazyQuery instances where the provided operation takes variables.

Link to Reproduction

https://codesandbox.io/p/devbox/determined-elbakyan-vrsjcs?workspaceId=59bd876a-c3ad-4ffd-a783-ffc36a11e7d9

Reproduction Steps

Try it out

1. Run the Codesandbox
2. Type in any name and click "Add engineer"
3. Observe that the name provided is correctly added to the list and notice that they're annotated as an Engineer

To the code

1. Jump into the code and find two as any typecasts around useMutation and the usage of the execution function in src/index.tsx
2. Remove both typecasts

Observe type errors despite the working functionality described in the first section above

@apollo/client version

3.11.8

[jerelmiller]

Hey @maciesielka 👋

Good catch! FYI the team is away at a conference this week so we won't be able to look at this until at least next week. Thanks for raising the issue!

[naman1608]

Hi @jerelmiller, can I have a try at this if possible?

[jerelmiller]

@naman1608 absolutely!

[naman1608]

@jerelmiller, I have a doubt if you could take a look? Should MutationFunctionOptions type have a mutation key? The mutate function shouldn't be able to take a mutation right? It's also not mentioned in the docs - https://www.apollographql.com/docs/react/data/mutations#options

[phryneas]

Hi @maciesielka and @naman1608,
I'm sorry that I do have to go back on this a bit:

As these "double options", especially the merging behaviour between them, are very confusing, we're actually considering to deprecate or even remove the variables option on the useMutation/useLazyQuery hooks and only allow variables to be passed into the respective execute function.

In that context, I'm honestly very reluctant towards any type changes here now - we want to start work on 4.0 very soon, and it would be very confusing for users if we went back-and-forth on this so quickly.

[maciesielka]

Okay that makes sense if improving the typing experience here might encourage usage of functionality that's on its way out. I'll try and soft-deprecate usage of the variable merging in my projects until y'all get around to officially deprecating it.

[naman1608]

Yup makes sense, thanks @phryneas!!