Typescript inconsistency: MutationFn returns a promise that can resolved with void?

[codepunkt]

I'm confused: When can a MutationFn return a promise that is resolved with void?

This forces me to do things like this:

  private handleFormSubmit = async (
    login: MutationFn<ILoginMutation, ILoginMutationVariables>,
    values: ILoginFormValues,
    actions: FormikActions<ILoginFormValues>
  ) => {
    try {
      const result = await login({ variables: values })
      if (result) {
        localStorage.setItem(AUTH_TOKEN, result.data!.login.token)
        this.props.history.push('/')
      } else {
        // WTF. Help plx! When can this happen? What do i do?!?
      }
    } catch (error) {
      handleSubmitError(error, actions)
    }
  }

When i do a blame on src/Mutation.tsx, i can see this snippet being last edited in #1830

export declare type MutationFn<TData = any, TVariables = OperationVariables> = (
  options?: MutationOptions<TData, TVariables>,
) => Promise<void | FetchResult<TData>>;

When i have a look at the changes introduced by the pull request, it looks like this: Promise<FetchResult<TData>>! No clue where the void | came from or why it's there.

/cc @brettjurgens @jbaxleyiii

[brettjurgens]

@codepunkt I'm not entirely sure why it can be void, that was there before #1830.

If I had to hazard a guess I think it's void if an error is thrown and an onError function is provided to the mutation component see here

that being said, @jbaxleyiii will probably know for sure

[codepunkt]

@brettjurgens @jbaxleyiii Yeah, it seems to be void when onError is provided - thanks for the hint!
Still, i don't like the current state - without providing an onError, it will never be void. Can it somehow be based on whether or not an onError is provided?

[alamothe]

The current typing is non-optimal for the most common use case for everyone who uses TypeScript. For every mutation, an additional (useless) check has to be done against void.

I even ended up creating a wrapper component which fixes the typing for me.

I don't think onError use case is common... to be honest I even find the callback functionality confusing.

[codepunkt]

Can you contribute that wrapper, as a comment here, an npm package or a gist?

[alamothe]

Hope it helps!

import * as React from 'react'

type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>>

import {
    Mutation as __Mutation,
    MutationProps as __MutationProps,
    MutationOptions,
    FetchResult,
} from "react-apollo";

// Apollo's typing can also return void (in case of onCompleted/onError callbacks):
// https://github.com/apollographql/react-apollo/issues/2095
// We never use that functionality so fix here
export declare type MutationFn<TResult, TVariables> = (options?: MutationOptions<TResult, TVariables>) => Promise<FetchResult<TResult>>;

type MutationProps<TResult, TVariables> = Omit<__MutationProps<TResult, TVariables>, "children"> & {
    children: (mutateFn: MutationFn<TResult, TVariables>) => React.ReactNode;
}

export class Mutation<TResult, TVariables> extends React.Component<MutationProps<TResult, TVariables>> {

    public render() {
        const {
            children,
            ...otherProps
        } = this.props

        return (
            <__Mutation<TResult, TVariables> {...otherProps}>
                {children as any}
            </__Mutation>
        )
    }
}

[codepunkt]

Two questions remain

1. can this also be void when onCompleted is used, but not onError?
2. what is the easiest way to adopt the typings in order to reflect this?

[zenVentzi]

Still facing the same void issue. Any newer news? Using 2.4.1 react apollo.

[tomaszwylezekperform]

Same here.
I have a situation like (code adjusted to simplify):

  <Mutation<{add: {id}}, {name}> mutation={add}>
   {(addMutation: MutationFn<{add: {id}}, {name}>) => (
    <Form 
      onSubmit={({name}: {name}) => {
        addAreaMutation({ variables }).then(
          ({
            data: {
              add: {
               id
              }
            }
           }) => {
              history.push({
               pathname: '/add',
               search: `?id=${id)}`
              });
            }
        );
      }}
    />
   )}
  </Mutation>

And it prints me error:

TypeScript error: Property 'data' does not exist on type 'void | FetchResult<{add: {id}}, Record<string, any>, Record<string, any>>'.  TS2339

When I remove that 'void' in node_modules in Promise<void | FetchResult<TData>>; it works fine and does not throw any error. Why it's void there? How to handle that?

[variousauthors]

I'm experiencing this issue as well. I'm used to writing code like this:

const result = await mutate(variables)

if (isDefined(result)) {
  // work with the result
}

but now I seem to need to do this:

const result = await mutate(variables)

if (isAFetchResultAtAll(result)) {
  if (isDefined(result)) {
    // work with the result
  }
}

My feeling is that a function with a void return type is saying "do not use the return type of this function". I'm not sure what it means for a function to sometimes be void... If the function throws an exception, then the caller will either catch the exception or it will be re-thrown, in either case the return value is not available I think?

I think this should probably be <undefined | FetchResult<TData>> and you can leave it up to the user to decide whether they want to have --strictNullChecks or not.

For now I will just be casting the return type:

const result = _dangerouslyCastTo<FetchResult>(await mutate(variables))

[wwwebman]

Would be nice if authors could provide us much more examples for render props pattern + typescript. This page includes few, but still not enough I think.

[Jasonmk47]

Going to bump this again since it's still an issue. Can people confirm void can safetly be ignored here?

[developer239]

I understand that the query might fail but that shouldn't prevent us from writing code like this:

I am probably missing something. This can't be the correct solution. 😅If the query fails then auth.setAccessToken is not going to be executed. Why am I forced to do the null check?

[hwillson]

This was fixed in #3334, and will be published shortly.

[Mike-Gibson]

That's great it's been fixed for react-hooks in #3334, but I think this is still an issue when using react-components.

I believe the type is defined here:

react-apollo/packages/common/src/types/types.ts

Lines 140 to 145 in 3c9ac0e

	export declare type MutationFunction<
	TData = any,
	TVariables = OperationVariables
	> = (
	options?: MutationFunctionOptions<TData, TVariables>
	) => Promise<void | MutationFetchResult<TData>>;

[hwillson]

@Mike-Gibson would you mind opening a separate issue to track that change? Thanks!