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

Overriding properties in subclass for InputType does not work correctly. #1109

Closed
rdehuyss opened this issue Nov 7, 2021 · 4 comments
Closed
Assignees
Labels
Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community Solved ✔️ The issue has been solved
Milestone

Comments

@rdehuyss
Copy link

rdehuyss commented Nov 7, 2021

Describe the Bug
First of all, thanks for this great project!

I'm experiencing a bug when creating a subclass for an InputType where one overrides properties with subclasses (I know - a lot of inheritance 😄 ). The resulting schema is generated correctly but when calling the query, data is not available in the resolver.

To Reproduce
class QueryArgs.ts

@InputType("PaginationArgsInput")
export class PaginationArgs {
    @Field()
    skip: int = 0 as int;

    @Field()
    take: int = 25 as int;
}

export enum SortOrder {
  ASC = 'ASC',
  DESC = 'DESC',
}


@InputType("SortingArgsInput")
export class SortingArgs  {
    @Field({defaultValue: 'ASC', nullable: true})
    createdAt?: SortOrder;

    @Field({defaultValue: 'ASC', nullable: true})
    updatedAt?: SortOrder;
}

@InputType("FilterArgsInput")
export class FilterArgs {
    @Field({nullable: true})
    query?: string;
}

@InputType("QueryArgsInput", {isAbstract: true})
export class QueryArgs {
    @Field(() => FilterArgs, {nullable: true})
    filter?: FilterArgs;

    @Field(() => SortingArgs, {nullable: true})
    sorting?: SortingArgs;

    @Field(() => PaginationArgs, {nullable: true})
    pagination?: PaginationArgs = new PaginationArgs();
}

class ProductQueryArgs

@InputType()
export class ProductMasterDataFilterArgs extends FilterArgs {

    @Field({nullable: true})
    productName?: string;

}

@InputType()
export class ProductMasterDataSortingArgs extends SortingArgs {

    @Field({defaultValue: 'ASC', nullable: true})
    productName?: SortOrder;

}

@InputType()
// why does extend QueryArgs not work? Values that are given from GraphQL are not available
export class ProductMasterDataQueryArgs extends QueryArgs {

    @Field(type => ProductMasterDataFilterArgs, {nullable: true})
    filter?: ProductMasterDataFilterArgs;

    @Field(type => ProductMasterDataSortingArgs, {nullable: true})
    sorting?: ProductMasterDataSortingArgs;
}

In the resolver:

  @Query(() => [ProductMasterData])
  async getProductMasterDataByCompanyId(
      @Arg("companyId") companyId: string,
      @Arg("queryArgs", {nullable: true, defaultValue: new ProductMasterDataQueryArgs()}) pmdQueryArgs: ProductMasterDataQueryArgs
  ): Promise<ProductMasterData[]> {
      console.log(pmdQueryArgs);
      return [];
  }

Output of console.log:

ProductMasterDataQueryArgs {
  pagination: PaginationArgs { skip: 0, take: 4 },
  filter: FilterArgs {},
  sorting: SortingArgs { createdAt: 'ASC', updatedAt: 'ASC' }
}

The schema is generated correctly but when I pass data for sorting, it is not available in the pmdQueryArgs. If I remove the inheritance (extends QueryArgs) and add the pagination property in ProductMasterDataQueryArgs, all is well. The output of console.log is then:

ProductMasterDataQueryArgs {
  pagination: PaginationArgs { skip: 0, take: 4 },
  filter: ProductMasterDataFilterArgs { productName: 'prod' },
  sorting: ProductMasterDataSortingArgs {
    productName: 'ASC',
    createdAt: 'ASC',
    updatedAt: 'ASC'
  }
}
Sorting ProductMasterDataSortingArgs {
  productName: 'ASC',
  createdAt: 'ASC',
  updatedAt: 'ASC'
}

Expected Behavior
I expect the same logging in both cases.

Logs
See above

Environment (please complete the following information):

  • OS: Ubuntu 20.04
  • Node (e.g. 14.7.6)
  • Package version [e.g. 1.1.1] (please check if the bug still exist in newest release)
  • TypeScript version (e.g. 4.4.4)
@MichalLytek
Copy link
Owner

First of all, you should not do PaginationArgs = new PaginationArgs(); nor efaultValue: new ProductMasterDataQueryArgs() 😝

Secondly, instead of a weird inheritance chain, you should use generic types and pass the enhanced inputs as arguments instead of overwriting the properties. It makes things much more maintainable and understandable.

However, I've managed to reproduce your issue about provided data not showing up in the injected object. Will take a look at that and try to fix it 😉

@MichalLytek MichalLytek added Bug 🐛 Something isn't working Community 👨‍👧 Something initiated by a community labels Nov 7, 2021
@MichalLytek MichalLytek self-assigned this Nov 7, 2021
@MichalLytek MichalLytek added this to the 1.x versions milestone Nov 7, 2021
@rdehuyss
Copy link
Author

rdehuyss commented Nov 7, 2021

Hi - wauw, you have the fastest response time ever! Thanks!

First of all, you should not do PaginationArgs = new PaginationArgs(); nor efaultValue: new ProductMasterDataQueryArgs() stuck_out_tongue_closed_eyes

As I'm new to the whole NodeJS/Graphql world (long time Java/.Net/python) developer here, can you tell me why I should not provide default arguments? Or is there another way?

Secondly, instead of a weird inheritance chain, you should use generic types and pass the enhanced inputs as arguments instead of overwriting the properties. It makes things much more maintainable and understandable.

I agree, still in the walking skeleton phase and did not think of it yet.

However, I've managed to reproduce your issue about provided data not showing up in the injected object. Will take a look at that and try to fix it.

You are a hero!

@MichalLytek
Copy link
Owner

why I should not provide default arguments? Or is there another way?

It makes no sense in your case. The default for PaginationArgs are already defined and will be initialized in runtime by graphql execution engine. You don't need to do that by yourself.

@MichalLytek
Copy link
Owner

Fixed in 3ac1a27, will be released in RC2 🔒

@MichalLytek MichalLytek added the Solved ✔️ The issue has been solved label Nov 7, 2021
@MichalLytek MichalLytek modified the milestones: 1.x versions, 1.2 Nov 25, 2021
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 Solved ✔️ The issue has been solved
Projects
None yet
Development

No branches or pull requests

2 participants