graphql-dotnet/conventions

Support custom directives

BilyachenkoOY opened this issue · 4 comments

BilyachenkoOY commented

It would be nice to add support of custom directives, since currently it is only available via reflection to get _schema from GraphQLEngine and invoke RegisterDirective passing DirectiveGraphType.
There are few ways:

  • Add public getter to access _schema, so that any custom operation wich is possible in graphql-dotnet will be possible with graphql-dotnet conventions
    • this is the simplest but cons is that it only possible after BuildSchema executed
  • Intoduce WithDirective(DirectiveGraphType) method which will be just a proxy to _schema
    • it can remember provided type until BuildSchema is called or register directory immediately to _schema if it is already constructed
  • Add adapter as for Queries and Mutations so that is would be possible to describe directive like a class: 
    [Name('mydircetive')]
[DirectiveLocations(DirectiveLocation.Field)]
class Directive {
  public int Count { get; set; } // some argument
}
tlil commented

Keen to raise a PR? Ideally something like option 3, but you could also go with option 2 as a stepping stone.

K-Pavlov commented

@tlil @BilyachenkoOY Hey guys, I implemented option 2 and did some investigation related to it as a first step to this enhancement - looking at graphql-dotnet Schema.cs, registering directives is treated the same way as registering types, so the WithDirective(DirectiveGraphType) should not call _schema.RegisterDirective/s directly as this can cause an InvalidOperationException if the the Schema is Initialized; Instead it should work like WithQuery for example, only remembering the provided types;
Thinking about where the correct place to add the Directives would be -
First and most simple place I considered - in GraphQLEngine.BuildSchema(SchemaPrinterOptions, params System.Type[]), add them with a call to _schema.RegisterDirectives; Doesn't seem correct, the GraphQLEngine class starts taking responsibilities, which are not his. Also adding support for option 3 would require an overhaul or would result in even more unrelated code in GraphQLEngine. Adding them in SchemaConstructor.Build (seems like the most appropriate place) would involve pretty much the same complexity as just directly going with option 3, making option 2 kind of pointless;
Continuing with option 3:
What should the end result look like? Should it be like a marking attribute? And then based on the attributed all marked classes are collected and transformed to DirectiveTypeInfo

[GraphQLDirective(name, locations)]
public class Directive
{
    // only properties / fields in this class are handled; Derive types and transform the properties / fields to QueryArgument
     public int Count { get; set; } 
}

Or should it be somewhat like the Query / Mutations classes, having a wrapper class, which will be passed to a new method in GraphQLEngine - WithDirective. Then all first level nested classes will be handled as Directives

public class Directive
{
   [Name('mydircetive')]
   [DirectiveLocations(DirectiveLocation.Field)]
    public class FirstDirective
    {
        // ....
    }
    
   [Name('mydircetive')]
   [DirectiveLocations(DirectiveLocation.Field)]
    public class SecondDirective
    {
         // ....
    }
};

These are the two options that come to mind when I think about describing directives as classes - it is completely possible that I am missing something here. Can you give me an example of how exactly the end result should look like?

K-Pavlov commented

@sungam3r Thanks for the heads-up! It would be better to wait for GraphQL.NET v4 before implementing this. Also it seems reasonable that adding support for server-side directives when fields are generated should be part of this enhancement (a field / property attribute seems reasonable); Excellent work on the directives and the much appreciated documentation 👍

  • 👍1