Apollo-tooling: Generated Flow/TypeScript doesn't include the actual queries

[jesseschalken]

Problem

At the moment running apollo-codegen with --target of either flow, flow-modern, typescript or typescript-modern generates only type definitions, but the generated code doesn't include the GraphQL queries themselves, leaving the programmer to get their GraphQL queries into their code by some other means (eg gql tagged templates, importing the parsed contents of .graphql files with graphql-document-collector or webpack-graphql-loader, etc).

This means the programmer has to correlate the query itself with the types generated by apollo-codegen manually, and could get it wrong.

Example:

# getUser.graphql

query getUser($id: Int!) {
  user(id: $id) {
    id
    name
    email
  }
}

apollo-codegen produces:

// graphql_types.ts

export type getUser = {
  id: number,
  name: string,
  email: string,
}

export type getUserVariables = {
  id: number
}

The programmer can write something erroneous like this, and it will still type check.

// some_code.ts

let apollo = new ApolloClient(...)

// This isn't the right type!
type User = {
  firstName: string,
  lastName: string,
}

function getUser(username: string): Promise<ApolloQueryResult<User>> {
  return apollo.query({
    query: require("./getUser.graphql"),
    // getUser() takes an id, not a username!
    variables: {username}
  }) 
}

This is because:

1. ApolloClient.query has type query<T>(options: WatchQueryOptions): Promise<ApolloQueryResult<T>>, which means the output can be used as whatever type the caller wants.
2. WatchQueryOptions.variables has type { [key: string]: any }, so the caller can pass whatever variables they want.
3. WatchQueryOptions.query has type DocumentNode, so the caller can pass whatever GraphQL document they want.

There is no static typing to make sure these three things actually match.

Is there a solution to this? Maybe I am not using apollo-client and apollo-codegen correctly?

Solution?

I would have expected the types in apollo-client to be something like:

class Query<Tvars, Tret> {
  constructor(public document: Document) {}

  // Dummy method to force TypeScript to pay attention to the Tvars and Tret types
  private __nominal(x: Tvars): Tret { return 1 as any }
}

export interface WatchQueryOptions<Tvars, Tret> {
  variables?: Tvars
  query: Query<Tvars, Tret>;
  // ...
}

class ApolloClient {
  // ...
  query<Tvars, Tret>(
    options: WatchQueryOptions<Tvars, Tret>
  ): Promise<ApolloQueryResult<Tret>> {
    // ...
  }
  // ...
}

Then for apollo-codegen to generate something like this:

// graphql_docs.ts

let getUser = new Query<{
  id: Int,
}, {
  id: Int,
  name: String,
  email: String,
}>(gql`
  query getUser($id: Int!) {
    user(id: $id) {
      id
      name
      email
    }
  }
`)

And ApolloClient.query called like:

import {getUser} from "./graphql_docs"

let apollo = new ApolloClient(...)

apollo.query({query: getUser, variables: {id: 1}})

This way the type of the variables, the type of the result, and the query document itself are packaged together, so:

1. The developer doesn't have to import the query documents by some other means. They're there in the output from apollo-codegen.
2. The developer can't misuse the queries with the wrong variables and output types, due to the extra typing in ApolloClient.query, WatchQueryOptions, and output from apollo-codegen.

[lewisf]

To answer: Is there a solution to this? Maybe I am not using apollo-client and apollo-codegen correctly?

I think what you've stated here is along the right lines (and looks like a good next step). We'll have to refine the type output along with the apollo-client project to make this work. Without integrating with apollo-client's generics, there will be room for user error and that sucks :(

However, there are some use cases where people want access to the raw types like when they're defining functions that take the query output as input, they're using fragments, etc. Due to that, I think it's still worth it to keep the current output (raw types) around.

I'd love to continue discussing the correct implementation of this, since I agree that it'll improve the overall usability of apollo-codegen.

[infogulch]

This looks like the best solution. @lewisf maybe a new query method with the additional type checking?

Since it looks like it will require coordination with apollo-client, perhaps an intermediate implementation for the meantime would be good. You could just generate one extra function that takes an apollo instance and the query parameters, and returns the result. Like:

// (getUserVariables and getUserQuery defined just above)

function getUser(apollo: Apollo, variables: getUserVariables):
Observable<ApolloQueryResult<getUserQuery>> {
  return apollo.query<getUserQuery>({
    query: gql`embed query`, // or import it from another file or webpack-loader or ....
    variables,
  });
}

Now as long as you execute the query by calling this function everything is type safe. And this only requires a change to apollo-codegen, and only to generate one more object. I'm not sure what the correct return type signature is, or if it would need to be configurable.

