Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
shawnjones253 opened this issue Mar 6, 2024 · 3 comments
Closed

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

shawnjones253 opened this issue Mar 6, 2024 · 3 comments
Assignees
Labels
Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community
Milestone

Comments

@shawnjones253
Copy link

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):

  • OS: Mac 14.2
  • Node 20.4.0
  • Package version: 2.0.0-beta.3
  • TypeScript version: Version 5.3.3

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 MichalLytek added Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community labels Mar 6, 2024
@MichalLytek MichalLytek self-assigned this Mar 6, 2024
@MichalLytek MichalLytek added this to the 2.0 release milestone Mar 6, 2024
@MichalLytek
Copy link
Owner

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
Copy link
Owner

MichalLytek commented Mar 6, 2024

Fixed via 9b2d2bb 🔒

@shawnjones253
Copy link
Author

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 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community
Projects
Status: Backlog
Development

No branches or pull requests

2 participants