Graphql: @Args() in Fields not supported after migrating from type-graphql

[kylecannon]

I'm submitting a...

[ x ] Regression 
[ ] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

In type-graphql it was possible to do this: https://graphql.org/learn/schema/#arguments

@ArgsType()
export class FilterJobResultsArgs {
  @Field(type => JobContentTypeEnum, { nullable: true })
  type?: JobContentTypeEnum | null;

  @Field(type => JobContentStatusEnum, { nullable: true })
  status?: JobContentStatusEnum | null;
}

@ObjectType()
export class MyJob {
  @Field(type => Number)
  jobResultsCount(@Args() filters: FilterJobResultsArgs) {
    return this.jobResults(filters).length;
  }
}

This would output to the GraphQL schema

enum JobContentStatus {
  Completed
  Errored
}


enum JobContentType {
  image
  video
}

type MyJob {
     jobResultsCount(type: JobContentType, status: JobContentStatus): Float!
}

But instead, the output is:

enum JobContentStatus {
  Completed
  Errored
}

enum JobContentType {
  image
  video
}

type MyJob {
     jobResultsCount: Float!
}

This makes it impossible to upgrade past Nest 7.x at this time.

Environment

Nest version: 8.0.1


For Tooling issues:
- Node version: 12
- Platform: Mac

Others:

[elie222]

Seems to work fine for us:

  import { Args, Mutation, Query, Resolver, ResolveProperty, Parent } from '@nestjs/graphql'

  @Query(() => Contract)
  async contract(@Args('id') id: string) {
    return this.contractService.findOne(id)
  }

=>

contract(id: String!): Contract!

[doug-martin]

@elie222 Args work fine for @Query methods but if you have a field that requires args or you have a @ResolveField decorator on the resolver, arguments are not picked up.

It looks like https://github.com/nestjs/graphql/blob/master/lib/schema-builder/factories/object-type-definition.factory.ts#L87 does not even consider args.

[kylecannon]

@doug-martin is correct.

[phoenix741]

I have the same problem, on my application. Args are ignored on ResolveField.

[kamilmysliwiec]

@doug-martin @phoenix741 @Args()+ @ResolveField should work now. Please, update to 7.0.8 version.

However, we don't plan to support this:

@Field(type => Number)
jobResultsCount(@Args() filters: FilterJobResultsArgs) {
    return this.jobResults(filters).length;
}

So basically: @Args() in combination with the @Field() decorator.

All resolvers which take any arguments should be defined in resolver classes (not the object type itself). To migrate, simply move the resolver under a resolver class:

@Resolver(of => MyJob)
export class MyJobResolver {
  @ResolveField(type => Number)
  jobResultsCount(@Args() filters: FilterJobResultsArgs) {
    return this.jobResults(filters).length;
  }
}

[kylecannon]

This is is impossible to implement when you use arguments in a paginated result. https://graphql.org/learn/pagination/

This upgrade was supposed to be backwards compatible and this literally puts me in a position where I am stuck on an older version. I cannot move this to the resolver class cause it makes it impossible to do the computations I need to do.

Please reconsider because you're not supporting the graphql specification by removing this critical feature.

[kamilmysliwiec]

Please reconsider because you're not supporting the graphql specification by removing this critical feature.

Can you share an example of what's impossible (to implement) now?

[kylecannon]

Sure, this is not possible: https://graphql.org/learn/pagination/#complete-connection-model due to the arguments happening in a grand grand child DTO which is the core power of the GraphQL specification -- which allows you to have arguments anywhere.

A lot of this code was derived from: https://github.com/nestjs/nest/issues/2420

Here's an excerpt from my code:

@ArgsType()
export class FilterJobResultsArgs {
  @Field(type => JobContentTypeEnum, { nullable: true })
  type?: JobContentTypeEnum | null;

  @Field(type => JobContentStatusEnum, { nullable: true })
  status?: JobContentStatusEnum | null;
}

@ObjectType()
export class MyJob {
  @Field(type => Number)
  jobResultsCount(@Args() filters: FilterJobResultsArgs) {
    return this.jobResults(filters).length;
  }
}

@ObjectType({ isAbstract: true })
export abstract class PaginationNode<Node> {
  @Field(type => String)
  cursor: string;

  abstract node: Node;
}

@ObjectType()
export class MyJobNode extends PaginationNode<MyJob> {
  @Field(type => MyJob)
  node: MyJob;
}

@ObjectType({ isAbstract: true })
export abstract class PaginationPage<Node> {
  @Field(type => Number)
  totalCount: number;
  @Field(type => PaginationPageInfo)
  pageInfo: PaginationPageInfo;

  abstract edges: PaginationNode<Node>[];
}

@ObjectType()
export class MyJobsPage extends PaginationPage<MyJob> {
  @Field(type => [MyJobNode])
  edges: MyJobNode[];
}

@InputType()
export class MyJobQueryOptions implements QueryOptions {
  @Field(() => Int, { nullable: true })
  first: number | null;

  @Field(() => Int, { nullable: true })
  last: number | null;

  @Field(type => MongoObjectIdCursor, { nullable: true })
  before: ObjectId | null;

