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

Created on 17 Oct 2020  路  18Comments  路  Source: ChilliCream/hotchocolate

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?

馃尪 hot chocolate 馃帀 enhancement 馃棧 discussion

Most helpful comment

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();



md5-0e5320fe8b53dedcb92ef71cb82060b6



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



md5-65946488cef623ab3b66338540102351



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



md5-b569bb5439b3bbb7cb04656228e9b986



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



md5-6cc54ac29c9b5717ddfa1e78ada01b05



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

All 18 comments

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

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();

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.

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

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

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();



md5-0e5320fe8b53dedcb92ef71cb82060b6



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



md5-65946488cef623ab3b66338540102351



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



md5-b569bb5439b3bbb7cb04656228e9b986



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



md5-6cc54ac29c9b5717ddfa1e78ada01b05



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

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?

@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

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

As a reply to myself:

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

I realized there is actually another common scenario which I missed: A nullable list of non-nullable elements:

  1. .Type<ListType<NonNullType<X>>
    And this is not achievable with either my .NonNull() or .NonNullList(), except if NonNullList() received a parameter (like nonNullList) which I think would make it a little unintuitive. So, maybe we can rule out my proposals!

Eventually, I've come to like both .Type<NonNullType>(), AND a single .NonNull() method with optional parameters, which would default to everything being non-nullable, as @PascalSenn proposed

They're both intuitive.

So, I'd say whichever you guys @michaelstaib @PascalSenn finally go for is a matter of difficulty of implementation and other under-the-hood factors maybe. The external interface of both is equally nice.

I also believe these 2 are the only reasonable ways of implementing this helper. So, case closed!

Did this make it into code yet?

I have a scenario where I has a Basket type and a BasketItem type. I've written the descriptor and resolver for items but that generates a type in the schema showing the list as nullable.

I cant figure out for the life of me how to tell the descriptor that the 'items' property in Basket is a non-nullable list of Item and that each of those items is non-nullable.

type Basket {
items: [BasketItem!]!
}

Isn't this a common requirement?

@ztolley Why can't you do the following?!

descriptor.Field(b => b.BasketItems).Type<NonNullType<ListType<NonNullType<BasketItem>>>>()

ok. Trying that, just have to get around

Solution OrderApi.sln
Project GraphQL
GraphQL\Baskets\BasketType.cs:24 The type 'OrderAPI.GraphQL.Data.BasketItem' must be convertible to 'HotChocolate.Types.IType' in order to use it as parameter 'T' in the generic class 'HotChocolate.Types.NonNullType'

Thanks for the pointer

You should have a schema type for BasketItem. Something like BasketItemType that derives from ObjectType<BasketItem>. Replace BasketItem with BasketItemType in .Type<NonNullType<ListType<NonNullType<BasketItem>>>>() in order to get rid of the error.

That worked. Looking forward to a sweeter syntax :)

Any updates on this?

Not yet... at the moment focus is on the client API. We will have a look at this once we have implemented the main features for 11.1

@michaelstaib Ok, fine.
Happy to hear that the focus is on Strawberry Shake!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hognevevle picture hognevevle  路  3Comments

louisjrdev picture louisjrdev  路  3Comments

nigel-sampson picture nigel-sampson  路  5Comments

marcin-janiak picture marcin-janiak  路  4Comments

RohrerF picture RohrerF  路  3Comments