Graphql-code-generator: Suggestion: Use Pick<T, K> to generate query types

Created on 19 Oct 2018  路  25Comments  路  Source: dotansimha/graphql-code-generator

Is your feature request related to a problem? Please describe.
Currently, we just have a copy of original type for a query

Describe the solution you'd like
I think a more clear approach would be to use Pick operator to make a connection to the original type.

So this would be like this

export interface TypeA {
  id: string;
  name: string;
  age: number;
}

export namespace MyQuery {
  export type TypeA = Pick<TypeA, 'id' | 'name'>
}

See it's in action here https://medium.com/@curtistatewilkinson/typescript-2-1-and-the-power-of-pick-ff433f1e6fb

enhancement plugins waiting-for-release

Most helpful comment

Plugin
typescript-client

I've been taking a look to this feature proposal, the idea of using Pick it's quite interesting as y'all have mentioned before. I'd love to discuss what would be the best way implement this, as I have no much idea about the library, I'd really appreciate your opinion & hopefully guidance. I've noticed that the typescript-client is the responsible for generating the types that'd change, refactoring its handlebar template it'd probably work but yep, surely I'm missing a lot of corner cases.

Check this example out and let's discuss how we could solve it, the proposal is tested & working with typescript-react-apollo with no changes in its template.

Query

query allRockets {
  rockets {
    id
    name
    first_stage {
      reusable
      engines
    }
  }
}

Current Types

export type AllRocketsVariables = {};

export type AllRocketsQuery = {
  __typename?: 'Query';
  rockets: Maybe<AllRocketsRockets[]>;
};

export type AllRocketsRockets = {
  __typename?: 'Rocket';
  id: Maybe<string>;
  name: Maybe<string>;
  first_stage: Maybe<AllRocketsFirstStage>;
};

export type AllRocketsFirstStage = {
  __typename?: 'RocketFirstStage';
  reusable: Maybe<boolean>;
  engines: Maybe<number>;
};

Proposal
Iterate over the fields until all of them are primitive types (didn't code it yet but I guess this is possible), it'd require to include the typescript-server in order to generate & pick the interfaces!
Probably the Pick types have to include Maybe types too.

export type AllRocketsVariables = {};

export type AllRocketsQuery = {
  __typename?: 'Query';

  rockets: Maybe<AllRocketsRockets[]>;
};

export type AllRocketsRockets = Pick<Rocket, 'id' | 'name' | 'first_stage'> & {
  first_stage: Pick<RocketFirstStage, 'reusable' | 'engines'>;
};

cc/ @dotansimha @ardatan

Help wanted hahah 馃檹馃徎

All 25 comments

Thank you for your suggestion!
That looks a good idea, but how about nested types.

query SomeQuery {
   foo
   bar
   something {
               x {
                      a
                      b
                  }
               y
    }
}

@ardatan I think it should work the same Pick<SomeQuery, 'foo' | 'something'>

here's the example with nested complex subtype in Typescript playground

the problem is that you can't reference the outer type in namespace (or I don't know how). So I've introduced alias _TypeA for server type to bypass this.

Oh I just understand what you mean! Yes, this will reduce the code size and everything more consistent! I like that idea, and it will also solve the duplicate issues!
@dotansimha What do you think? If it is ok, I can work on it!

It's actually looks good and more clean. I think we might need a lot of effort to support this use case, or I'm missing something? @ardatan @fetis

@dotansimha hi, sorry for a long reply.
I don't think it should be more efforts than now. Instead of generating a new type every time you'll use a Pick statement on existing server type. There might some edge cases of course, but I can't forsee it now.

@fetis There are other things to consider, such as fragment, aliasing and more.
I'm doing some research now to see if how that effects the template.
In general, I agree that this syntax is way better than creating inner types.

@fetis we implemented this concept in the new flow plugins: https://github.com/dotansimha/graphql-code-generator/pull/917
Maybe later we'll refactor the typescript packages to use the same concept :)

Picking this, @dotansimha where could I ping you up, have some questions about how to approach it!

Plugin
typescript-client