  @Field(type => MongoObjectIdCursor, { nullable: true })
  after: ObjectId | null;
}

@ArgsType()
export class MyJobsArgs {
  @Field(type => MyJobQueryOptions)
  queryOptions: MyJobQueryOptions;
}


@Query(() => MyJobsPage)
async getMyJobs(@Args() args: MyJobsArgs) {
  if (args.queryOptions.first === 0 || args.queryOptions.last === 0) {
    return fromPaginationHelpers.EMPTY_PAGE;
  }
  return this.repository.paginate(args);
}

This is an excerpt of my query:

query getJobsTableRows {
    jobs: getMyJobs(queryOptions: {}) {
        totalCount
        edges {
            cursor
            node {
                ...MyJobsTable
            }
        }
    }
}

fragment MyJobsTable on MyJob {
    jobResultErrorCount: jobResultsCount(status: Errored)
    jobResultCompleteCount: jobResultsCount(status: Completed)
}

[kamilmysliwiec]

And why is it impossible? You can pass arguments to fields (this is completely fine), you just have to define field resolvers inside the resolver class, not the object type itself

[doug-martin]

@kylecannon you can move your example to the resolver, I implement the connection specification in the resolver in nestjs-query, and so far everything works as expected in v7.0.9

I think if you change your resolvers a little bit you can make this work.

You can add the resolver is decorated with @Resolver(() => MyJob) the important part being the () => MyJob this tell @nestjs/graphql that the resolver is for DTOs of that type.

@Resolver(() => MyJob)
class MyJobResolver {
  @Query(() => MyJobsPage)
  async getMyJobs(@Args() args: MyJobsArgs) {
    if (args.queryOptions.first === 0 || args.queryOptions.last === 0) {
      return fromPaginationHelpers.EMPTY_PAGE;
    }
    return this.repository.paginate(args);
  }

  @ResolveField(type => Number)
  jobResultsCount(@Parent() myJob: MyJob, @Args() filters: FilterJobResultsArgs) {
    // not sure where  jobResults comes from its not in your example
    return myJob.jobResults(filters).length;
  }
}

IMO This cleans things up a little bit keeping business logic out of your DTO so they are plain objects, and if in the future you need to use a repo to get count because you need to count/query thousands of records you wont need to figure out how to inject the repository or service into your DTO.

Hope this helps!

[kylecannon]

@doug-martin I will try this and see if I can get it to work. The only bummer is I have to restructure a lot of code to get this to work. I was hoping this would be a drop-in as promised. I am not even sure if the abstract classes work either. I will keep you guys posted.

[kylecannon]

Also, this kind've makes it difficult when the DTO is shared across two resolvers as well.. meaning I have to copy the same code in two places to get it to work. A good example would be having a resolver that returns just a single MyJob DTO that needs the jobResultsCount as well. Is there a way to share it without having to extend the resolver class itself?

[phoenix741]

It's ok for me now.
Thanks.

[j]

@kamilmysliwiec Yeah, @ResolveField is what I had to add which just calls the same method on the model. Getters and setters (as in getFoo()/setFoo()) is a known coding style for models. I get the whole "no business logic" in models, but sometimes shared business logic is needed. I happen to use these methods I want as GraphQL fields in various areas.

It "cleans" the models, but then brings a few "WTF"s to how a field is there when it really belongs on a model in our code-base. I've always used @FieldResolvers in type-graphql for relations and stuff that is not part of the model. But I feel that it's perfectly fine to have getX methods with arguments in a model, as long as it's something simple. My case is it's just filtering an array.

My Auth models have a hasRole(role: UserRole) method that I want to expose. You already support get foo(), which I use as well, which people put business logic into these types of methods as well.

Thankfully our stuff is fully tested so I can figure out exactly which fields I have to move into the resolvers for now, but for people coming from type-graphql without a test suite..... good luck!

[kylecannon]

@j I agree. This really does make following the code paths easier. I wish it would be reconsidered.

[kamilmysliwiec]

Actually, it was an "accident" (not planned side-effect) that field resolvers defined as methods within object types worked in the past. Any Nest feature, in fact, wasn't supported there, including guards, interceptors, filters, pipes (validation & transformation - invalid arguments were accepted), and custom decorators. Everything worked "well" unless you tried to use any framework feature. Nest cannot create the execution context outside of the DI IoC (for classes not defined as either providers or controllers), and therefore it's pretty much impossible for us to enable such a feature (unless we decide to force people to define object types as providers, but this would bring even more side-effects because providers can be, for example, request-scoped + they can also be injected into other providers which in case of models doesn't make any sense).

Hence, any logic that requires dealing with arguments should be located inside the resolver class. If you want to isolate some domain/business logic and put it into the object types (which is completely fine), simply define a normal public method and call this method from within the @ResolveField after extracting/processing arguments and that's it. The fact that you must use @ResolveField() instead of defining a method decorated with @Field() doesn't mean that you have to put the entire logic in there. You can still isolate domain-specific logic and call it from the resolver class. The only limitation here is that you can't use @Args decorator in the ObjectType class, but the rest of your code can remain in the object class.