dotansimha / graphql-code-generator

A tool for generating code based on a GraphQL schema and GraphQL operations (query/mutation/subscription), with flexible support for custom plugins.
https://the-guild.dev/graphql/codegen/
MIT License
10.84k stars 1.33k forks source link

[typescript-resolvers] Generated resolver return types don't allow for async/partial resolution #1219

Closed dobesv closed 5 years ago

dobesv commented 5 years ago

Currently if I write a resolver for a User object, the resolver type just emits a resolver that should return just the User type as defined by the GraphQL type. This doesn't match the reality of a resolver return type because:

  1. fields can be resolved lazily using a resolver defined in a type-specific resolvers file
  2. fields can be resolved lazily by returning a resolver function instead of a field value
  3. the caller might request a subset of fields which we can detect by looking at the provided GraphQLResolveInfo

For that reason we cannot simply use the generated GraphQL types as the return value. Instead we have to return a generated class where each field could be absent, undefined, a value of the expected type, or a resolver function. e.g.

interface UserResolverResult<Context> {
    email?: string | Resolver<string, UserResolverResult, Context>;
    documents?: DocumentResolverResult[] | Promise<DocumentResolverResult[]> | Resolver<DocumentResolverResult[], UserResolverResult, Context>;
    profile?: ProfileResolverResult | Promise<ProfileResolverResult> | Resolver<ProfileResolverResult, UserResolverResult, Context>
}
lgandecki commented 5 years ago

yes.. I just went to the issues to ask about this.

I'm a bit surprised to be honest. How are people even using this? Even in your own example for MongoDB:

type User @entity {
    id: String! @id
    username: String! @column
    email: String! @column
    profile: Profile! @embedded
    friendsCount: Int! # this field won't get a generated MongoDB field
    friends: [User]! @link
}

the friendsCount should be resolved in a field resolver. If you propose to just calculate that in the query that returns the User, then that's a huge graphql anti-pattern, that actually prevents us from benefiting from one of the main points of GraphQL. I hope I misunderstand something!

https://github.com/prisma/graphqlgen gets it right, but it forces the use of namespaces for now.. which means I can't use babel..

lgandecki commented 5 years ago

So far I went around this problem by introducing partials for the return types: https://github.com/dotansimha/graphql-code-generator/compare/master...TheBrainFamily:partialResolution?expand=1

it doesn't give as much assurance as before, but for that I have a graphql layer integration tests with mocked context. (so super flat and simple)

Obviously I wouldn't mean to the PR above to be merged in, it changes the design quite a bit, and I didn't touch the tests. But I'd like to know the thinking and reasoning here.. Something like this could be introduced as an opt-in configuration.

dortamiguel commented 5 years ago

Hey, same problem here. Is really not possible to tell codegen to allow partial resolution?

dobesv commented 5 years ago

You can, of course, edit the template yourself to fix it. But it seems like the built-in template doesn't work this way.

lgandecki commented 5 years ago

editing template makes little sense I think.. since I want to regenarate it everytime I change my schema (and that does happen in real life :-) ). If I have to make the changes manually every time, it's a cumbersome task, prone to "human errors", and I don't really get the typesafety I'm looking for by using this template.

dortamiguel commented 5 years ago

Is there a way in graphql to know which fields are async/partial?

If it can be detected it will be easy to tell in the resolver response which fields are required and which ones not.

dobesv commented 5 years ago

It's not something specified in the graphql schema, which is what this code generator looks at to generate the types. In fact at runtime there's no constraint on whether a field is returned as a value, a promise, or whatever. Required fields do have to be resolved eventually but I don't think this code generator can do the analysis to figure out which resolvers are already present in the type resolvers versus the ones returned as resolve functions in a resolved object.

dortamiguel commented 5 years ago

I see, this makes it hard to type this, uhm.

Maybe the resolver type can take an additional optional 4th param where you can manually specify which fields are partials.

An alternative would be to add an option on the codegen.yml config to make any field on a resolver response to be optional, that would be better than not being able of using the type.

Another third option could be to add a comment on the schema so the generator can know that is a partial, something like this:

type User {
  id: String!
  name: String!
  comments: [Comment!]! # @gql-gen partial
}

Not sure if a comment like that can be read by gql-gen, if not, an actual graphql description could be another option, though it looks hacky.

type User {
  id: String!
  name: String!
  "The comments made by the user. @partial"
  comments: [Comment!]!
}
dobesv commented 5 years ago

Personally I think it is simpler to just mark all fields as partial, at least for now. You still get the benefit of having to return the the correct type of thing if you do return it.

kamilkisiela commented 5 years ago

I don't know if I understand the problem but if a GraphQL Field is non-optional then your field's resolver should always return a value.

Given an example:

type Comment {
  id: ID!
  text: String!
}

type Query {
  comments: [Comment!]!
}

Query.comments returns an array of Comment objects and it shouldn't be null or [null, null, ...], always an array with items. Same with the Comment type itself, id and text should always be non-null.

At least GraphQL Specification says that.

kamilkisiela commented 5 years ago

As described here, the default behavior of Codegen Resolvers plugin expects that your resolvers returns a data with the same structure as defined in Schema.

So again:

type Comment {
  id: ID!
  text: String!
}

type Query {
  comments: [Comment!]!
}
{
 Query: {
  comments() {
    return [{id: 1, text: 'GraphQL is awesome'}];
  }
 }
}

If you want to return a list of IDs instead of entire objects you map Comment type to number (or any other type) so then:

{
 Query: {
  comments() {
    return [1, 2];
  }
 },
 Comment: {
   id: id => id,
   text: id => fetchComment(id).text
 }
}

We call it Mappers and we covered that in "TypeScript Resolvers" chapter in docs.

lgandecki commented 5 years ago

Thanks for the explanation. How would you combine this with types generated for mongo? I don't have access to them before generating types, so I cant map to them. But that's what I get from my services (and indirectly from the DB). Basically, my DB objects are my "parents"

kamilkisiela commented 5 years ago

I don't know if you mean types generated by MongoDB plugin or your own generated types?

If you meant the plugin then you can map to them:

# ...
  config:
    mappers:
      User: UserDBObject # if in the same file
      User: ./mongo#UserDBObject # if in a separate file

I know it's a painful process but maybe in the future we will simplify that, PR welcome (I can help).

lgandecki commented 5 years ago

I see. yeah automating that would be great (so graphql schema is the only source of truth, otherwise things start to mismatch overtime.. ) I guess I could write a script that parses the graphql schema and adds the config mappers part for each entity...

kamilkisiela commented 5 years ago

@lgandecki yeah, iterating over schema.getTypeMap() and filtering those with @entity directive should do the work.

Also, codegen accepts .js files as a config:

// iterate
// filter
// mappers

module.exports = { ... };
kamilkisiela commented 5 years ago

@lgandecki Also, we could change the default behavior of typescript-resolvers based on the config (which we have access to in every plugin), so when typescript-mongodb plugin is used next to typescript-resolvers, we can make typescript-resolvers to use *DbObject types.

I think it's easy to do, if you want to implement something like this in Codegen I can help and answer questions :)

lgandecki commented 5 years ago

I would love to give this a try. seems much better than my approach with the forked codebase above. do you have any specific files that you would recommend to look at first?

On similar note (and sorry for an offtop but you seem knowledgable in this area, and I don't want to create an issue that's more of a question) - is there a reason why enums are treated as strings in DbObjects? this way I get type mismatches. I change them manually with a script (from a string to a defined enum) and everything works just fine , I'm trying to think if there is something I'm missing

nettofarah commented 5 years ago

I'm curious as to what would be a good solution for this too.

We're currently stuck in a scenario where we basically can't have "calculated" fields in our Schema. (only if you make the calculated field optional)

I actually like the idea of using something similar to what @ellipticaldoor suggested here: https://github.com/dotansimha/graphql-code-generator/issues/1219#issuecomment-457961761

Maybe we could have some sort of directive we could use to make certain fields optional, e.g.

type User {
  id: String!
  firstName: String!
  lastName: String!
  fullName: String! @calculated
}

and the generated type:

interface User {
  id: string
  firstName: string
  lastName: string

  // we could either make fullName? an optional field
  fullName?: string

  // or force the property to be required,
  // but accept a resolver function as an option, that way we could typecheck
  // that whatever needs to return `User` could also return the custom resolvers.
  fullName: string | UserResolvers.FullNameResolver
}
lgandecki commented 5 years ago

I agree, @calculated would be great to have. I think then we shouldn't be able to return anything directly for a field like that, but HAD to use a field resolver.

even though @kamilkisiela suggestion is great, I already realized where it falls apart, unfortunately. I have a type with a relation one to many. I keep the relation in a separate mongodb collection. Lets say I have.

type School {
id: ID
name: String
kids: [Kid]
}

but the school object knows nothing about the kids directly (so I can't map on for example an array of kidIds), the way I deal with kids is that I have a School.kids resolver in which I call a repository function that returns me the kids for the school (based on the parent.id which in this case is the schoolId).

@calculated would allow this to work for me.

kamilkisiela commented 5 years ago

Maybe I'm still missing something but if you mark a field as non-optional it should always return a value, if server's logic says the value might sometime be null or undefined then you should change the schema, otherwise field's definition lies about the API.

If you return a different value based on some condition you can use mappers. For example, instead of a string it might return an object with id, so you define a mapper like this: string | { id: string } (or a type, whatever). Now you schema matches resolvers and resolvers matches server's logic.

dotansimha commented 5 years ago

I agree with @kamilkisiela . If a field is declared as String!, then you must return a string from it's resolvers. You can use mappers to change the type if you are doing it in a different way (and if that's a valid use-case, please provide a repository with a complete example, and we'll check it).

nettofarah commented 5 years ago

It’s possible I wasn’t super clear in my example.

But to re-iterate, the current code generation does not allow for calculated fields to exist in the schema.

e.g. code that requires IO calls, async calculation, or straight up logic that isn’t wired up to the object being directly returned by a resolver.

It’s a very common pattern to define a resolver only module for a specific type in GraphQL, especially when you have a type that shows up in multiple places in your Schema. Imagine a User type that gets referenced by many other types.

What we’re all trying to suggest is that the codegen code allows for some flexibility in how these calculated resolvers are represented as types, so you don’t always have to return a full representation of a type when a custom resolver exists.

nettofarah commented 5 years ago

@dotansimha: is it possible to define a mapper that only covers the dynamic fields within a larger type?

dotansimha commented 5 years ago

I think I know what you mean now. Let's say that you have a field myValue: String!, and it must return a string, but that's a a calculated field, and you don't want the codegen to mark it as string because this way, you'll have to include this field in the generated type, right? The actual issue here is not the type generated by typescript-server plugin, but that's related to typescript-resolvers - by default it uses the type generated by typescript-server. If you wish to have a different type - you can use mappers to tell the codegen how your resolvers are implemented.

I think @calculated is a very specific use case, and it should not be part of the codegen's code-base. We aim to provide a general types (with typescript-server). We understand that resolvers are more dynamic and it's return values may be different from the type declared in the schema, so we allow you to customize it in 2 way: either with generics, or with mappers.

Also, mappers are not specific to fields, they are specific to types - if you have a field that has a calculated value, you should create a TypeScript type for the parent type (you can also use the generated one and wrap it with Partial<>), and then point mappers to it.

nettofarah commented 5 years ago

Oh, that sounds like a really sweet idea. Would you be able to give me an example of how to use the mapper type in this scenario?

I’m having a bit of a hard time understanding the type mapper example in the docs, so I guess a more concrete example would go a long way.

I’m happy to try and backport whatever example you could share as part of the documentation for typescript-resolvers :)

kamilkisiela commented 5 years ago
type Query {
  me: User
  users: [User]
  post(id: ID): Post
  feed: [Post]
}

type Post {
  id: ID!
  title: String!
  text: String!
  author: User!
}

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

Query.me gives User Query.users gives User[] Query.post gives Post Query.feed gives Post[] Post.author gives User User.id gives string and receives User

Let's say your object has no id prop, but _id instead.

You define:

interface MyUser {
  _id: string;
  # ... other fields
}

You map User to MyUser (through mappers).

Then your resolvers get that:

Query.me returns MyUser Query.users returns MyUser[] Query.post returns Post Query.feed returns Post[] Post.author returns MyUser User.id returns string but receives MyUser so you define a resolver that returns MyUser._id

lgandecki commented 5 years ago

I think I misunderstand something here... Let's assume:

const schools = [
  {
    name: "firstSchool"
  }
];

const students = [
  {
    school: "firstSchool",
    name: "a"
  }
];
const resolvers = {
  Query: {
    AllSchools: () => schools
  },
  School: {
    students: (school) => {
      return students.filter(student => student.school === school.name)
    }
  }
};
type Query  {
  AllSchools: [School]
}

type School {
  name: String
  students: [Student]
}

type Student {
  name: String
  school: School
}

In this case you might think that I should just add the student ids to the school in an array. But, let's say I want to filter them:

type School {
  name: String
  studentsForGrade($grade: String): [Student]
 studentsBySex($sex: String): [Student]
}

then it really only makes sense for those fields to be calculated in resolvers, and only when needed. What am I supposed to pass from the main school resolver query to map over? :)

