RC3 query options don't allow for strongly typed variables

[bbugh]

Hi, I appreciate the attempts to improve typings, but it's worse than before, at least for smart queries. The issue that I failed to fix correctly in #726 is still in RC3, as more things have lost strict correct typing. I'm excited for 3.0 release and want to help ensure the typescript types are as correct as possible before release.

Issues

1. VueApolloQueryDefinition allows OperationVariables for the variables, which means that we can no longer strongly type variables. OperationVariables is just a generic index signature. This type might as well be any.

2. We also can't specify what kind of query definition is with VueApolloQueryDefinition, so the value of update is a generic any, rather than strongly typed. update: ((data: any) => any).

Previously, we could define what the variable type and the result type were, which allowed strong typing: VueApolloQueryOptions<EquipmentQueryVariables, EquipmentQuery>. But now the generalized VueApolloQueryDefinition just allows any for basically everything.

I definitely appreciate an attempt to improve the typings, but this is a big step backwards for type safety. any is an enemy!

Examples

< rc3, this strongly typed returns an EquipmentQuery in Update, and the variables are strongly typed for EquipmentQueryVariables (supposed to be, see #726).

  get (id: ID, type: EquipmentTypes): VueApolloQueryOptions<EquipmentQueryVariables, EquipmentQuery> {
    return {
      query: require('@/queries/gql/equipment.gql'),
      variables: { id: parseInt(id), type }, // ❓should be type of EquipmentQueryVariables (#726)
      update (data) {} // ✅ correctly inferred (parameter) data: EquipmentQuery
    }
  },

rc3 +, everything is basically "any"

  get (id: ID, type: EquipmentTypes): VueApolloQueryDefinition {
    return {
      query: require('@/queries/gql/equipment.gql'),
      variables: { id: parseInt(id), type },  // ❌inferred as any key signature
      update (data) {} // ❌inferred as any
    }
  },

Purpose

We auto-generate our type definitions from our graphql schema using graphql-codegen, much like someone would with React. All of our queries (and mutations, subscriptions) are strongly typed.

• variables being strongly typed is very important, as if we make changes to our server schema, we want to catch issues in our queries. This has been the source of at least two production issues now, which is what I tried to fix in Fix VariableFn<V> type returns Object instead of V #726.
• update being strongly typed is very important for the same reason - making changes to the schema types should result compilation errors.

Thanks for reading.

[Akryum]

1. You can use any type that is compatible with OperationVariables, which means any object-like type will be fine. So no issue here.

2. You can pass the Result type in VueApolloQueryDefinition which is generic. Currently you also need to pass the component type, but I'm looking at ways of removing this requirement.

[Akryum]

Note: Improvements to this typing are blocked by microsoft/TypeScript#33392

[bbugh]

Hey @Akryum thanks for the reply! 👋

You can use any type that is compatible with OperationVariables, which means any object-like type will be fine. So no issue here.

While it is technically true that you can pass in any object, this assertion is problematic in other ways, which I'll demonstrate below. Strong type safety is definitely preferred to "you can pass in anything". 😎

You can pass the Result type in VueApolloQueryDefinition which is generic. Currently you also need to pass the component type, but I'm looking at ways of removing this requirement.

That is useful, thanks for pointing it out. I think the trouble then is that (like #726) we can't strongly type the variables.

For example, we have methods that return queries as I demonstrated above. The reason this works (but isn't exactly correct) is why #726 was broken. However, this is the ideal way it works:

// rc2, this is ideal
get (id: ID, type: EquipmentTypes): 
  VueApolloQueryOptions<EquipmentQueryVariables, EquipmentQuery> {
    
  return {
    query: require('@/queries/gql/equipment.gql'),
    variables: { id: parseInt(id), type } // expects EquipmentQueryVariables
  }
},

With the new API, we do it like this:

// rc3 - variables problem
get (id: ID, type: EquipmentTypes): VueApolloQueryDefinition<Vue, EquipmentQuery> {
  return {
    query: require('@/queries/gql/equipment.gql'),
    variables: { id: parseInt(id), type } // ❌ no longer typed, can put in anything
  }
}

But as the new rc3 types define, variables is basically any, which requires an opt-in to specifically type the variable, which is brittle and easy to forget:

// rc3 - variables manual casting
get (id: ID, type: EquipmentTypes): VueApolloQueryDefinition<Vue, EquipmentQuery> {
  return {
    query: require('@/queries/gql/equipment.gql'),
    variables: ({ id, type } as EquipmentQueryVariables)
  }
}

This also means we can't use the code generator to build a top-level type that applies everywhere, because anywhere we use that type, we still have to manually specify the variables interface.

// rc3 - missing variables definition 
type EquipmentQueryDefinition = VueApolloQueryDefinition<Vue, EquipmentQuery, ...❌>

const query: EquipmentQueryDefinition = { 
	variables: { } // have to manually cast this again :(
}

Moreover, the return value of get()'s variables is still OperationVariables, even if we cast the type in the definition.

// rc3 - even if casting variables at definition, the type is wrong
const result = equipment.get('id', EquipmentTypes.XXX)
result.variables // ❌ This is still OperationVariables :(

I hope that this makes sense. The hope is to be able to specifically type the variables, so we don't have to remember to do a manual casting in every single place that we use the queries. Something like this:

// Usage
const query: VueApolloQueryDefinition<Vue(?), EquipmentQuery, EquipmentQueryVariables>

// or with the code generator output:
const query: EquipmentQueryDefinition

For reference, Apollo does define OperationVariables as the default, but their base type QueryBaseOptions allows you to specify an alternative:

https://github.com/apollographql/apollo-client/blob/7a2067e33f748372aa6342ef0a097679e3239d29/packages/apollo-client/src/core/watchQueryOptions.ts#L47-L51

export interface QueryBaseOptions<TVariables = OperationVariables> {
  // ...
  variables?: TVariables;

As does the MutationBaseOptions:

https://github.com/apollographql/apollo-client/blob/7a2067e33f748372aa6342ef0a097679e3239d29/packages/apollo-client/src/core/watchQueryOptions.ts#L171-L174

export interface MutationBaseOptions<
  T = { [key: string]: any },
  TVariables = OperationVariables
> {
  // ...
  variables?: TVariables;

Perhaps extending the relevant vue-apollo types from the correct Apollo core types would resolve the issue easily.

[Akryum]

You can now use: VueApolloQueryDefinition<Result, Variables>

[bbugh]

Excellent! It works great, thank you. 👏 I appreciate the quick turnaround!

I imagine that you already know that this is broken in variables() again, but the const vm/self = this workaround works fine for now. Hopefully the TS people have a good answer to the ThisType issue.

[Akryum]

You have to use arrow functions if it's already inside a function.