name property in @Field does not work when used with @ArgsType

[borremosch]

Describe the bug
I am trying to combine the arguments to one of my resolver's functions by using a class that I annotate with @ArgsType(). Since the name I want to expose is different in one case from the name I want to use internally, For this I am using the name property for one of my fields, which has worked fine so far in my entities (annotated @ObjectType). However, whereas name does change the name that is exposed to the outside, it also changes the name of the property to which the incoming value is assigned.

To Reproduce
Using the following code:

import Organization from '../../entities/Organization';

@ArgsType()
export class DeleteOrganizationArgs {
  @Field({
    name: 'id'
  })
  slug!: string;
}

@Resolver(Organization)
export default class OrganizationResolver {
  @Mutation(returns => Organization)
  async deleteOrganization(@Args() args: DeleteOrganizationArgs) {
    console.log(args);

    // Actually delete organization
  }
}

Calling my resolver shows the log {"id": "my-id"}. This looks like a bug to me, because the type DeleteOrganizationArgs does not have a property id.

Expected behavior
I expect args to be equal to {"slug": "my-id"}

Enviorment (please complete the following information):

• OS: Ubuntu 18.04.2 LTS
• Node 10.15.1
• Package version 0.16.0
• TypeScript version 3.2.4

[MichalLytek]

The name option in decorator was introduced only for making resolvers inheritance possible:
https://19majkel94.github.io/type-graphql/docs/interfaces-and-inheritance.html#resolvers-inheritance

But using name in @Field of InputType or ArgsType is an undocumented feature.
It can't map the property names from schema one to the type one as it would require a lot of overhead for each call of the query/field. There's already a big overhead (#254), I can't add more things that are so rarely used.

[borremosch]

Ok fair enough, if it's not meant for that then it probably shouldn't be done using name. However, it would be nice to be able to expose properties to the outside under a different name using @Args. With @Arg (singular) it's actually possible:

async deleteOrganization(@Arg('id') slug: string) {
  ...
}

Now I am not able to switch from @Arg to @Args, although I like that last pattern a lot more than cluttering up my resolvers with annotations like I'm doing now. Is there a specific reason that mapping the names performs ok with @Arg but wouldn't with @Args?

[MichalLytek]

Is there a specific reason that mapping the names performs ok with @Arg but wouldn't with @Args?

Yes - you can declare your function as:

declare function fooBar(foo: string, bar: number);

And then call it with your variables of different names:

declare const foo: number;
declare const bar: string;

fooBar(bar, foo);

With objects you have to map the original graphql-js args object to the one you declare in the resolver method.

Why you can't just stick with 1:1 mapping between the names? Different names just make things harder to debug (I send id in query, I receive slug in error stacktrace).

[borremosch]

Of course I can, but this would contradict a design decision that's been made in the application I'm working on. In the datebase and code, we're using integer IDs, but these are never shown to the outside world, for example because of security reasons. Another, textual ID (slug) is calculated and exposed to the outside, but to the outside this slug is called id, because they are being used to uniquely identify objects from the outside.

I will probably either stick to @Arg or solve it using 'native mapping' like this:

async deleteOrganization(@Args() { id: slug }: DeleteOrganizationArgs) {
  ...
}

but this is more error prone as a programmer might forget to put this mapping in a resolver function one day and another programmer might confuse an id for a slug.

You could say that this issue is more of a nice-to-have than a need-to-have, and I would agree. But I do think there are more use cases in which one might want to have different namings for variables in one's code than in an API that is exposed to the outside world.

[MichalLytek]

I agree that it may be a useful feature so I will try to implement it with a performance impact in mind (detect if there's any mapping and if not, just skip it).

But it's a more feature than a bug and it has lower prority that the missing GraphQL features, so it also need the new pipeline #183 to have access to built metadata.