dobesv commented 5 years ago

Maybe I'm still missing something but if you mark a field as non-optional it should always return a value, if server's logic says the value might sometime be null or undefined then you should change the schema, otherwise field's definition lies about the API.

This isn't true for GraphQL resolvers. As I said before:

These are not special cases by any means, anything with a relation to other objects or calculated fields might make use of this. These are pretty much the whole point of using GraphQL.

The actual issue here is not the type generated by typescript-server plugin, but that's related to typescript-resolvers - by default it uses the type generated by typescript-server.

Yes, my thought is that typescript-resolvers has to generate its own types so that it can mark every field of every resolved object as optional and possibly an async resolve function.

if you have a field that has a calculated value, you should create a TypeScript type for the parent type (you can also use the generated one and wrap it with Partial<>), and then point mappers to it.

I could do all this by hand but it would be much nicer if things were generated the way I need them by the tool.

nettofarah commented 5 years ago

I 100% agree with @dobesv here.

I love the experience of using typescript-resolcers. It’s amazing to be able to drive my type definitions from the SDL and trust that if my code compiles, there’s really big chance that I did everything right.

Schemas get so big and complicated over time though, that I feel that I would probably need custom mappers for 60% of my types.

At that point is it worth using typescript-resolvers anymore?

I have not been as excited about a tool like I am with gql-gen in a long time, but I’m afraid we’re limiting the “a-ha” moment significantly by ignoring such a core requirement for building GraphQL servers. 😥

dotansimha commented 5 years ago

@nettofarah We are always trying to improve this tool, and we are asking a lot of questions because we want to make sure that we are doing the right thing before modifying the codegen's plugins :)

@dobesv @nettofarah Do you think that adding a configuration for typescript-resolvers that wraps the parent and the return type with Partial<...> will solve this issue?

jacobwgillespie commented 5 years ago

What would be super helpful in my case is for the mapper I specify to be used everywhere - basically I think of a mapper type as the "entrypoint" data that will be provided to a type's resolvers, which could be from a field from any other type. Right now it seems like mappers only apply to one level, so like if I have type A with a field of type B, in the TS types for A's resolvers, they expect an input of AMapperType but expect the field to return a full BType, even if I've defined a mapper.

What I'd like then is if I have GraphQL types A, B, and C and I've defined mapper types for all of them, and if I have the structure A has a B field that has a C field, that all the fields all down the chain are expected to return the mapper type. This seems consistent with the fact that you only define resolvers for a particular GraphQL type once, so they need to be somewhat agnostic to what their parent could be.

ajordanow commented 5 years ago

I think adding ability to specify wrapper type is a great idea, but it should be configurable - not only predefined as Partial<...> (so specifying its source definition should be configurable too - like in mappers).

My current workaround (that does not involve mangling with template files :) ) for the problems mentioned above is:

  1. set config.optionalType to "NestedPartial<T> | null" (so Maybe will be generated as export type Maybe<T> = T | NestedPartial<T> | null;)
  2. use Add plugin to inject NestedPartial definition:
    # ...
    generates:
    path/to/file.ts:
    plugins:
      - add: 'type NestedPartial<T> = {[K in keyof T]?: T[K] extends Array<infer R> ? Array<NestedPartial<R>> : NestedPartial<T[K]>};'

    This allows to return partial results from the resolvers in a type-safe manner. (NestedPartial works as Partial but also allows nested objects to be partially returned.)

kamilkisiela commented 5 years ago

Could one of you prepare a small repository that explains what we're talking about and it would be also something we could base our work on. This would help to implement the solution.

hlehmann commented 5 years ago
type Post {
  id: ID!
  content: String!
  authorId: ID!
  author: User!
}

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

One of the benefits of graphql is to have properties loaded async and on demand with resolvers. In this case author is not saved with the other post properties. Not to mention when extend type is used in an different file.

@dotansimha a configuration for typescript-resolvers that wraps the parent and the return type with Partial<...> would be much appreciate (and I would enable it by default).

subhash commented 5 years ago

@ajordanow I am not able to understand where you would set this option? Is it part of the config for any of the plugins?

set config.optionalType to "NestedPartial | null" (so Maybe will be generated as export type Maybe = T | NestedPartial | null;)

dotansimha commented 5 years ago

I'll summarize the point of this issue:

There was a bit mixup in the issue regarding mappers, so please read the comments and if you're still having issues with it - feel free to open a new issue :)

nathanforce commented 5 years ago

