Why is client nullable in MutationResult type?

[fromi]

Using Typescript, I notice that when I use useQuery, I collect a non nullable 'client' property.
However, with useMutation, it is nullable.

This does not seems right: to get a mutation result, one must have an Apollo Client.
Is this intended or is it a mistake in the typing?

[fromi]

I have another related question:
When I do this:

  const [addTodo] = useMutation<{ addTodo: Todo }>(CREATE_TODO, {
    update: (cache, {data}) => {
      // data can be null or undefined: why?
    }
  })

The data passed in the update function can be null or undefined. Why is that?

[jeffreyffs]

@fromi Default graphql behaviour is null | T if you don't specify it is a required field. Apollo then adds on the possibility of undefined if the data hasn't loaded yet. Hence you end up with null | undefined | T. If you update your schema to say the field is required (adding a !), you will end up with undefined | T

[fromi]

@jeffreyffs Here we are inside an "update" function of a mutation: why can it be undefined? If we run the function, it mean that it has returned, so the data has loaded.
Regarding the 'null', I already have ! everywhere in my schema. This mutation returns a non nullable object, so data should not be null either.
See type MutationUpdaterFn in the source code: mutationResult is a FetchResult, whereas I think it could be some "ResolveFetchResult" where data cannot be null.

[jeffreyffs]

You've forced the type of your useMutation here with useMutation<{ addTodo: Todo }>... there is no! so, it is nullable as far as the mutation is concerned.

I could see an argument for having it not be undefined though. It will never be in a loading state, as you have already finished the mutation.

[fromi]

I am not exactly forcing the type, I am specifying it. Otherwise data has "any" type, which indeed cause no warning or compilation error of any kind, which is not what I want.
I cannot specify a type for data which is not nullable, it is declared as so in the framework.

[hwillson]

Let us know if this is still a concern with @apollo/client@latest - thanks!

[fromi]

There is no change in the latest version of Apollo:
The type MutationUpdaterFn defined here uses the type FetchResult defined here, which data property can be undefined.
In the context of a mutation updater function, data is never null so the typing is no has good as it can be.

[hwillson]

Happy to look at a PR that addresses this @fromi - thanks!

[emilio-martinez]

@hwillson can you share what the rationale is behind the data property in FetchResult being potentially undefined or null? I'm happy to put a PR in place for that, but I assume it was set that way for a reason? Or is it just an oversight?

[apieceofbart]

I think this is a bug - I don't see a case where mutationResult in useMutations' hook can be undefined and clearly apollo authors did not provide any such case. It looks like someone was lazy writing the types definition and just added null | undefined without much thinking

[benjamn]

This has been fixed since @apollo/client@3.1.0, thanks to #6617.

[effektsvk]

That PR only fixes the client being optional, data property is still optional inside of update function. Is it safe to just assume that data will be always there?

[tetchel]

As pointed out above, I think this issue should still be open unless a reason is given why .data is nullable:

Types as pasted from my @apollo/client: "3.5.10"

export interface FetchResult<TData = Record<string, any>, TContext = Record<string, any>, TExtensions = Record<string, any>> extends ExecutionResult<TData, TExtensions> {
    // Here
    data?: TData | null | undefined;
    extensions?: TExtensions;
    context?: TContext;
}

[tetchel]

I found this in the documentation, which gives a reason for undefined-ness of data, but that could be fixed with some stricter typing: https://www.apollographql.com/docs/react/api/react/hooks/#data

[IdkMan2Usertive]

The problem still exists and is very bothersome.
As @tetchel mentioned, we could have some stricter typings.