I've been taking a look to this feature proposal, the idea of using Pick it's quite interesting as y'all have mentioned before. I'd love to discuss what would be the best way implement this, as I have no much idea about the library, I'd really appreciate your opinion & hopefully guidance. I've noticed that the typescript-client is the responsible for generating the types that'd change, refactoring its handlebar template it'd probably work but yep, surely I'm missing a lot of corner cases.

Check this example out and let's discuss how we could solve it, the proposal is tested & working with typescript-react-apollo with no changes in its template.

Query

query allRockets {
  rockets {
    id
    name
    first_stage {
      reusable
      engines
    }
  }
}

Current Types

export type AllRocketsVariables = {};

export type AllRocketsQuery = {
  __typename?: 'Query';
  rockets: Maybe<AllRocketsRockets[]>;
};

export type AllRocketsRockets = {
  __typename?: 'Rocket';
  id: Maybe<string>;
  name: Maybe<string>;
  first_stage: Maybe<AllRocketsFirstStage>;
};

export type AllRocketsFirstStage = {
  __typename?: 'RocketFirstStage';
  reusable: Maybe<boolean>;
  engines: Maybe<number>;
};

Proposal
Iterate over the fields until all of them are primitive types (didn't code it yet but I guess this is possible), it'd require to include the typescript-server in order to generate & pick the interfaces!
Probably the Pick types have to include Maybe types too.

export type AllRocketsVariables = {};

export type AllRocketsQuery = {
  __typename?: 'Query';

  rockets: Maybe<AllRocketsRockets[]>;
};

export type AllRocketsRockets = Pick<Rocket, 'id' | 'name' | 'first_stage'> & {
  first_stage: Pick<RocketFirstStage, 'reusable' | 'engines'>;
};

cc/ @dotansimha @ardatan

Help wanted hahah 馃檹馃徎

@swcarlosrj that's awesome!!! we would love to get some help on this one!

So basically, today we are generating interfaces and then use them, and instead of doing that, we can use single type/interface that has all other fields nested.
When we first started working on this one, TypeScript did not allow you to access a nested type in a form of MyType['fieldName'] and get the actual interface for it. Now we can do it, and we can do type aliasing so it makes it easier.

So today we have these packages:

  • typescript-common - contains enums, scalars, input types
  • typescript-client - everything related to the clients - which means documents and fragments.
  • typescript-server - types, unions, interfaces.

Basically, what we should have with Pick is:

  • typescript - contains all types, input types, enums, unions, scalars - basically everything.
  • typescript-documents - contains all documents and fragments.

And given the following schema:

type Query {
   user(id: ID!): User
}

type User {
  id: ID!
  name: String
  email: String
}

The typescript will generate only the types, as-is (same as we do today):

export interface User {
  id: string;
  name?: string;
  email?: string;
}

And given a query that uses Query.user, the typescript-documents should use the types from typescript and only Pick the fields according to the query selection set:

query userById {
   user(id: "2") {
      id
      name
   }
}

and the output should be:

export interface UserByIdQuery {
   user: Pick<User, 'id', 'name'>
}

The exact same implementation exists in flow templates here: https://github.com/dotansimha/graphql-code-generator/tree/master/packages/plugins/flow https://github.com/dotansimha/graphql-code-generator/tree/master/packages/plugins/flow-documents

I think we can share almost the same logic with the flow plugin, just need to make sure that we are keeping support for all existing features of typescript. We also need to update typescript-resolvers accordingly.

There are some issues I noticed with Pick approach (and I addressed them in the flow plugins):

  • In case primitive scalar (such as string) we can just use Pick and the field as is.
  • In case of link to other type, we need to open a new Pick and take the fields from there.
  • In case of field aliasing, we need to replace Pick and take only the actual type, with the aliased field name.
  • In case of fragments on interfaces, we need to use &.
  • In case of fragments on unions, we need to use |.

I chose to implement a partial visitor pattern solution in the flow plugin because it makes it easier to write recursive and nested code that generated output, along with some helpers that helps generate wrapping braces and more. I think we can reuse all of those from the flow plugin.

I recommend to use tools like https://www.typescriptlang.org/play/ to check the validity of the output and to verify that the objects are matching the interfaces/types.

@dotansimha just checking this out! Thank so much for all this info, I really appreciate it! I'll give a look tomorrow, I'm pretty sure that it'll help a lot in order to design an efficient solution covering all the use cases!

This is a huge improvement! Can't wait to see it come through! Does this also replace Maybe types with optionals? In code bases like ours, nulls are a bad practice.

Also, it might be nice to consider exposing fragment pick types per #1312

WIP here: https://github.com/dotansimha/graphql-code-generator/pull/1353 :)
@NickClark in the new implementation, we are using both optionals and Maybe, but you can disabled optionals with avoidOptionals and you'll be able to override the definition of Maybe (the default is Maybe<T> = T | null , but you'll be able to set it to Maybe<T> = T).

