Question about the place of this package in GraphQL.NET ecosystem

[sungam3r]

Hi @tlil . Reading #235 I opened readme to see what this projct about. As it states

This library is a complementary layer on top that allows you to automatically wrap your .NET classes into GraphQL schema definitions using existing property getters and methods as field resolvers.

At the moment, the main package also provides such a feature. It was almost written by @Shane32 . I assume that some parts of this project may now be irrelevant. I must admin that I do not use your project and did not watch its source code.

[sungam3r]

If everything is really as I believe, now such a situation can confuse people when they do not understand what they should use.

[Shane32]

I have never used this project either. However, after looking through it a bit, I have a few comments:

• While it is likely that the new AutoRegisteringGraphTypes can easily replicate the majority of the features of the conventions project, it should not be hard to maintain this project for existing users. Issue #234 is not the fault of this project, for example, and there were minimal changes required to update this project to be compatible with GraphQL.NET 7.

• The new AutoRegisteringGraphTypes infer nullability via NRT attributes or via manually specified .NET attributes. There is no need for the NonNull and similar wrappers used by the conventions project.

• The new AutoRegisteringGraphTypes do not have support for union types besides directly via UnionGraphType. So the object graph types that a union represents can be auto-registering graph types, but the union itself cannot be.

• AutoRegisteringGraphTypes precompile field resolvers, so they are as efficient as "native" code-first field resolvers, with no extra memory allocations. The conventions project does not do this, and runs some logic at runtime.

• The AutoRegisteringGraphTypes supports extensive modifications either via .NET attributes applied to classes/methods/arguments, global attributes, or via deriving from the auto-registering class and incorporating behaviors there. This allows to do things like override the name of a field/argument, have a method argument pull a service from DI, override the graph type, and so on. I am not sure if the Conventions project has similar features.

• The AutoRegisteringGraphTypes fully support synchronous and asynchronous methods, either via Task<T> or ValueTask<T>. I'm not sure if the conventions project does or does not.

• The AutoRegisteringGraphTypes are fully compatible with and based upon code-first graph types. As such, it is relatively easy to add code-first code on top of an auto-generated graph type. It is also easy to intermix some auto-generated graph types into a code-first project, or configure a custom code-first graph type for an otherwise auto-generated schema. I'm not sure if this can be done with the Conventions project.

• It is likely that while this project and the new AutoRegisteringGraphTypes have the same goals, they may be implemented differently enough that it would take a considerable amount of effort to switch a large schema over.

• The new AutoRegisteringGraphTypes have 100% code coverage, for what it's worth, as well as XML comments for every method. However, they are not well documented within the official GraphQL.NET docs. Just as a starting point, there should be a page added covering setup nuances and common techniques that may be required, such as how to implement unions. Ideally I would like to see all the docs for GraphQL.NET covering "schema-first", "code-first" and now "type-first" for each sample, plus a copy of the Harness sample written in "schema-first", "code-first" and "type-first". The conventions project may have better docs/samples at this time.

[floge07]

• The AutoRegisteringGraphTypes supports extensive modifications either via .NET attributes applied to classes/methods/arguments, global attributes, or via deriving from the auto-registering class and incorporating behaviors there. This allows to do things like override the name of a field/argument, have a method argument pull a service from DI, override the graph type, and so on. I am not sure if the Conventions project has similar features.

• The AutoRegisteringGraphTypes fully support synchronous and asynchronous methods, either via Task<T> or ValueTask<T>. I'm not sure if the conventions project does or does not.

I think I can give an example to hopefully answer some of these questions:

[Name("getTestText")]
[Description("this will return an empty string")]
public async Task<NonNull<string>> Test(
    IUserContext userContext,
    [Inject] ServiceA a,
    [Inject] ServiceB b,
    [Name("arg1")] int argument1,
    [Name("arg2")] [Description("this is a description for this argument")] string text
) {
    return string.Empty;
}

• async is supported.
• There are [Name] and [Description] Attributes you can use on Arguments and Fields.
• There is the IDependencyInjector service, which is used by the [Inject] Attribute. In our case, we implement that Interface and just call IServiceProvider.GetService()

[sungam3r]

From this example I see that GraphQL.NET can do the same. It has NameAttribute to set name as well but not for description. I can't find one. Did we write attribute for description, @Shane32 ? NonNull wrapper is not needed, you may use NRT instead. Use [FromUserContext] on IUserContext argument and [FromServices] instead of [Inject] . NameAttribute works for parameters as well.

[sungam3r]

Ah, DescriptionAttribute is one of those from System.ComponentModel namespace. GraphQL.NET supports those attributes for a long time.

[tlil]

From this example I see that GraphQL.NET can do the same. It has NameAttribute to set name as well but not for description. I can't find one. Did we write attribute for description, @Shane32 ? NonNull wrapper is not needed, you may use NRT instead. Use [FromUserContext] on IUserContext argument and [FromServices] instead of [Inject] . NameAttribute works for parameters as well.

[tlil]

@sungam3r I'll close this issue now; I don't see anything constructive here to be honest, other than critique of differences in work that has been years in the making.

This project was started more than 6 years ago, has been used and running in production systems since its inception, and serves use cases for a bunch of users as-is.

I assume that some parts of this project may now be irrelevant.

I am not going to force anyone to migrate and refactor their code bases just because there are synergies and opportunities to consolidate some of the work with what's later been added to the parent project (e.g., AutoRegisteringGraphTypes, which is a much more recent addition). I'd rather give our users the option of choose what suits their use case and preference better.

Also, re comments above such as the "NotNull wrapper is not needed" – this stems from the fact that this project predates NRT language support. :-)

[Shane32]

Well said