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:
StringType part is effectively unnecessary and redundantI'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?
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
NonNullListfor 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:
.Type<NonNullType<X>>.Type<NonNullType<ListType<X>>.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>()
.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
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:
.Type<ListType<NonNullType<X>>.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!
Most helpful comment
Another option would be:
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: