dangcuuson / graphql-schema-typescript

Generate TypeScript from GraphQL's schema type definitions
190 stars 36 forks source link

[feat request] "Non-Optional Resolver Functions" option #7

Closed tomasAlabes closed 6 years ago

tomasAlabes commented 6 years ago

Hi, first thanks for your work! It's really useful.

I was developing a class implementing the QueryResolvers interface, but one of the things I wanted was to make sure that all resolvers are implemented, to make sure that if some mutation/query is added, the corresponding resolver is there. Does it makes sense?

class MyResolvers implements QueryResolvers.Resolvers {
//no error because the resolvers are optional
}
export namespace QueryResolvers {
  export interface Resolvers {
    getSomething?: getSomethingResolver;
  }
}

Hopefully it makes sense and it's easy to implement :) Thanks again!

dangcuuson commented 6 years ago

Hi tomasAlabes,

I'm glad that you find the lib useful to you :)

If you are using Typescript >= 2.8, you can use the utility Required<T> type to remove the optional modifier (it's kind of the opposite of Partial<T>).

You can see the feature introduced here

I have tested and it seems that classes can implement types as well, as long as it's not a union type or primitive types.

So if you want to enforce your class to implements all field resolvers, you can do this:

class MyResolvers implements Required<QueryResolvers.Resolvers> {
}

However, if you cannot use this feature in your project, I can implement an option in this case, it should be simple.

tomasAlabes commented 6 years ago

Hi @dangcuuson, thanks for your response. I didn't know of that feature! I'm new to TS so I'm learning as I go. I'll use it for sure. I personally think is better to have a way of enforcing it from the types so that the developer can't skip or forget to implement a resolver. It's safer I guess, if you want to skip something you can Partial<> it, right?

igorrmotta commented 6 years ago

My use case is better to have them as optional. I have so many resolvers that I find better splitting them in different files.

Perhaps it could be a argument?

‘—requireResolverTypes’ On Sat, 30 Jun 2018 at 12:00 pm, Tomas Alabes notifications@github.com wrote:

Hi @dangcuuson https://github.com/dangcuuson, thanks for your response. I didn't know of that feature! I'm new to TS so I'm learning as I go. I'll use it for sure. I personally think is better to have a way of enforcing it from the types so that the developer can't skip or forget to implement a resolver. It's safer I guess, if you want to skip something you can Partial<> it, right?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dangcuuson/graphql-schema-typescript/issues/7#issuecomment-401509941, or mute the thread https://github.com/notifications/unsubscribe-auth/AG5CfIXqP1wWdGnU5NfQwJrNj53wOsxGks5uBtupgaJpZM4U8J2y .

tomasAlabes commented 6 years ago

@igorrmotta yeah! I didn't mean to say that it should always be mandatory, I think the argument is the most flexible option.

dangcuuson commented 6 years ago

Please check latest version (1.2.2) for the new option: requiredResolverTypes

Happy coding :smile:

tomasAlabes commented 6 years ago

Thank you!