@dotansimha Thanks! In the Typescript world, nulls are typically bad practice... but GraphQL libs tend to return nulls, so maybe there's nothing I can/should do about it. Unless there was a way for libraries like Apollo to do the conversion for us. Anyways, thanks again! Really excited to see this coming!

In the Typescript world, nulls are typically bad practice...

@NickBolles do you have a proof on that? In connection with GraphQL, I don't think we can get rid of null

@fetis I assume you meant me. Yes. A lot of TS even non-TS projects shy away from null. The Typescript project itself doesn't use null (TS guidelines) and even Crockford doesn't use null anymore (video) That being said... after considering it further... I don't think we can get rid of null in regard to GraphQL,

Thank you @fetis for a great idea. It took some time, but we are finally there.
All plugins are now (1.0.0) using Pick - the generated code is much simpler and it resolved so many issues.

Just trying out the new 1.0.0 release and I have now hard time figuring out how to access deeper field types.

Example. This type is now generated for my GraphQL query:

export type ApartmentsQuery = { __typename?: "RootQuery" } & {
    apartments: Maybe<
        { __typename?: "RootQueryToApartmentConnection" } & {
            edges: Maybe<
                Array<
                    Maybe<
                        {
                            __typename?: "RootQueryToApartmentConnectionEdge";
                        } & {
                            node: Maybe<
                                { __typename?: "Apartment" } & Pick<
                                    Apartment,
                                    "id" | "title"
                                > & {
                                        buildingTypes: Maybe<
                                            {
                                                __typename?: "ApartmentToBuildingTypeConnection";
                                            } & {
                                                nodes: Maybe<
                                                    Array<
                                                        Maybe<
                                                            {
                                                                __typename?: "BuildingType";
                                                            } & Pick<
                                                                BuildingType,
                                                                "name" | "slug"
                                                            >
                                                        >
                                                    >
                                                >;
                                            }
                                        >;
                                    }
                            >;
                        }
                    >
                >
            >;
        }
    >;
};

I'm looping through the data.apartments.edges array and inside it I want to pass node.buildingTypes to an external function. In pre 1.0 versions I could just import the buildingTypes type for the external function argument types but now that all types are inlined like this. How should I go about it?

There are 2 options:

  1. Use MyType['fieldA']['fieldB'] until you reach your type. You can also create aliases by using MyInnerType = MyType['fieldA']['fieldB'].
  2. Use GraphQL fragments. This way the types you need to use internally will point to your fragments, and you can use the generated fragment type.

We know it's not ideal, but eliminating namespaces was very important in order to make the output optimized, and using a single type is important because it makes sure that there are no name conflicts and mis-matches in the generated output.

@epeli

  1. Doesn't seem to work with intersections/unions?

image

  1. Fragments help a bit it's quite awkward still. You need handle bunch of Maybes and __typenames...

It seems that I must do this to be able to reach the type:

type ArrayType<T> = T extends Array<infer V> ? V : never;

type BuildingTypes = NonNullable<
    NonNullable<
        ArrayType<
            NonNullable<NonNullable<ApartmentsQuery["apartments"]>["edges"]>
        >
    >["node"]
>["buildingTypes"];

@epeli I didn't understand if that's an issue. If so, can you please open a new issue with examples?

Will do

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dotansimha picture dotansimha  路  3Comments

steebchen picture steebchen  路  3Comments

mszczepanczyk picture mszczepanczyk  路  3Comments

fvisticot picture fvisticot  路  3Comments

mdaouas picture mdaouas  路  3Comments