Type-graphql: Mark a @Field property as resolved by a @FieldResolver

Created on 11 Oct 2019  路  11Comments  路  Source: MichalLytek/type-graphql

Is your feature request related to a problem? Please describe.
We have @ObjectType() classes which are reused by the front end, however many properties are resolved by @Resolver() classes using @ResolveProperty() (we're using NestJS).

This means we have to ensure that our property resolver signature matches what's declared in the object type, and if they are different, which one gets used in the schema generation?

Describe the solution you'd like
An option on the @Field() decorator like, { expectFieldResolver: true } which tells type-graphql that this field should have a corresponding @FieldResolver() (or @ResolverProperty() in NestJS applications) with a matching signature.

If the property resolver doesn't exist on any @Resolver() class, or the signature is different, a warning/error should be emitted from the schema generation stage.

Describe alternatives you've considered
I've tried using the ResolverInterface interface but our @Root() type is an instance of our TypeORM @Entity() class and not an instance of our @ObjectType() class. We have tried ... implements ResolverInterface<MyEntity> so that @Root() is the correct type, but sometimes the field resolver returns an @ObjectType() instance (or a scalar), and other times it returns a related @Entity() class, which goes against the ResolverInterface definition.

Another solution I've considered is to just omit the @Field() definition on the @ObjectType() class but then these types are missing properties when they're imported by the UI.

Question Solved

Most helpful comment

I won't implement new runtime check option if the new syntax of pointing type and field solves that problem too and also allow for custom names of method:

@ObjectType()
class Author {
  @Field(type => [Post])
  posts: Post[]
}

@Resolver()
class Post Resolver
  @FieldResolver(of => Author, author => author.posts) 
  getAuthorPosts(@Root() author: Author, @Args() args: PostsArgs) {
    // ...
  }
}

All 11 comments

and if they are different, which one gets used in the schema generation?

The @FieldResolver overwrites the field type if type annotation provided.

I've tried using the ResolverInterface interface but our @Root() type is an instance of our TypeORM @Entity() class and not an instance of our @ObjectType() class.

So create your own interface that takes two parameters - object type and model type.

The @FieldResolver overwrites the field type if type annotation provided.

This just did the opposite for me. I viewed the newly generated schema.graphql to check it had done what I was expecting and it had generated the type from the @ObjectType()/@Field() declaration, not the @FieldResolver(). I am using @ResolveProperty() from NestJS though, could this be a thing?

So create your own interface that takes two parameters - object type and model type.

Isn't this just moving the responsibility but not solving the problem? You still have to maintain a new type?

What problem solving? Moving from compile-time to runtime with { expectFieldResolver: true }?

You still have to maintain a new type?

I can't maintain every use case of TypeGraphQL - maybe should I also add a third parameter with a list of allowed keys that you can oveerride the type?

With #193 you would be able to point the type and field that you want to implement resolver for.

I suppose I could have worded the OP better.

The suggestion of a new option expectFieldResolver was to mitigate the ambiguous results of schema generation because in my case, the schema was generated from the @Field() definition, not the field resolver which you mentioned should be the case. I'm also 99% certain I have experienced the opposite too, but can't find evidence right now.

I am using Nest's @ResolveProperty() and not @FieldResolver() though, could this be something to cause different behaviour?

the schema was generated from the @Field() definition

Yes, sorry for confusing - checked in the code and it uses types info only for creating new fields, not for implementing resolvers.

I can't maintain every use case of TypeGraphQL - maybe should I also add a third parameter with a list of allowed keys that you can oveerride the type?

Apologies in either case, but I don't know if this is serious or sarcasm.

I don't think changing the ResolverInterface is the answer. I do agree that to make it type safe, it should be the developer's responsibility to create and maintain that mapping interface/type and not the responsibility of this library.

Yes, sorry for confusing - checked in the code and it uses types info only for creating new fields, not for implementing resolvers.

In that case, you could do away with my suggestion of a new option and just warn if the type from @FieldResolver(() => NotSomeType) is different to @Field(() => SomeType)?

I've just noticed, we're actually discussing two separate problems which can't be solved with one solution.

  1. Type safety for the return type between @ObjectType() properties and @FieldResolver() methods, has to be done through the ResolverInterface interface, or some other custom interface.
  2. Safety that one @Field(() => SomeType) isn't conflicted by another @FieldResolver(() => NotSomeType) can't be mitigated by TypeScript. It's a type being used as a value within a decorator. For example you could quite easily solve 1) and then do this:
class ... {
  @FieldResolver(() => NotSomeType)
  someType(@Root() root): SomeType { ... }
}

Whatever solution you do for 1) to make the above type safe, you've still made a mistake that your @Field() and @FieldResolver() don't match and TypeScript will never know.

I won't implement new runtime check option if the new syntax of pointing type and field solves that problem too and also allow for custom names of method:

@ObjectType()
class Author {
  @Field(type => [Post])
  posts: Post[]
}

@Resolver()
class Post Resolver
  @FieldResolver(of => Author, author => author.posts) 
  getAuthorPosts(@Root() author: Author, @Args() args: PostsArgs) {
    // ...
  }
}

Ahh, I read the issue you linked but at first glance didn't quite understand how it would solve the problem.

But yes, you're correct. You don't duplicate the field type with that new syntax so can never get it wrong 馃憤

Great! So closing this issue as it will be fixed in #193 馃敀

Was this page helpful?
0 / 5 - 0 ratings