A helper method equivalent to the [GraphQLNonNullType] attribute for IObjectFieldDescriptor

[marksmithhh]

Currently in Hot Chocolate, unless you use C# 8.0 nullable reference types (which is not really a nice-to-use and pretty feature, and has its own limitations and problems), you would have to explicitly specify which properties/methods that have a reference type should be non-nullable.

You could currently do this using attributes like this:

[GraphQLNonNullType]
public string Title { get; set; }

But to achieve the same thing using IObjectFieldDescriptor's fluent API, this is what you would have to do:

descriptor.Field(p => p.Title).Type<NonNullType<StringType>>();

which is:

• Ugly and long
• The StringType part is effectively unnecessary and redundant
• Looking at that line of code, you're not sure whether the type itself is being changed or it's just being set to non-nullable.

I'm sure you would agree that having a nice little helper method like NonNull() would make the code above way cleaner, more readable, and more understandable at first sight.

descriptor.Field(p => p.Title).NonNull();

It also makes sense to have such a method since we already have the attribute that does the exact same thing.

What do you think?

[aahmadi458]

Agreed.
I'm sick of .Type<NonNullType<Type...s at this point! :D

[aradalvand]

I initially thought descriptor.Field(b => b.Author).Type<NonNullType>() would work, but it apparently doesn't.

+ I would also like a method for lists, that is, a shorthand for:

descriptor.Field(p => p.Title).Type<NonNullType<ListType<NonNullType<SomeType>>>();

which is even uglier !
maybe a helper like:

descriptor.Field(p => p.Title).NonNullList();

[michaelstaib]

While I agree, I think this needs a bit more discussion.

There are a lot of edge cases here, for instance, ListsOfLists.
Why should NonNullList for instance translate to .Type<NonNullType<ListType<NonNullType<SomeType>>>().

I will add this to the December release and hope we get some more ideas in here. Once we have this API refined we might also rework the attributes to align them.

[aradalvand]

@michaelstaib

Why should NonNullList for instance translate to .Type<NonNullType<ListType<NonNullType<SomeType>>>()?

I assume you mean why not instead translate to .Type<NonNullType<ListType<SomeType>>() (non-nullable list of nullable elements)?
Well, simply because you should use NonNull() for that. That would make it go from (the default) Type<ListType<X>> to Type<NonNullType<ListType<X>>. That's what the attribute [GraphQLNonNullType] does too.

In general, we know that there are 3 common scenarios to cover:

1. .Type<NonNullType<X>>
2. .Type<NonNullType<ListType<X>>
3. .Type<NonNullType<ListType<NonNull<X>>>

Now, the first 2 could/should be handled by .NonNull(), but the last one could be handled by another method, which I suggested we could call .NonNullList() or if you want it to be more descriptive .NonNullListOfNonNull() perhaps. But if you find that verbose, how about .NonNull(nonNullElements: true)?

What do you think?

In the case of more complex scenarios, such as a list of lists like you mentioned, maybe the user should express what they need explicitly with the .Type<> method, but for other simpler scenarios (which take up about 90% of scenarios :D) those helper methods would be extremely useful.

[marksmithhh]

@michaelstaib

What about just allowing:

.Type<NonNullType>()

Pros:

• No need for extra helper methods
• Can handle ALL edge cases (e.g. list of lists, list of list of lists, etc.)

Cons:

• More verbose than .NonNull() obviously, but I think the advantages outweigh this disadvantage.

I myself originally suggested adding .NonNull() but given the fact that there are more edge cases than I initially thought, as @michaelstaib mentioned, now I would say .Type<NonNullType> is a good choice too.

Since our problem was basically the fact that SomeType in .Type<NonNullType<SomeType>> was redundant, now with this, our original problem is solved, in addition, any complex edge cases can also be handled.
For example, non-nullable list of nullable list of non-nulls:

.Type<NonNullType<ListType<ListType<NonNullType>>>()

You guys @AradAral @aahmadi458 also tell us what you think about this.

[PascalSenn]

Another option would be:

public IObjectFieldDescriptor NonNull(
    bool scalar = true,
    bool list = true,
    bool nestedList= true);

For scalars list and nested lists would the default behaviour of NonNull be that everything is non null.
It would be possible to override specific parts of the result.

Like:

foo: [[Bar!]!]!

descriptor.Field(x => x.Foo).NonNull();

foo: [[Bar]!]!

descriptor.Field(x => x.Foo).NonNull(scalar: false);

foo: [[Bar]]!

descriptor.Field(x => x.Foo).NonNull(scalar: false, nestedList: false);

foo: [[Bar]]

descriptor.Field(x => x.Foo).NonNull(scalar: false, nestedList: false, list: false);

foo: [[Bar!]]

descriptor.Field(x => x.Foo).NonNull(nestedList: false, list: false);

[aahmadi458]

Y'know I'm fine with both @PascalSenn's parameterized .NonNull(), and @marksmithhh's .Type<NonNullType>(). As long as I can get rid of the ugly long .Type<NonNullType<Something>>() lol. But seriously they both look pretty neat to me.

I have a question though, it might be irrelevant but how would someone achieve say a [[[[Bar]]!]] (list of list of non-nullable list of list of Bars) with @PascalSenn's .NonNull()? I know it's almost ridiculous and it's extremely rare to have a three-or-four-dimensional list, but if we're talking edge cases then... :P
In other words, what would nestedList be referring to exactly if we had a more than 2 dimensional list?
So does that mean that .Type<NonNullType>() is more flexible in that aspect?

[PascalSenn]

@aahmadi458
we do not have to worry about 3 or more dimensions because in GraphQl only two dimensional lists are supported. This is defined by the offical specification

[aahmadi458]

@PascalSenn
Oh okay, I didn't know that. There's no problem then. What you suggested can handles edge cases, basically.
Would you guys then consider this for the HC-11.0.0 milestone, instead of HC-11.1.0, perhaps?