Field Directive Fails with Pipe Symbol in Object Arg Value

[creatzor]

Describe the bug
Given a code-first approach with a field directive that has an arg with a value that include the pipe symbol |, 'Syntax Error: Expected <EOF>, found |' is produced while generating a schema.

To Reproduce
Create an object type:

@ObjectType()
export class Thing {
  @Field(type => Boolean)
  @Directive('mydirective', { param1: 'A|B' })
  type: boolean;
}

Create a resolver for the object type

 @Resolver(of => Thing)
  export default class ThingResolver {
    @Query(r => Int, {name:'foo'})
    foo(){
      return 2
    }
  }

Create schema

  try {
    const { typeDefs, resolvers } = await buildTypeDefsAndResolvers({
      resolvers: [ThingResolver]
    });

    const schema = makeExecutableSchema({
      typeDefs: typeDefs,
      resolvers
    });
  }catch(err){
    console.error(err);
  }

Expected behavior
No error

Logs
GraphQLError: Syntax Error: Expected <EOF>, found | at syntaxError (node_modules/graphql/error/syntaxError.js:15:10) at Parser.expectToken (node_modules/graphql/language/parser.js:1404:40) at Object.parseValue (node_modules/graphql/language/parser.js:54:10) at node_modules/type-graphql/dist/schema/definition-node.js:95:34 at Array.map (<anonymous>) at getDirectiveNode (node_modules/type-graphql/dist/schema/definition-node.js:89:42) at Array.map (<anonymous>) at Object.getFieldDefinitionNode (node_modules/type-graphql/dist/schema/definition-node.js:52:39) at node_modules/type-graphql/dist/schema/schema-generator.js:193:60 at Array.reduce (<anonymous>) { message: 'Syntax Error: Expected <EOF>, found |', locations: [ { line: 1, column: 14 } ] }

Enviorment (please complete the following information):

• OS: OSX 10.14.6
• Node 13.1.0
• Package version ^0.18.0-beta.9
• TypeScript version 3.7.5

Additional context
https://github.com/MichalLytek/type-graphql/blob/master/src/schema/definition-node.ts#L129 is where the error occurs. When parseValue(args[argKey]) is replaced with args[argKey] it works as expected. However, it could pose problems for other use-cases (which are unknown to me). In my opinion, it should be a strict requirement that if users supply an object to the directive args, the values of the object must be string, number, boolean, null, or undefined.

@j what was the motivation for using parseValue there? Why is it needed?

[MichalLytek]

the values of the object must be string, number, boolean, null, or undefined.

What about arrays and enums?

was the motivation for using parseValue there? Why is it needed?

The JSDocs tells that:

Given a string containing a GraphQL value (ex. [42]), parse the AST for that value

As the object syntax for directives does not work well for string and apparently for arrays, I think I'm gonna remove it in next beta as it's not usable for 0.18 release.

[creatzor]

Ah I see, I realized very shortly after posting that parseValue would be needed to generate the ast even if it was a simple type. so replacing parseValue(args[argKey]) with args[argKey] would be invalid.

I was thinking that for more complex types, users would have to use the full definition syntax of the directive. @Directive('@mydirective(param1: [SOME_ENUM])') but thanks for the heads up! It works as expected without the object syntax.. and in terms of consistency it is better to have just 1 notation which also aligns with the rest of the ecosystem, so i think removing the object syntax is a good move.

Thank you!

[MichalLytek]

Leaving that as open, I will close it when the object syntax has been removed.

[MichalLytek]

JS-like syntax support has been temporarily disabled in 78b524e 🔒