[RFC] "3.1.5 Enums" - requires additional consideration. Use complex objects with functions for enum values.

[nodkz]

In graphql-compose-connection I use Enum type for storing avaliable sortings for connections, so values represent complex objects with function, eg:

var ConnectionSortUserEnum = new GraphQLEnumType({
  name: 'ConnectionSortUserEnum',
  values: {
    AGE_ID_DESC: {
      uniqueFields: ['age', '_id'],
      sortValue: { age: -1, _id: 1 },
      directionFilter: (cursorData, filter, isBefore) => {
        // some code for extending filter with data obtained from `before` or `after` cursors 
        return filter;
      },
    },
    ...someOtherSorts
  }
});

With such Enum graphql server starts and works perfectly (example).

But throws error when I try to build introspection graphql(Schema, introspectionQuery) or run printSchema(Schema):

[11:33:10] Starting 'buildSchema'...
ERROR introspecting schema:  [
  {
    "path": [
      "__schema",
      "types",
      1,
      "fields",
      21,
      "args",
      5,

      "defaultValue"
    ],
    "originalError": {}
  }
]
Error
    at invariant (invariant.js:19:11)
    at astFromValue (astFromValue.js:112:3)
    at astFromValue.js:123:22
    at Array.forEach (native)
    at astFromValue (astFromValue.js:117:23)
    at printInputValue (schemaPrinter.js:180:44)
    at Array.map (native)
    at printArgs (schemaPrinter.js:174:39)
    at schemaPrinter.js:155:28
    at Array.map (native)
    at printFields (schemaPrinter.js:154:17)
    at printObject (schemaPrinter.js:121:65)
    at printType (schemaPrinter.js:100:12)
    at Array.map (native)
    at printFilteredSchema (schemaPrinter.js:72:87)
    at printSchema (schemaPrinter.js:37:10)

According to astFromValue.js#L116 it accepts only string values for Enums:

  // JavaScript strings can be Enum values or String values. Use the
  // GraphQLType to differentiate if possible.
  if (typeof _value === 'string') {
    if (type instanceof GraphQLEnumType &&
        /^[_a-zA-Z][_a-zA-Z0-9]*$/.test(_value)) {
      return ({ kind: ENUM, value: _value }: EnumValue);
    }
    // Use JSON stringify, which uses the same string encoding as GraphQL,
    // then remove the quotes.
    return ({
      kind: STRING,
      value: JSON.stringify(_value).slice(1, -1)
    }: StringValue);
  }

• But in graphql specification https://facebook.github.io/graphql/#sec-Enums I did not found explicit requirement that enum value must be string.
• Also in flow-type definitions value has any type:

export type GraphQLEnumValueConfig/* <T> */ = {
  value?: any/* T */;
  deprecationReason?: ?string;
  description?: ?string;
}

So, according to concept of Enum ...

• client gets only names from enum
• enum values can be defined only on server (and client does not know anything about internal enum value)

... I think, that if developer define complex value for enum, so it expects that get it in args for resolve function. And astFromValue should not deeply extract AST for enum's value it should stops on enum's name.

I don't know enough FB internals requirements for ENUMs, so can not provide correct PR for this problem. @leebyron please consider this issue, as soon as possible, because it blocking me.

PS. Passing to enum's value arrow functions, provides for me awesome ability to customise logic in resolve methods - https://github.com/nodkz/graphql-compose-connection/blob/master/src/resolvers/connectionResolver.js#L207

[nodkz]

Deeper investigations shows that problem with defaultValue for input field with Enum type.
Eg.

const RootQuery = new GraphQLObjectType({
  name: 'RootQuery',
  fields: {
    'userConnection': {
      type: UserConnection,
      args: {
         first: { type: GraphQLInt },
         after: { type: CursorType },
         last: { type: GraphQLInt },
         before: { type: CursorType},
         ...additionalArgs,
         sort: {
           type: SortEnumType,
           defaultValue: SortEnumType.getValues()[0].name, // <<<< problem here  <<<<<<<<<<<
           description: 'Sort argument for data ordering',
         },
      },
   },
});

• If sort arg provided for query (eg. { userConnection(first: 5, sort: AGE_ID_DESC) { ... }) then
  • astFromValue works without errors 💚
  • resolve functions gets complex object via args.sort 💚
• If sort is not provided, then it takes defaultValue from config option:
  • when defaultValue equals SortEnumType.getValues()[0].name, which equals to AGE_ID_DESC, then
    • astFromValue works without errors 💚
    • resolve function gets string name of sorting value in args.sort (I think it should implicitly convert enum name to value) 💔
  • when defaultValue equals SortEnumType.getValues()[0].value, which equals to { age: -1, _id: 1 }, then
    • astFromValue throws error. It was called such way astFromValue(undefined, { age: -1, _id: 1 }) 💔
    • resolve function gets complex object in args.sort 💚

So what defaultValue should be provided for field with Enum type?

[leebyron]

I added a test to illustrate complex enums which passes correctly. I believe the issue here is that your enum definitions are ill-formed.

Enums are defined as a list of enum descriptors, one field of which is value. Example:

  const Complex1 = { someRandomObject: 1 };
  const Complex2 = { someOtherRandomObject: 2 };

  const ComplexEnum = new GraphQLEnumType({
    name: 'Complex',
    values: {
      ONE: { value: Complex1 },
      TWO: { value: Complex2 },
    }
  });

[leebyron]

b2c8332 also illustrates the use of the Enum getValues() method

[leebyron]

Added one last test illustrating defaultValue.

Are there any other concerns you have here? I'll add more tests

[nodkz]

Your tests answered to all my questions with enums. No more 💚/💔 in my head. Thank's a lot!!!

Enum definition in my case was covered by flow connected with graphql. Also I checked it with your example. And they are identical.

Problem in my case, that I use function in sort value. This function needs for passing custom logic to resolve function. And graphql(schema, introspectionQuery) throws error, when it tries build AST for function in the enum's defaultValue.

Add PR with broken test fd96f5a

[leebyron]

Thanks for isolating that! I'll take a closer look

[leebyron]

Closing this now, and will get this fix out in the next point release - feel free to reopen if theres anything I missed