Thoughts?

[leoasis]

It would also be great for this to support react-apollo's HOCs or something that would be suitable to be used there. Not sure how that would look and how it could be such that it's not particularly tied to react-apollo, since it would also be nice to support other UI frameworks as well.

[ecstasy2]

I think that @infogulch would work great. Unfortunately it ties the generated code to apollo client.
I took a stab at creating a quick implementation of this and I am able to produce the below result.

interface GraphqlClient<TVars, TRet> {
  query(options: { query: string, variables: TVars}): Promise<TRet>;
  mutate(options: { mutation: string, variables: TVars}): Promise<TRet>;
}

export type FetchRestaurantsQueryVariables = {|
  menuItemId: string,
|};

export type FetchRestaurantsQuery = {|
  fetchMenuItem: ?MenuItem,
|};

function fetchRestaurants(graphqlClient: GraphqlClient, variables: FetchRestaurantsQueryVariables): Observable<ApolloQueryResult<getUserQuery>> {
  const query = gql`
    query FetchRestaurants($menuItemId: String!) {
      fetchMenuItem(menuItemId: $menuItemId) {
        id
        categoryId
      }
    }
  `;
  return graphqlClient.query<FetchRestaurantsQuery>({
    query,
    variables,
  }
}

The idea is that we should not tie the generated code to apollo-client. This way someone using a different client can always implement the interface.

If this is the direction we want this to go, I would be happy to take a stab at implementing this or something similar.

[infogulch]

That would be awesome in my opinion. (unsurprisingly)

[leoasis]

@ecstasy2 how would this work with apollo bindings like react-apollo that use the client under the hood?

I think we should either:

1. Add targets for different bindings. This has the drawback of having an explosion in the combinations: flow + react, typescript + react, typescript + angular, flow + angular, ...
2. Provide a way to inject targets so that 1. can be different packages. At least that explosion would live in different packages and each one would care about its bindings. Maybe it could also live as part of the official bindings repos? The problem here is figuring out a powerful API to inject targets.
3. Create a Query, Mutation and Subscription type/interface/class that serve as the API that all bindings should use. This one would require all libraries to change to accept this, which I don't know how feasible is. The good thing is that I think the Swift target is doing something like this. Also it would allow us to attach more info to a query, such as the operationId (the hash of the query to be used for persisted queries)

[infogulch]

What about a combination of 2 & 3? Come up with a reasonable interface for 3 and just implement it without worrying too much about adoption. But also expose bindings so users of other clients can inject a shim that converts from whatever their clients expects into into the new interface.

Reasonable?

[leoasis]

@infogulch I think that would be a good idea, I agree that generating the adapters for different libs using the different operation interfaces would be good to get things out quicker than waiting for upstream changes to happen.

[infogulch]

Despite my inexperience in this area, this is something I'd really like. What can I do to help make progress? A more formal proposal? An alpha implementation/PR to play with it? I'm willing to dive in headfirst.

[alamothe]

I feel this does not have to be completely solved by the code generator. The interface to Apollo Client can be improved with function overloading (no need to change the implementation, instead just add an overload which is better typed for our use case). And I believe the Query definition from the original post should also live there. This way, others can also benefit from strong typing, and there is no need for adapters - people can use the original API.

The most important thing for codegen is to export GraphQL query - either with gql or by re-exporting graphql file (when queries are in separate files). We can change its type to fit our needs:

export interface XyzQueryResult ...
export interface XyzQueryVariables ...

// Make sure XyzQuery has the type Query<XyzQueryResult, XyzQueryVariables>
export XyzQuery from 'X.graphql';
// ... or ...
const XyzQuery = gql`...` as Query<XyzQueryResult, XyzQueryVariables>;
export XyzQuery;

(In this solution, we don't create instances of Query class, it is just used as a proxy. But Query definition has to live in Apollo Client, otherwise we will end up creating more and more adapters for each functionality that Apollo provides)

Then Apollo Client could be typed as suggested in the original post.

Arguably, this will tie code generation to Apollo. In all seriousness though, the name of this project is apollo-codegen 😄

Additionally, we can generate React-specific types too (such as XyzProps) if such an option is provided.

Thoughts?

[standy]

Take a look on my PR #423
There is approach with intermediate module, generated code will use it to produce any method for you need:

Generated code:

import codeGenerationModule from '...';
export const doTestQuery = codeGenerationModule<TestQuery, TestQueryVariables, {operationType: "query"}>(
gql`...`,
{operationType: "query"}
);