Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some thoughts compared to graphqlgen #740

Closed
benawad opened this issue Oct 25, 2018 · 6 comments
Closed

Some thoughts compared to graphqlgen #740

benawad opened this issue Oct 25, 2018 · 6 comments
Labels
waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@benawad
Copy link
Contributor

benawad commented Oct 25, 2018

So I just tried out https://github.com/prisma/graphqlgen and this library with the following template: graphql-codegen-typescript-template to create Typescript types for resolvers.

I wanted to give some feedback on some things I liked better about graphqlgen.

  1. If a query/mutation doesn’t have any arguments, graphqlgen generates the args Typescript type as {} which I like better than the template which uses any.

  2. graphqlgen let’s me set what the Context type is upfront, so I don’t have to do QueryResolvers.Resolvers<Context>, I can just do QueryResolvers.Resolvers.

  3. I’m not sure how to handle this case:


So my schema is this:

type Post {
  id: ID!
  title: String!
  content: String!
  published: Boolean!
  author: User!
}

type User {
  id: ID!
  name: String
}

type Query {
  post(id: ID!): Post
}

So in the post resolver it expects me to return an author, but I really want to return a authorId and have the Post.author resolver use the authorId to fetch and return an author. graphqlgen solves this by allowing you to create your own typescript types and it will use those.

@darkbasic
Copy link
Contributor

darkbasic commented Oct 25, 2018

Regarding number 3 I suggest you to have a look at the graphql-codegen-typescript-mongodb-template: https://github.com/dotansimha/graphql-code-generator/tree/master/packages/templates/typescript-mongodb

It allows you to define the following schema:

type Post @entity {
  id: ID! @id
  title: String! @column
  author: User! @column(overrideType: "string") @map(path: "createdBy")
}

It will generate the following types:

export interface Post {
  id: string;
  title: string;
  author: User;
}

export interface PostDbObject {
  _id: ObjectID;
  title: string;
  createdBy: string;
}

So you can use Post for the object returned by the resolver and PostDbObject for the object returned by the database.

@kamilkisiela
Copy link
Collaborator

Hi @benawad

  1. it's something we can fix asap.
  2. It's is not implemented but we can do it :)
  3. it's possible, you can overwrite all: parent, context, returned value.

Each field has it's own type that looks like this:

export type PostResolver<R = Post | null, Parent = any, Context = any> = Resolver<R, Parent, Context>;

So you can:

const postResolver: PostResolver<string>;

@benawad
Copy link
Contributor Author

benawad commented Oct 25, 2018

@darkbasic thanks, I'll check it out that looks good

@kamilkisiela sweet

@dotansimha
Copy link
Owner

dotansimha commented Oct 26, 2018

Thank you @benawad !

Regarding 1, @kamilkisiela already did a fix for it: #741 . We loved this feedback but actually thought to take it a step further and to use never in this case instead of {}, because this way it will enforce that args is not in use, because it’s empty for sure.

Regarding 2, As @kamilkisiela said, we have support for this use case, any you can override it easily. If you wish to change the default value, we added support for this in this PR: #743

Regarding 3, as @kamilkisiela mentioned above, you can override every type because we are using generics in the generated resolvers and we think it’s a nicer solution to that issue.
But! I also want to add that think this approach of returning the id from a query resolver might not be that efficient and good practice, because this way you have to fetch the whole object by id in each Post. field, instead of fetching it only once. we would encourage you not to do it and same goes for our users.

The changes I mentioned above are in master, and available now to test in alpha version:

Successfully published:
 - graphql-code-generator@0.13.0-alpha.34c9c8c7
 - graphql-codegen-compiler@0.13.0-alpha.34c9c8c7
 - graphql-codegen-core@0.13.0-alpha.34c9c8c7
 - codegen-templates-scripts@0.13.0-alpha.34c9c8c7
 - graphql-codegen-apollo-angular-template@0.13.0-alpha.34c9c8c7
 - graphql-codegen-graphql-files-typescript-modules@0.13.0-alpha.34c9c8c7
 - graphql-codegen-introspection-template@0.13.0-alpha.34c9c8c7
 - graphql-codegen-typescript-mongodb-template@0.13.0-alpha.34c9c8c7
 - graphql-codegen-typescript-react-apollo-template@0.13.0-alpha.34c9c8c7
 - graphql-codegen-typescript-resolvers-template@0.13.0-alpha.34c9c8c7
 - graphql-codegen-typescript-template@0.13.0-alpha.34c9c8c7

@dotansimha dotansimha added enhancement waiting-for-release Fixed/resolved, and waiting for the next stable release labels Oct 26, 2018
@benawad
Copy link
Contributor Author

benawad commented Oct 26, 2018

@dotansimha I'm using dataloader to batch requests, so it will only be 1 db call to fetch all the posts. Would you suggest taking a different approach?

@dotansimha
Copy link
Owner

@benawad If you are using dataloader to batch requests then it's great :) Then your resolver should expect to get then full object and not just the id, but the linked resolvers should return string in order to fetch it from the dataloader.

Then you should be able to use the generics to override the return value of each resolver, and set it to string. And with the latest version you can override the Parent value (using generics).

Also, @kamilkisiela added a mappers features in this PR, so now you can also custom mapping between GraphQL types and TypeScript interfaces, and this way it will override it in the whole generated file (and then you don't have to use generics each time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

4 participants