Typed arguments in resolvers

[terion-name]

It's kinda feature request. I've posted a question how to do this using existing tools (btw I'l be very grateful if someone can help with this), but I think this really should be in a spec/ref implementation.

Resolver accepts arguments as plain javascript object, already combined from inline arguments and variables. But GraphQL is strongly typed and this benefit doesn't work here. I get arguments, but I don't know types of these arguments. Getting types of args in resolver is useful because it gives more possibilities to process input. To validate or transform it based on types. This is especially useful in scenarios when you have deeply nested input with relations, multiple named input types, repeated types (lists of objects), etc.

And I see two variants of improving this:

1. Change plain object in resolver args to special object with types mappings
2. Add a function to graphql core that given args and resolverInfo will return type map

[IvanGoncharov]

@terion-name Taking query from StackOverflow as an example:

class Query {
  document(args, ctx, info) {
    const argDefs = info.parentType.getFields()[info.fiedName].args;
    for (const argName of Object.keys(args)) {
      const argValue = args[argName];
      const argType = argDefs.find(arg => arg.name === argName).type;
      if (isInputObjectType(argType) {
        for (const argFieldName in Object.keys(argValue)) {
          const argFieldType = argType.getFields()[argFieldName];
          // ...
        }
      }
    }
  }
}

Is it something close to what you are trying to do?
If not can you please provide more details about your use case?

[terion-name]

@IvanGoncharov thank you, after some fixes — yes, it is close to desired. Just need to tweak it for recursion, I will dig in this.

And my point that this is very, very useful data for input processing and it would be great to have something like this in core library. Sort of TypeInfo but for arguments.

[terion-name]

@IvanGoncharov thank you for help, I've written a rough draft of function I speak of:

function visitArguments(args, info, visitor = {}) {
  let path = [];
  let result = {};
  const argDefs = info.parentType.getFields()[info.fieldName].args;
  for (const argName of Object.keys(args)) {
    const argValue = args[argName];
    const argType = argDefs.find(arg => arg.name === argName).type;
    path.push(argName);
    const apply = (argType, argValue, argName, path) => {
      const cbs = [visitor.enter, visitor[argType.toString()]].filter(cb => !!cb);
      cbs.forEach(cb => {
        const process = cb(argType, argValue, argName, path);
        argValue = process === undefined ? argValue : process;
      });
      return argValue;
    };
    const traverse = (argType, argValue, argName) => {
      argValue = apply(argType, argValue, argName, path);
      if (isInputObjectType(argType)) {
        for (const argFieldName of Object.keys(argValue)) {
          path.push(argFieldName);
          const argFieldType = argType.getFields()[argFieldName];
          if (isListType(argFieldType.type) || isListType(argFieldType.type.ofType)) {
            const innerType = getNamedType(argFieldType.type);
            return argValue[argFieldName].map(v => {
              return traverse(innerType, v, argFieldName);
            });
          }
          if (isInputObjectType(argFieldType.type)) {
            return traverse(argFieldType.type, argValue[argFieldName], argFieldName)
          }
          argValue = apply(argFieldType.type, argValue[argFieldName], argFieldName, path);
          path.pop();
          return {[argFieldName]: argValue};
        }
      }
      return argValue;
    };

    const processed = traverse(argType, argValue, argName);
    switch (processed) {
      case undefined:
        result[argName] = argValue;
        break;
      case null:
        break;
      default:
        result[argName] = processed;
    }
    path = [];
  }
  return result;
}

And some examples:

const resolver = function (parent, args, ctx, info) {
  args = visitArguments(args, info, {
    enter(type, value, name, path) {
      // transform input
      if (name === 'first') {
        return argValue > 10 ? 10 : argValue;
      }
    },
    DocumentWhereInput(type, value, name, path) {
      // validate
      validatorForDocumentWhere(value); // throw on error
    }
  });
}

[IvanGoncharov]

And my point that this is very, very useful data for input processing and it would be great to have something like this in core library. Sort of TypeInfo but for arguments.

@terion-name Makes sense 👍 Validation and transformation of complex argument is definitely something that we need to improve. Moreover, I just realize that you can use something like this to generate SQL query based on your args similar to how print uses visit to convert AST to string: https://github.com/graphql/graphql-js/blob/master/src/language/printer.js

So I think it would be extremely useful to have such function inside src/utilities but I think it shouldn't be limited to only calling it from resolver so I propose to change API as follows:

const resolver = function (parent, args, ctx, info) {
  const argDefs = info.parentType.getFields()[info.fiedName].args;
  args = visitArgumentsWithTypes(args, argDefs, {
    enter(type, value, name, path) {
      ...
    },
  });
}

For example, you want to analyze arguments as part of validation, in this case, you don't have access to info object.

Also, having enter and DocumentWhereInput on the same level is ambiguous since you can create a type named enter or leave. In some systems, users are able to provide their own custom types (e.g. GraphQL CMS) so by creating type enter they would be able to mess with internal implementation, so how about something like this:

{
  enter(type, value, name, path) {
    // ...
  },
  types: {
    DocumentWhereInput(type, value, name, path) {
      // ...
    }
  }
}

Do you interested in working on such PR?

[terion-name]

For example, you want to analyze arguments as part of validation, in this case, you don't have access to info object.

seems reasonable. then it will be useful to add a function that will extract arg defs from info, to prevent writing info.parentType.getFields()[info.fiedName].args construction in resolvers

Also, having enter and DocumentWhereInput on the same level is ambiguous since you can create a type named enter or leave

yes, I've thought about this but didn't find a bulletproof interface for this. Your variant looks interesting

Do you interested in working on such PR?

Of course. I'm currently testing out the concept in a project I develop, after some proofing I'll start a PR

[IvanGoncharov]

Of course. I'm currently testing out the concept in a project I develop, after some proofing I'll start a PR

@terion-name Great 🎉 Just open WIP PR after you stabilize API so I could provide an early feedback.

[terion-name]

@IvanGoncharov some thoughts during proofing. It became obvious that this function should be async, to allow async visitors that will perform remote calls for validation or other async actions (uploading files or interacting with S3).

One thing I am confused of — is should it respect NotNull type. E.g. should the function trigger visitor for MyType on MyType! input. As for now it does, but maybe there could be cases when it should respect this modifier?

[IvanGoncharov]

some thoughts during proofing. It became obvious that this function should be async, to allow async visitors that will perform remote calls for validation or other async actions (uploading files or interacting with S3).

@terion-name Using Promises will absolutely kill performance for people who want to use this function for simple validation/transformation. Not sure about your use case but can you use external promise like that:

const resolver = function (parent, args, ctx, info) {
  const argDefs = info.parentType.getFields()[info.fiedName].args;

  let argsActionPromise = Promise.resolve();
  args = visitArgumentsWithTypes(args, argDefs, {
    enter(type, value, name, path) {
      argsActionPromise = argsActionPromise.then(someValidation);
    },
    leave(type, value, name, path) {
      argsActionPromise = argsActionPromise.then(someTask);
    },
  });

  return argsActionPromise.then(() => getData());
}

If not can you please give some simplified example of your usecase?

One thing I am confused of — is should it respect NotNull type. E.g. should the function trigger visitor for MyType on MyType! input. As for now it does, but maybe there could be cases when it should respect this modifier?

Yeah, that's tricky 🤔. The alternative idea: can you bind callbacks to fields instead of types:

{
  enter(type, value, name, path) {
    // ...
  },
  'SomeType::someField': (type, value, name, path) => {
    // ...
  }
}

As a bonus, it works similar to resolve on output types and solves the problem with enter/leave name clash.

[terion-name]

If not can you please give some simplified example of your usecase?

Moving files for example. Two variants:

1. with apollo upload server I get promises with file streams in args and need to process them
2. or an external uploader sends me path in s3 storage in args and I need to move these files (also async operation)

[IvanGoncharov]

@terion-name Make sense.
However be cautious of Promises performance since even if you do Promise.resolve(0) is still scheduled through the event loop.
So in your implementation, you need to propagate Promise only if it's returned from visitor fn, see execute for example on how you can do that:
https://github.com/graphql/graphql-js/blob/master/src/execution/execute.js