Should I use GraphQL for fine-grained validation?

[SimonDegraeve]

For example, I want to validate an email field.

Should I define my own email scalar type or just use GraphQLString and validate the value in the resolve function?

new GraphQLScalarType({
  name: 'Email',
  coerce: value => '' + value,
  coerceLiteral: ast => ast.kind === Kind.STRING && ast.value.match(/somePattern/) ?
    ast.value :
    null
});

Similar question for checking the length of a string. If I have a field that map to a VARCHAR(100) in a MySQL database, should I create a specific scalar that check for a valid length? Not sure if it's belongs to GraphQL... or if it is a good practise.

Any thoughts?

Thanks

[SimonDegraeve]

From slack talk:

currently we have “coerceLiteral” for this type of thing. However this is quite underspecified (especially in terms of errors) and we should firm this up before we finalize the spec

[leebyron]

FYI, if you follow this pattern, be sure to do validation for value coercion in addition to literal coercion.

I think this is an area we will need to cover in deeper documentation. As long as the validation has semantic value, I think this pattern is a good one. For example, at Facebook we have a type called "URL" which explains very clearly what kind of value will come from such a field and allows us to do validation if it's provided as input.

IMHO, doing this for "Email" makes a lot of sense, and I imagine you would want to use this not just to describe semantic input but also output

[SimonDegraeve]

Ok, thank you.
So it could be useful to expose Kind from lib/language...

And also in the comment, it is only specified coerce and not coerceLiteral but if you don't provide both you get an error.

[leebyron]

Nice catch. I'm leaving this open as a reminder to improve comments and docs

[SimonDegraeve]

Could you give a quick explanations about the difference between both coerce and coerceLitteral?
The one that validate from the AST make sense but the other one, I am not sure when it is used...

[leebyron]

There are two scenarios:

1. when used as the type of a field of an object, the value returned by the resolve function will be passed through coerce first. This ensures, for example, that Boolean is always Boolean, Int is always an integer, and String is always a string.

2. when input is provided as a query variable, the value of that query variable passed at runtime is first passed through coerce to ensure its of the correct type.

[SimonDegraeve]

Allright, clear as crystal.

[SimonDegraeve]

Not really related, but since I am digging into scalars....
This line for GraphQLFloat seems wired

// [...]
coerce(value) {
    var num = +value;
    return num === num ? num : null; // <= 
  },

[leebyron]

Thanks for the note, I'll add a comment in code to explain what's going on here.

+value calls ToNumber internally.

num === num is a cheap way to identify if num is the value NaN. NaN is not equal to anything, including itself. Everything else will be a number.

[SimonDegraeve]

Sorry, if it sounds like a stupid question but the schema is supposed to be server-side only, or it can be embedded in client?

I ask because if I want to share some enhanced types (ex: GraphQLExtrasString({min:2, max:10}), it is important to know if the build size matters.

UPDATE: I am not just worried about the the build size, but also about features like Buffer which is only available on the server.

[leebyron]

It's expected that schema is represented client-side, usually in the form of code generation. You could imagine generating an NSObject, Java Object, or Flow type for every type represented in a GraphQL schema so clients can benefit from strong typed data.

Of course, if your server presents custom scalars, this can be a challenge for these tools. You could imagine defining a DateTime scalar in GraphQL and wanting to ensure that they become JS Date and Java DateTime, etc. That's custom logic in those tools.

[SimonDegraeve]

So to be clear, this is wrong?

/**
 * Import dependencies
 */
import SomeLibWorkingOnlyServerSide from 'some-lib';
import {GraphQLScalarType} from 'graphql';
import {Kind} from 'graphql/lib/language';
import {GraphQLError} from 'graphql/lib/error';


/**
 * Export enhanced String scalar type
 */
export default class String {
  constructor(options = {}) {

    return new GraphQLScalarType({
      name: 'String',

      coerce(originalValue) {
        const {error, value} = SomeLibWorkingOnlyServerSide(originalValue, options);

        if (error) {
          throw new GraphQLError(error.message);
        }

        return value;
      },

      coerceLiteral(node) {
        if (node.kind !== Kind.STRING) {
          return null;
        }

        const {error, value} = SomeLibWorkingOnlyServerSide(node.value, options);

        if (error) {
          throw new GraphQLError(error.message, [node]);
        }

        return value;
      }
    });
  }
}

[leebyron]

Well, at a high level no, but this implementation in particular has a lot of issues:

To be clear: there's nothing wrong with defining custom scalars, that's exactly why the GraphQLScalarType class constructor is exposed.

With this particular code there are a couple problems:

First, just from a JavaScript point of view, a class constructor should not return an object of another type (in fact, I'm surprised Babel isn't choking on this).

You should instead do:

export var CustomString = new GraphQLScalarType({

Next, you can't name your custom scalar String since the GraphQL spec defines the behavior of the String type which this doesn't adhere to, and this will result in multiple definitions of String which will soon be a validation error (when the related issue is completed).

Here would be code that is perfectly fine:

/**
 * Import dependencies
 */
import SomeLibWorkingOnlyServerSide from 'some-lib';
import {GraphQLScalarType} from 'graphql';
import {Kind} from 'graphql/lib/language';
import {GraphQLError} from 'graphql/lib/error';


/**
 * Export enhanced String scalar type
 */
export default function CustomString(typeName, options = {}) {
    return new GraphQLScalarType({
      name: typeName,

      coerce(originalValue) {
        const {error, value} = SomeLibWorkingOnlyServerSide(originalValue, options);

        if (error) {
          throw new GraphQLError(error.message);
        }

        return value;
      },

      coerceLiteral(node) {
        if (node.kind !== Kind.STRING) {
          return null;
        }

        const {error, value} = SomeLibWorkingOnlyServerSide(value, options);

        if (error) {
          throw new GraphQLError(error.message, [node]);
        }

        return value;
      }
    });
}