Graphql-code-generator: Type 'undefined' is not assignable to baseOptions

[quentinus95]

Describe the bug

When trying to update @vue/apollo-composable from 4.0.0-alpha.4 to 4.0.0-alpha.7, I'm getting the following type errors with graphql-code-generator (on argument baseOptions) for all my mutations:

Argument of type 'UseMutationOptions<DoSomethingMutation, DoSomethingMutationVariables> | undefined' is not assignable to parameter of type 'UseMutationOptionsWithVariables<DoSomethingMutation, DoSomethingMutationVariables> | ReactiveFunction<...>'.
       Type 'undefined' is not assignable to type 'UseMutationOptionsWithVariables<DoSomethingMutation, DoSomethingMutationVariables> | ReactiveFunction<...>'.
                  return VueApolloComposable.useMutation<DoSomethingMutation, DoSomethingMutationVariables>(DoSomethingDocument, baseOptions);

To Reproduce

Use the latest version of graphql-code-generator with @vue/apollo-composable 4.0.0-alpha.7 and any kind of mutation.

Config:

# ...
    plugins:
      - typescript
      - typescript-operations
      - typescript-vue-apollo

Expected behavior

Generated code triggers no compiler error.

Environment:

• OS: Docker
• @graphql-codegen/*: 1.13.1
• NodeJS: 13.1.0

[dotansimha]

It think it might be some changes to the API of @vue/apollo-composable?

I'm not familiar with it, so it might be difficult for us to reproduce.

@quentinus95 can you please create a repo / codesandbox that reproduces it? thank you!

[quentinus95]

@dotansimha here you go: https://github.com/quentinus95/graphql-code-generator-3679

[quentinus95]

@dotansimha do you need me to provide you any other piece of information?

[JoeSchr]

I got it too.

Funnily enough not inside my docker (on a windows host), only when used natively on gallium-os/ubuntu-bionic. both tsc are on 3.8.3, I really don't get it...

EDIT: I now get this errors on all systems..

My packages:

├─ @app/[email protected]
│  ├─ @babel/cli@^7.8.4
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @graphql-codegen/[email protected]
│  ├─ @vue/apollo-composable@^4.0.0-alpha.7
│  ├─ babel-plugin-import@^1.12.0
│  └─ graphql-toolkit@^0.7.1

I also vimdiff the files from the working and non-working, they are exactly the same. still tsc complains on one and not on the other.

[rebolyte]

@dotansimha @iskanderbroere Any updates here? This seems to have been introduced with the overloads added in @vue/apollo-composable v4.0.0-alpha.5 in vuejs/vue-apollo#895.

[iskanderbroere]

You're right it only work with alpha.4, i'll take a look making it compatible with alpha.8

[iskanderbroere]

@bbugh Do you have any ideas on what the generated code could look like? Would you suggest introducing overloads here as well? (like in vuejs/vue-apollo#895)

[Dodobibi]

This workaround work for me :

For query, no need to implement all overloads. just replace

return `type ${reactiveFunctionTypeName} = () => ${operationVariablesTypes}\nexport function use${operationName}(variables${
          operationHasNonNullableVariable ? '' : '?'
        }: ${operationVariablesTypes} | VueCompositionApi.Ref<${operationVariablesTypes}> | ${reactiveFunctionTypeName}, baseOptions?: VueApolloComposable.Use${operationType}Options<${operationResultType}, ${operationVariablesTypes}>) {
          return VueApolloComposable.use${operationType}<${operationResultType}, ${operationVariablesTypes}>(${documentNodeVariable}, variables, baseOptions);
        }`;

by

return `export function use${operationName}(variables: ${operationVariablesTypes} | VueCompositionApi.Ref<${operationVariablesTypes}> | ${reactiveFunctionTypeName}${
          operationHasNonNullableVariable ? "" : "={}"
        }, baseOptions: VueApolloComposable.Use${operationType}Options<${operationResultType}, ${operationVariablesTypes}>=\{\}) {
            return VueApolloComposable.use${operationType}<${operationResultType}, ${operationVariablesTypes}>(${documentNodeVariable}, variables, baseOptions);
          }

type ${reactiveFunctionTypeName} = () => ${operationVariablesTypes}`;

I move type ${reactiveFunctionTypeName} = () => ${operationVariablesTypes} under the useXXXQuery, else, JSDoc for useXXXQuery won't work...

then, "variables" is optional only if they have non-nullable variables, and baseOptions is always optional.
Generated function like this (look at the default initializer of "variables"):

export function useAutocompleteCommunesQuery(
  variables:
    | AutocompleteCommunesQueryVariables
    | VueCompositionApi.Ref<AutocompleteCommunesQueryVariables>
    | ReactiveFunctionAutocompleteCommunesQuery = {},
  baseOptions: VueApolloComposable.UseQueryOptions<
    AutocompleteCommunesQuery,
    AutocompleteCommunesQueryVariables
  > = {}
) {
  return VueApolloComposable.useQuery<
    AutocompleteCommunesQuery,
    AutocompleteCommunesQueryVariables
  >(AutocompleteCommunesDocument, variables, baseOptions);
}

or (if operationHasNonNullableVariable, default variables is not set because it's not optional):

export function useAdministrationsCommuneQuery(
  variables:
    | AdministrationsCommuneQueryVariables
    | VueCompositionApi.Ref<AdministrationsCommuneQueryVariables>
    | ReactiveFunctionAdministrationsCommuneQuery,
  baseOptions: VueApolloComposable.UseQueryOptions<
    AdministrationsCommuneQuery,
    AdministrationsCommuneQueryVariables
  > = {}
) {
  return VueApolloComposable.useQuery<
    AdministrationsCommuneQuery,
    AdministrationsCommuneQueryVariables
  >(AdministrationsCommuneDocument, variables, baseOptions);
} 

I don't know if it's the good way...

For mutation, it's more complicated because we can't know if the user want to pass variables in options parameter of useMutation... Maybe we can test if there is variables and we return the good overload ?

[bbugh]

@bbugh Do you have any ideas on what the generated code could look like? Would you suggest introducing overloads here as well? (like in vuejs/vue-apollo#895)

Good question. I haven't worked on this for a while so I'm going from memory but I'll do my best to answer!

I don't expect that the generator will have to change much because the behavior (as far as I know, I'm still on alpha4) hasn't changed, the typing just got more specific and correct. I believe that @Dodobibi is on the right track by assigning defaults if something isn't specified. I think they should be optional and not defaulted, though, because vue-apollo maps undefined values to the correct default internally:

https://github.com/vuejs/vue-apollo/blob/35ea099209a36f4825988f9c0f4343441d9ff3f7/packages/vue-apollo-composable/src/useQuery.ts#L106-L107

I'm not sure if the generator checks if the type itself requires variables, that would be a useful enhancement to make variables required or not. Queries obviously don't always require variables or options, so either one can be undefined.

I believe that to fix the issue reported by @quentinus95 , the useQuery generator signature should be:

useThing(variables?: <types>, baseOptions?: <types>)
                  ^ maybe whether this is optional is determined by the generator

I hope that helps! I'm happy to answer more questions if needed.

[bbugh]

Personally, though, I think this presents an opportunity for the generator to offer a more developer-friendly API than the vue-apollo one. I would personally like to see the functions take in a single object that requires variables or not depending on the query. There are many times I want to call a query that has no variables but does have options (like for fetchPolicy) and that requires passing useThing(undefined, { options }) which just seems so clunky and y2k. It could generate something like this:

export useThingThatRequiresVariables(baseOptions: RequiredVariablesQueryOptions<x>)

export useThingWithoutVariables(baseOptions?: QueryOptions<x>)

🔥

[dotansimha]

Fixed in 1.13.3

[Dodobibi]

Work partially (when operationHasNonNullableVariable)

The operand ? on variables must be set when all variables are default values, not when operationHasNonNullableVariable
And in xxxxQueryVariables too.
Each variable can be nullable or not, but also optional if it has a default value. If it's not optional, we must pass it in the variables object, also if it's Nullable :

OPTIONALS
query allOptionals($dep: String = "a string value") {
 .....
}

NON OPTIONAL : 
query allOptionals($dep: String!) {
 .....
}

Maybe we can check (in visitor) if variables has default values (like typescript-operation) and if all variables are optionals apply "?" operator on variables parameter

Look at https://graphql.org/learn/queries/#default-variables

Also, can you add (when operationHasVariables is True) :

* @variables ......

in JSDoc just upper :

* @param options that will be passed into the query,

Else : all working 👍

[bbugh]

@iskanderbroere I investigated this further this morning.

I believe the specific issue reported here by @quentinus95 was caused by https://github.com/vuejs/vue-apollo/commit/a9d950156d1f8e89cf8ba3532936b6a09e0b776a, but that incorrect behavior is the result of an underlying incorrect assumption on my part. I believe this will be resolved after the PR for https://github.com/vuejs/vue-apollo/issues/961.

[bbugh]

I submitted a PR for this: https://github.com/vuejs/vue-apollo/pull/962

If y'all have time to check it out and make sure I have it right this time, that'd be really helpful.

[JoeSchr]

what do we need to do to test this PR? update [email protected] and @vue/apollo-composable to your branch?

[bbugh]

I don't know very much about mono repos, unfortunately. I'm not sure how to get a version from the vue-apollo repo. yarn link maybe?

[NickBolles]

I think the generator also needs to be updated to include the reactive function part of the useMutation defiintion too

apollo-composable

export declare function useMutation<TResult = any, TVariables extends OperationVariables = OperationVariables>(document: DocumentNode | ReactiveFunction<DocumentNode>, options?: UseMutationOptionsWithVariables<TResult, TVariables> | ReactiveFunction<UseMutationOptionsWithVariables<TResult, TVariables>>): UseMutationReturn<TResult, TVariables>;

Generated useMutation

export function useAddRecipeMutation(options: VueApolloComposable.UseMutationOptionsWithVariables<AddRecipeMutation, AddRecipeMutationVariables>) {
            return VueApolloComposable.useMutation<AddRecipeMutation, AddRecipeMutationVariables>(AddRecipeDocument, options);
          }

this part in particular

options: VueApolloComposable.UseMutationOptionsWithVariables<AddRecipeMutation, AddRecipeMutationVariables>

should probably be more like this:

options?: UseMutationOptionsWithVariables<AddRecipeMutation, AddRecipeMutationVariables> | ReactiveFunction<UseMutationOptionsWithVariables<AddRecipeMutation, AddRecipeMutationVariables>>

[bbugh]

@NickBolles if I understand what you're suggesting, the patch in https://github.com/vuejs/vue-apollo/pull/962 should address it. The vue-apollo maintainer has been MIA for a while, which unfortunately happens pretty often. Hopefully this will get merged soon as vue-apollo alpha is currently borked for TypeScript users.