@dotansimha due to #1041, the solutions you mentioned above are incomplete. I’m currently solving this issue by using generics to manually fix the mappings that were missed. It’s not the end of the world but I thought I’d mention it here for future sake.

dotansimha commented 5 years ago

@nathanforce can you please open a new issue with an updated example?

dotansimha commented 5 years ago

Update: We are currently implementing this PR: https://github.com/dotansimha/graphql-code-generator/pull/1593 The goal is to automatically create a new type per each GraphQL type, based on it's fields and mapping. It should be much easier now. We are also adding the ability to wrap types with Partial<>.

mshwery commented 5 years ago

@dotansimha curious why the mappers only generate types for type resolvers but not all instances of a type? For instance we see this in mutation return types but the mutation resolvers don't use the mapped type? This is particularly a problem when using mappers to replace types like so:

overwrite: true
schema: 'src/**/*.graphql'
generates:
  src/@types/gql-schema.ts:
    plugins:
      - typescript
      - typescript-resolvers:
          contextType: ./graphql/types#ResolverContext
          mappers:
            Thing: ./graphql/types#ThingRoot

Here's a generated mutation... let's take a look at its return type UpdateThingPayload.

export type RootMutationResolvers<
  Context = ResolverContext,
  ParentType = RootMutation
> = {
  updateThing?: Resolver<
    UpdateThingPayload,
    ParentType,
    Context,
    RootMutationUpdateThingArgs
  >
}

export type UpdateThingPayload = {
  // Why isn't this `Maybe<ThingRoot>`?
  thing?: Maybe<Thing>
}

If we look at its return type UpdateThingPayload we can see that it expects a Thing type, not our mapped ThingRoot type. I think this is the missing piece for dynamically resolving a type. This mutation resolver's return type shouldn't expect Maybe<Thing> but rather Maybe<ThingRoot>, because the rest of type Thing is dynamically resolved. Using mappers (in my mind) is telling the generator "Hey, I got this. Here's the minimal representation of a type that is required because it will not be dynamically resolved. All other omitted properties will be dynamically resolved."

I would expect UpdateThingPayload to look like this since we defined a custom mapping for type Thing:

export type UpdateThingPayload = {
  thing?: Maybe<ThingRoot>
}
dotansimha commented 5 years ago

Hi @mshwery , I think that the recent changes we introduced related to the resolvers package should cover this issue, because mappers will now apply to other types where it's referenced. https://github.com/dotansimha/graphql-code-generator/pull/1593 If that's not covering your use-case, can you please open a new issue?

edorivai commented 5 years ago

Leaving this here for future googlers.

If you want to have all your types be wrapped in Partial<T> by default, you have to specify a defaultMapper (docs):

generates:
  ./src/generated-models.ts:
    plugins:
      - typescript
      - typescript-resolvers
    config:
      defaultMapper: Partial<{T}>
andrewgreenh commented 5 years ago

I think I found a working solution for our project. Afaik this only works with Typescript 3.1 and beyond, since mapped types now also work for arrays:

type MaybeInPromise<ValueType> = ValueType | Promise<ValueType>;

type MaybeAsFuncResultOrPromise<ValueType> =
 | ValueType
 | MaybeInPromise<ValueType>
 | ((...args: any[]) => MaybeInPromise<ValueType>);

type ResolvableArray<ArrayType> = {
  [Key in keyof ArrayType]: Resolvable<ArrayType[Key]>
};
type ResolvableObject<ObjectType> = {
  [Key in keyof ObjectType]: MaybeAsFuncResultOrPromise<Resolvable<ObjectType[Key]>>
};

export type Resolvable<ValueType> = ValueType extends Array<any>
  ? ResolvableArray<ValueType>
  : ValueType extends {}
  ? ResolvableObject<ValueType>
  : MaybeInPromise<ValueType>;

If you want to make the fields optional in objects, just add the questionmark to ResolvableObject. This recursivly traverses your GraphQLType and transforms object fields into Unions of functions, async functions, promises, or sync values.

smeijer commented 5 years ago

Having read each individual post in this issue, I still don't understand how we can deal with the Mappers in a safe way for more complex types.

I did map objects to parent types. But sometimes, the parent type is missing some computed fields. The issues that I find hard to fix, are about advanced types. "Connections" in my case;

Having the following schema:

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

type PageInfo {
    endCursor: String
    hasNextPage: Boolean!
    startCursor: String
    hasPreviousPage: Boolean!
}

type User @entity {
    id: ID! @id
    username: String! @column
    email: String! @column
    dateOfBirth: String! @column
    age: Int!
}

