Custom Param Decorator breaks other @Arg if not last argument

[russell-dot-js]

Describe the Bug We are using custom param decorators to inject args when derived by user, and allow overriding those args in other circumstances. For example, a normal user can only access their own account, while an admin user can access any account - so we use a nullable accountId argument and our decorator auto-fills it with the user's accountId if that user is a non-admin.

Here is some sample code:

export function AccountIdArg(
  argName: string = 'accountId',
): ParameterDecorator {
  // register a accountId param
  // override with current Account if not a staff user or not supplied
  const arg = Arg(argName, () => String, { nullable: true });
  const interceptor = createParamDecorator<OurCustomContextType>(({ args, context }) => {
    if (!context.currentUser) {
      throw new Error('User not logged in');
    }

    let accountId = args[argName];

    if (context.currentUser.userType === UserType.Consumer) {
      throw new Error('Not authorized');
    }

    if (context.currentUser.userType === UserType.Account) {
      accountId = context.currentUser.accountId;
    }

    if (!accountId) {
      throw new Error(`Missing required argument "${argName}"`);
    }

    return accountId;
  });
  return (target, key, index) => {
    interceptor(target, key, index);
    arg(target, key, index);
  };
}

The issue is, that when this custom param is the only argument, it works fine. If it's the last argument, it also works fine. But if it's somewhere in the middle (e.g. @Ctx() context, @AccountIdArg accountId, @Arg('input') input: SomeInputType), the third argument (input) will be undefined. Here are some screenshots of a debugger in different states:

First Arg

Notice how "input" is undefined

Second Arg

Now "input" is working as expected - the only change is switching the order of the arguments.

To Reproduce Use the decorator code above and set "currentUser.accountId" to some string in your context, then create a query or mutation with the new param decorator.

Expected Behavior The param decorator should work regardless of which order the argumetns are in.

Logs See screenshot, debugger is clearest example

Environment (please complete the following information):

• OS: macOS & Amazon linux
• Node: 14.x
• Package version: 1.1.1
• TypeScript version: 4.7.4

[MichalLytek]

I'm not sure if it's about createParamDecorator or because of your custom code wrapping decorators, like:

return (target, key, index) => {
	interceptor(target, key, index);
    arg(target, key, index);
};

Can you try to reproduce the issue with simple const AccountIdArg = createParamDecorator<OurCustomContextType>(({ args, context }) => {?

[russell-dot-js]

@MichalLytek how could I create a param decorator that creates an argument without doing it the way I am?

[MichalLytek]

Your issue is about arg after custom param decorator. I'm not responsible for supporting your patterns like composing decorators and calling them manually.

[russell-dot-js]

Your issue is about arg after custom param decorator. I'm not responsible for supporting your patterns like composing decorators and calling them manually.

? It's not my pattern - I am trying to use type-graphql the way it's intended. Namely, you support custom parameter decorators and annotation-based graphql Arguments.

I want to create a custom decorator (as you support) that creates an @Arg - are you saying you'd prefer it if I reached into the internals of type-graphql and recreate the way @Arg works, rather than the simple way outlined in the example?

[russell-dot-js]

It's worth calling out that when debugging, the provided values are correct when debugging the interceptor for both arg1 and arg2 - the values aren't incorrect until the function is actually called:

It's also worth noting that if the two arguments are the same type, they are both getting the value from the custom param decorator - when they are different types, the second argument comes in as undefined:

(sorry about the redlines when running in jest, not a priority to fix)

[russell-dot-js]

Also - at runtime the values in getMetadataStorage() are correct - yet the wrong value is being passed to the resolver:

[russell-dot-js]

And here is the root of the issue - if I step up into createHandlerResolver, you can see that it is creating three "resolvedParams". My custom parameter is taking index 0 and 1, even though it is properly configured to only use index 0 in the metadata storage:

I am guessing this is because I am registering an interceptor for my arg, but simply putting the configuration into metadata storage is adding a second one (I stopped calling Arg and went directly to metadata storage to test, so it is not the arg function):

[russell-dot-js]

After digging in some more, I can see how this isn't currently doable with type-graphql's implementation. The argument will always have two params in ParamMetadata - one with type "arg", one with type "custom". "getParams" maps the param metadata into an array of params, so the two params will always be in there, and there is no way to tell type-graphql which to prioritize.

I can think of a few ways to change this behavior and am open to submitting a PR but should discuss design first:

1. (Probably makes the most sense): add backwards-compatible options to createParamDecorator, "arg" being first one. The parameter will be registered as a custom param, but getParams will run "custom" params with an "arg" configuration through validateArg and convertArgToInstance prior to calling the resolver. The resolver will then be called (with the argument populated in the ArgsDictionary)

Example:

createParamDecorator({
  arg: {
    name: 'Foo',
    returnType: () => String,
    { nullable: true },
  }
}, (({ args, context }) => {
  if(context.bar) {  
    return context.bar;
  }
  return args.Foo;
});

2. Add a resolver configuration to @Arg that will run after type-graphql's default logic. Example:

@Arg('Foo', () => String, {
  nullable: true,
  resolver: (({ args, context }) => {
    if(context.bar) {  
      return context.bar;
    }
    return args.Foo;
  }) 
})

3. I can probably rewrite as a method decorator, e.g. @AccountIdArg('accountId') but this doesn't seem as clean as a parameter decorator. The docs say introduce custom decorators as: "Custom decorators are a great way to reduce the boilerplate and reuse some common logic between different resolvers. TypeGraphQL supports two kinds of custom decorators - method and parameter.". Creating reusable, shared logic, but in a confusing way, is a loss, and this seems like a pretty straightforward use case that plenty of other people would likely want, if it were easy to use. The fact that it worked, and I used it for a year, and didn't notice the bug until an additional argument was kind of crazy - I can imagine that others will possibly run into a similar issue.

[russell-dot-js]

@MichalLytek here is my proposed solution https://github.com/MichalLytek/type-graphql/pull/1330

[DrakeAnglin]

I am also having the same issue. We are trying to modify the incoming object to add the user created and modified at data to one of the args.

[russell-dot-js]

I am also having the same issue. We are trying to modify the incoming object to add the user created and modified at data to one of the args.

@DrakeAnglin looking at the history of issues and PR's on this repo, you might be better off forking the repo than waiting for a merge. Feel free to use our fork https://github.com/MichalLytek/type-graphql/pull/1330