Apollo-tooling: Generating flow definitions for queries vs schema

[jacobk]

The current flow type generation only generates specific flow types for queries/mutations detected.

I'm curious about if there are any benefits to this approach compared to just generating the flow equivalent of the entire schema. It should be a pretty simple mapping, but maybe I'm missing something.

E.g. if I have a couple of mutations that all resolve to SomeResponseInterface and a specific mutation resolves to SomeImplementationOfSomeResponseInterface I can find use for both the generic and specific types in my code (shared error handling code can be annotate with interface and code dealing with the special fields can be annotated with the other type etc.).

This should also enable any traversals of returned data automatically being type checked even when a query is changed, independent of regenerating flow types (as long as schema stays the same).

The obvious drawback with not-query-specific-annotations is that the flow type-checking doesn't guarantee that I've actually queried for a specific field (but this is currently a little sketchy depending on how diligent(/automated) you are in regenerating the flow annotations since that need to happen for every change in a query).

Maybe the current style of annotations should be accompanied by a complete set of type declarations for the entire schema too?

[jacobk]

FWIW gql2flow takes the schema only approach for comparison.

[martijnwalraven]

Yeah, apollo-codegen has been originally designed to support Apollo iOS and Android, and for those we definitely want to generate query-specific types (see this blog post for more details).

The way I see it, one of the great benefits of GraphQL is that you can ask for the exact data you need, and you are guaranteed to get everything you asked for. And by generating query-specific types or type definitions, you get static type safety for your data access.

You're right that you will have to make sure to regenerate the definitions, preferably automated as part of your build setup. I you feel that interferes too much with your workflow, then a project like gql2flow might be a better fit.

[jacobk]

Thanks! I'm planning on using both approaches and if schema wide generation of flow/ts annotations is out of scope for this repo that's cool.

However I still think it's a great addition to generalize over types in the schema to provide reusable abstractions and helpers not tied to a specific operation.

It would be awesome to have both options part of the same tooling to ensure similar conventions on how to handle custom scalar type, naming conventions etc. It would also be possible to make sure that the two sets of annotations interop. e.g. if a response is actually a generic MutationReponse the specialized type for the operation would need to either be a subtype or a union of that type etc.

[martijnwalraven]

It's not necessarily out of scope, but I'm mostly focused on the native clients myself. I can see how some generalization might be useful, but I have trouble seeing how to do that in a type safe way.

Are you proposing that query-specific types should be subtypes of schema types? Can you give an example of how that would work?

[jacobk]

I'm not sure I phrased that correctly but here's an example of what I'm planning to do. I will play around with it later today, maybe there's no issue. Please see comment in last code-snippet below.

Example schema

interface MutationResponse {
  ok: Boolean!
  message: String!
  errors: [Error]
}

type UserResponse implements MutationResponse {
  ok: Boolean!
  message: String!
  errors: [Error]
  user: User
}

type VerifyUserPayload {
  clientMutationId: String
  verifyUser: UserResponse
}

input VerifyUserInput {
  clientMutationId: String
  token: String!
}

type Mutations {
  verifyUser(input: VerifyUserInput!): VerifyUserPayload!
}

Generated by gql2flow:

export type MutationResponse = InviteResponse | RoleResponse | UserResponse | StatusResponse;

export type VerifyUserInput = {
  clientMutationId: ?string;
  token: string;
}

export type VerifyUserPayload = {
  __typename: string;
  clientMutationId: ?string;
  verifyUser: ?UserResponse;
}

export type UserResponse = {
  __typename: string;
  user: ?User;
  message: string;
  errors: ?Array<Error>;
  ok: boolean;
}

Generated by apollo-codegen:

export type VerifyUserInput = {
  clientMutationId: ?string,
  token: string,
};

export type VerifyUserMutationVariables = {
  input: VerifyUserInput,
};

export type VerifyUserMutation = {
  verifyUser: ? {
    verifyUser: ? {
      user: ? {
        id: string,
      },
      ok: boolean,
      message: string,
      errors: ?Array< {
        field: string,
        errors: ?Array< ?string >,
      } >,
    },
  },
};

Some code

// 
// This uses union from gql2flow (it can technically be generated as
// flow interface too i think)
//
checkResponse = (response: MutationResponse) => {
  // 
  // Type check to work with responses from multiple different operations
  // using same response interface
  //
  if (response.ok) { 
    ...
  }
}

//
// `variables` and returned `data` is annotated using the apollo-codegen
// generated types.
//
// Generic handling of data should be annotated according to schema
//
mutate({ variables }: { variables: VerifyUserMutationVariables })
  .then(({ data }: { data: VerifyUserMutation }) => {
    //
    // Not sure if fields can be safely declared as types from schema or 
    // if the inline types from apollo-codegen may conflict
    //
    // But I want to be sure that the following works
    //
    const payload: VerifyUserPayload = data.verifyUser
    const response: UserResponse = payload.verifyUser;
    checkResponse(response);
  })

It's not clear in my mind how flow behaves in the situation above. Maybe it's straight forward to mix two annotations or maybe some type casting is needed. But the definitions from apollo-codegen will always be a subset of the schema-based annotations and maybe not include all the fields.

From the example above I also need to deal with some identical annotations being generated by both tools e.g. export type VerifyUserInput.

[jacobk]

It seems that flow wont meaningfully let me do the above, it errors out similar to:

.../index.js:109
109:           const payload: verifyUserPayload = data.verifyUser;
                              ^^^^^^^^^^^^^^^^^ property `clientMutationId`. Property not found in
109:           const payload: verifyUserPayload = data.verifyUser;
                                                  ^^^^^^^^^^^^^^^ object type

It's possible to get around issues like that by first casting a value to any and then to another type;

const payload: VerifyUserPayload = (data.verifyUser: any);

Which should technically be safe in these situations.

I'm not sure if this could somehow be aligned better if the same tool would have generated both sets of annotations. Maybe it's trying to achieve to different, conflicting, things. But then again, I've seen some pretty funky flow definitions here and there :man_dancing:

[lewisf]

I don't think we actually want to necessarily do what gql2flow does because graphql allows you to request a subset of the fields on a type and that should be reflected in the types used throughout your UI.

An alternative solution would be to generate all the nested types so that you're able to reference them.

For instance, instead of:

export type VerifyUserMutation = {
  verifyUser: ? {
    verifyUser: ? {
      user: ? {
        id: string,
      },
      ok: boolean,
      message: string,
      errors: ?Array< {
        field: string,
        errors: ?Array< ?string >,
      } >,
    },
  },
};

We might do something like:

export type VerifyUserMutation = {
   verifyUser: VerifyUserMutation_verifyUser
};

export type VerifyUserMutation_verifyUser = ? {
  verifyUser: VerifyUserMutation_verifyUser_verifyUser
}

export type VerifyUserMutation_verifyUser_verifyUser = ? {
    user: VerifyUserMutation_verifyUser_verifyUser_user,
    ok: boolean,
    message: string,
    errors: ?Array<VerifyUserMutation_verifyUser_verifyUser_error>,
},

export type VerifyUserMutation_verifyUser_verifyUser_user = ? {
   id: string,
}

export type VerifyUserMutation_verifyUser_verifyUser_error = {
    field: string,
    errors: ?Array< ?string >,
}

that way you could type your payload as follows:

const payload: VerifyUserMutation_verifyUser = data.verifyUser;

It is a little verbose but should give a lot of flexibility in terms of how the types can be used. Because it's verbose, however, if we go with this we might have to think a little bit more about our organization of generated types.

[lewisf]

Closing this in favor of #159, since this one is getting out of date. If there is something not addressed in #159, please open another issue!