MichalLytek / type-graphql

Create GraphQL schema and resolvers with TypeScript, using classes and decorators!
https://typegraphql.com
MIT License
8.03k stars 676 forks source link

mixins don't work with overridden properties and ArgsType #1644

Closed shawnjones253 closed 7 months ago

shawnjones253 commented 7 months ago

Describe the Bug When extending an @ArgsType() using mixins, if you override a field in an attempt to change the @Field type (for example, in order to use a custom scalar, or to change nullability), the schema does not correctly represent the overridden type, but rather the base type.

However, if you extend a base @ArgsType() class and define the override inline, the overridden type is correctly reflected in the schema.

To Reproduce

@ArgsType()
export class UserFindManyArgs {
  @Field(_type => Int, { nullable: true })
  take?: number | undefined;
}

export function nonNullableTake<TClassType extends ClassType>(BaseClass: TClassType) {
  @ArgsType()
  class withNonNullableTake extends BaseClass {
    @Max(1)
    @Field(_type => Int, { nullable: false })
    take!: number;
  }

  return withNonNullableTake;
}

@ArgsType()
export class UserFindManyNonNullableTakeArgs extends nonNullableTake(UserFindManyArgs) {}

@Resolver()
export class UserResolver {
  private readonly usersData: User[] = [];

  @Query(_returns => [User])
  async users(@Args() args: UserFindManyNonNullableTakeArgs): Promise<User[]> {
    return this.usersData.slice(0, args.take);
  }
}

schema output:

type Query {
  users(take: Int): [User!]!
}

Expected Behavior The schema output shows that take is required.

Environment (please complete the following information):

Additional Context

changing the override to NOT use mixins causes the schema to correctly reflect the non-nullability:

@ArgsType()
export class UserFindManyPaginatedArgs extends UserFindManyArgs {
  @Max(1)
  @Min(0)
  @IsDefined()
  @Field(_type => Int)
  override take!: number;
}

schema output:

type Query {
  users(take: Int!): [User!]!
}
MichalLytek commented 7 months ago

Thanks for the report.

Looks like this is not related to mixins but deeply nested inheritance at all. It's because of using while loop for super class traversal in order, instead of doing it from top to bottom.

The fix should be fairly easy to implement, working on it rn 😉

MichalLytek commented 7 months ago

Fixed via 9b2d2bb 🔒

shawnjones253 commented 7 months ago

wow! that was a much faster fix than I expected, thanks!

any idea when you might be doing the next release @MichalLytek ? i'm working on overriding several hundred generated types from typegraphql-prisma and this would sure make the work a lot easier/cleaner ;)