type UserConnection {
    totalCount: Int!
    pageInfo: PageInfo!
    nodes: [User!]!
}

Generates the types:


export type User = {
  __typename?: "User";
  id: Scalars["ID"];
  username: Scalars["String"];
  email: Scalars["String"];
  dateOfBirth: Scalars["String"];
  age: Scalars["Int"];
};

export type UserConnection = {
  __typename?: "UserConnection";
  totalCount: Scalars["Int"];
  pageInfo: PageInfo;
  nodes: Array<User>;
};

import { ObjectID } from "mongodb";
export type UserDbObject = {
  _id: ObjectID;
  username: string;
  email: string;
  dateOfBirth: string;
};

On these base types, I would like to implement a resolver like:

export interface Connection<T extends Node> {
  totalCount: number;
  pageInfo: {
    endCursor?: string | null;
    hasNextPage: boolean;
    startCursor?: string | null;
    hasPreviousPage: boolean;
  };

  nodes: T[];
}

const resolvers = {
  User: {
    id: user => user._id,
    age: (user: UserDbObject): number => {
      const diff = Date.now() - new Date(user.dateOfBirth).getTime();
      return new Date(diff).getUTCFullYear() - 1970;
    },
  },

  Query: {
    users: (parent, args, { db }): UserConnection => {
      return {
        totalCount: 1,
        pageInfo: {
          hasNextPage: false,
          hasPreviousPage: false,
        },
        nodes: [{ _id: 'a', username: 'foo', dateOfBirth: '2000-01-01' }] as UserDbObject[],
      } as Connection<UserDbObject>;
    },
  },
};

But that doesn't work. Because I'm getting an error that UserDbObject doesn't have the required properties id and age. Which is true, as they should be resolved trough the sub resolvers. But I don't understand how to get this working.

Type 'Connection<UserDbObject>' is not assignable to type 'UserConnection'.
  Types of property 'nodes' are incompatible.
    Type 'UserDbObject[]' is not assignable to type 'User[]'.
      Type 'UserDbObject' is missing the following properties from type 'User': id, age

Small repo here: https://codesandbox.io/s/graphql-codegen-connections-ys8ps

User is indeed missing these properties at this stage. But I'm adding them in the User resolvers. UserDbObject indeed doesn't have this properties, as they don't exist in the database.

How are you managing this? What am I missing?

smeijer commented 5 years ago

On similar note (and sorry for an offtop but you seem knowledgable in this area, and I don't want to create an issue that's more of a question) - is there a reason why enums are treated as strings in DbObjects? this way I get type mismatches.

@lgandecki , did you ever find a solution?

smeijer commented 5 years ago

Figured out that I could fix this issue by mapping the UserConnection type as well. To some custom interfaces that is. Not generated ones.

mappers:
  UserConnection: ../types/connection#Connection<UserDbObject>

Although, If I understand #1909 correctly, that shouldn't be required.

dotansimha commented 5 years ago

@smeijer mappers are not required, but if you are using mongodb plugin, you'll need to map it, because the codegen can't know this at the moment. It's not ideal, but feel free to open a new issue for that :)

vjsingh commented 5 years ago

@dotansimha Wanted to start using this library instead of graphqlgen, but couldn't find anything about optional types in the docs. I'm really confused here.

The comment from @dobesv on 1/31 sums things up well, as indicated by its 15 likes. The whole point of graphql is that the client requests which fields it wants to be returned, and thus the return type cannot force all the fields to be required. I do this all over my app, returning only part of an object.

Unless we are all missing something, what is the intended solution here? I see the @edorivai comment on 6/6. If that's the intended way should it be put in the docs? Or is there some better pattern here that we all are missing?

dotansimha commented 5 years ago

@vjsingh can you please explain? I'm not sure how it's related to this issue. What optional types do you refer to? If you think it's something we can improve, can you please open a new issue for that?

vjsingh commented 5 years ago

@vjsingh can you please explain? I'm not sure how it's related to this issue. What optional types do you refer to? If you think it's something we can improve, can you please open a new issue for that?

@dotansimha I don’t see a recommended solution for the original issue as described by @dobesv.

In Graphql, I can define a User object with a required email field. A client is however free to request a user without the email field. The field is required for objects in the database, but not that every client request has to include it.

As others have pointed out, this is one of the fundamental abilities of Graphql. However, it seems that this library returns all required types as required, which makes it not useful on the client end.

@edorivai recommended a mapping solution which looks like it may work, but I’m just curious if that’s the intended solution here.