Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom directives #159

Open
BilyachenkoOY opened this issue Jun 3, 2019 · 4 comments
Open

Support custom directives #159

BilyachenkoOY opened this issue Jun 3, 2019 · 4 comments

Comments

@BilyachenkoOY
Copy link
Contributor

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
Copy link
Collaborator

tlil commented Oct 29, 2020

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

@K-Pavlov
Copy link
Contributor

@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?

@sungam3r
Copy link
Member

@K-Pavlov Working with directives (like so much else) has been heavily changed in GraphQL.NET v4. See graphql-dotnet/graphql-dotnet#2276 and especially https://github.com/graphql-dotnet/graphql-dotnet/pull/2276/files#diff-8cc34927497a4d6a5aa8a4626fee297f7090758816221962d3a312d89bc522c7

@K-Pavlov
Copy link
Contributor

@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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants