Introduce `MaybeSet<T>`: A better version of `Optional<T>`

[aradalvand]

Product

Hot Chocolate

Is your feature request related to a problem?

Optional<T> tells you whether an argument (or an input field, for that matter, but I'm only going to use the word "argument" from now on for simplicity's sake) was explicitly supplied by the client or not; however, suffice it to say, it has problems, and it has consistently shown to confuse people:

• https://github.com/ChilliCream/graphql-platform/issues/1808
• https://github.com/ChilliCream/graphql-platform/issues/6025
• https://github.com/ChilliCream/graphql-platform/issues/4353
• https://github.com/ChilliCream/graphql-platform/issues/5009
• https://github.com/ChilliCream/graphql-platform/issues/4354
• https://github.com/ChilliCream/graphql-platform/issues/2270#issuecomment-686356349

To be fair, Optional<T> does what it actually purports to do (although the name is very confusing and gives the wrong impression), but what it does is often "too specific" and it doesn't do quite enough for the real-world use cases in which it's typically used.

As was reiterated over and over again by Pascal and Michael in the issues linked above, unlike what its name implies, Optional<T> doesn't actually make the corresponding argument optional — which in GraphQL is achieved only either by making the type of the argument nullable, or giving it a default value — it just reports whether or not the argument was explicitly provided by the client, or whether it was omitted. This information can be useful in a number of scenarios, most notably when you have a mutation that allows clients to do partial updates, based on the arguments they choose to include when calling the mutation; this way, you, as the client, can specify exactly which fields you want to update. Simply making the argument nullable wouldn't be sufficient in this case since you wouldn't be able to distinguish a null value that the client explicitly supplied (e.g. they might have actually wanted to update that property and set it to null) from a null value that's just the default value of the argument because it wasn't supplied; so Optional<T> needs to be used in cases like this to provide this distinction.

For instance, in the example of a partial update mutation, this:

updateUser(input: {
    username: "newUsername",
    address: null
}) {
   success
   message
}

...means something different from this:

updateUser(input: {
    username: "newUsername"
}) {
   success
   message
}

...even though address is nullable (i.e. its default value is null) and so the eventual value will ultimately be the same in both cases. The omission/inclusion of address is semantically and behaviorally significant here. The first one means "set the address to null", whereas the second one means "leave the address as is"/"don't touch the address". Optional<T> comes to the rescue in these scenarios.

The problem arises when you need the following logic (assuming the same updateUser example above):

• Argument username is optional (i.e. can be omitted) — needless to say this would require the GraphQL argument to be nullable, because the GraphQL type system doesn't distinguish between nullability and optionality.
• But when username is explicitly provided, the value it's set to cannot be null (and ideally, an error should be thrown if this happened) — because the application's business logic doesn't allow a username-less user, for example.

This logic is very common, it's precisely what you would want when it comes to partially updating fields that are "actually" non-nullable. Currently, there's no way to express this to Hot Chocolate via Optional<T>, so you can easily shoot yourself in the foot without it being necessarily obvious:

var user = db.Users.Find(userId);
if (input.Username.HasValue) // if `username` was explicitly set to `null` by the client, this would evaluate to true and consequently `user.Username` would be set to `null`
    user.Username = input.Username;

But user.Username must not be null (and you won't even get a nullable reference type warning here by the way, because of this, so it's very easy to look at this code and not realize there's something wrong with it). You would have to add another condition:

var user = db.Users.Find(userId);
if (input.Username.HasValue && input.Username.Value.HasValue)
    user.Username = input.Username;

Which, apart from being obviously ugly and seemingly nonsensical, would also yield sub-optimal behavior; the optimal behavior would be to throw an error before hitting the resolver when an explicit null is provided for this field, so that the server lets the client know that there was something wrong with the input, as opposed to silently ignoring it like this would do.

The solution you'd like

In order to avoid a breaking change, I would propose we introduce a new struct: MaybeSet<T> (with properties bool WasSet and T Value). An argument of type MaybeSet<T> will always result in a nullable argument on the GraphQL side of things. However, if T (the runtime type) is NOT nullable, then it will throw an error if the client explicitly sets it to null. This means that if you have the following input type, for example

public record UpdateUserInput(
    MaybeSet<string> Username,
    MaybeSet<string?> Address
);

This would yield the following GraphQL input type:

input UpdateUserInput {
    username: String
    address: String
}

But then if the client explicitly sets username to null, an error would be thrown, because the runtime/CLR type was non-nullable (string), but the same thing would be permitted for address.

This is superior to Optional<T> in every way. It would:

• Establish orthogonality between nullability and optionality (which unfortunately isn't provided by the GraphQL type system, but nonetheless is a very commonly-needed thing (see this Stack Overflow question, for example) — thankfully the behavior can be enforced on the GraphQL-server-level.
• Eliminate the confusion that Optional<T> elicited in people's minds and fix its weirdness. For example, with Optional<int> you could end up with HasValue = false and Value = 5, which seems absurd at first glance, whereas in the case of MaybeSet<int>, since the naming is much clearer, you would have WasSet = false and Value = 5 in the same scenario, clearly implying that 5 wasn't explicitly supplied, but is the default value.
• Would be (almost) a drop-in replacement for Optional<T>, or more accurately, it would cover all the cases someone is currently using Optional<T> for; and would resolve people's grievances about Optional<T> once and for all (many of which were voiced in the aforementioned issues), so Optional<T> could also potentially be deprecated in favor of MaybeSet<T> at some point.

Although I must admit the name Optional would've been perfect for this construct (while it was confusing for the current Optional, it would be accurately descriptive of what I refer to as MaybeSet here) — so perhaps also consider replacing the current Optional with MaybeSet with the same name (Optional) but also changing the property name HasValue to WasSet (the name HasValue is fundamentally confusing here, as alluded to earlier), I think that would be the ideal outcome, if you can afford a breaking change. But if that's a nonstarter for you, there's nothing wrong with MaybeSet either, and they're both the same number of characters.

[ParadiseFallen]

any updates on this issue?

[xamir82]

This is direly needed IMO. Would love to see this added soon.

[glen-84]

Perhaps it should be:

v13 - Optional (old behaviour), MaybeSet (new behaviour) v14 - Optional (new behaviour), MaybeSet (deprecated)

Not sure about WasSet vs IsSet, I think I prefer the latter.

[michaelstaib]

I think this is just about convenience here :) We can add an extension method for Optional<T> called HasNonNullValue or something like that which basically checks for HasValue is true and Value is not null.

[glen-84]

That's not the same:

1. Optional<string> Example should result in example: String (nullable) in the schema.
2. You shouldn't need to choose between HasValue and HasNonNullValue based on whether or not the type is nullable.
3. HasValue would still return true if it holds an explicit null, even if the field is non-nullable.
4. There is no validation if an explicit null is sent for a field that is supposed to be non-nullable.

etc.

Arad does a great job of detailing the current issues with Optional<T>.

[michaelstaib]

I think you are wrong. HasValue and HasNonNullValue is about semantics not about nullability of the type ... depending on your business logic you can decide which one you want to use.

[michaelstaib]

Rereading this ...

its not superior ... as it supports not all use-cases of Optional. This would break the use-cases of many use-cases I know of.

[michaelstaib]

However I can see how this can be useful as an alternative to Optional<T>. I have put it on the backlog so core team can discuss this and decide if we take it on or close it.

[glen-84]

I think you are wrong. HasValue and HasNonNullValue is about semantics not about nullability of the type ... depending on your business logic you can decide which one you want to use.

1. As mentioned in the OP, this can easily lead to bugs (calling HasValue instead of HasNonNullValue).

2. As a developer, I want to know if a value has been set by the client, regardless of its nullability.

   if (input.Username.IsSet) // same property to check
       user.Username = input.Username; // string
   
   if (input.Address.IsSet)
       user.Address = input.Address; // string or explicit null
   

   vs

   if (input.Username.HasNonNullValue) // easy to forget to use
       user.Username = input.Username; // string
   
   if (input.Address.HasValue)
       user.Address = input.Address; // string or explicit null
   

3. Optional<string> should be valid, without having to set a default value for GraphQL (it should be nullable in the schema).

4. It is based on the nullability of the type, because if the type is non-nullable, then you should not use HasValue, as it will return true for an explicit null, even though the property is non-nullable (you also need to check that the value is non-null). Conversely, if the type is nullable, then you probably just want HasValue most of the time (though there may be some uses for HasNonNullValue).

[glen-84]

Rereading this ...

its not superior ... as it supports not all use-cases of Optional. This would break the use-cases of many use-cases I know of.

It can of course be a separate type, but I'm curious – do you have example use cases?

[aradalvand]

its not superior ... as it supports not all use-cases of Optional. This would break the use-cases of many use-cases I know of.

@michaelstaib I'd be curious to hear what those use cases are? Correct me if I'm wrong but I feel like you might've misunderstood the proposal; because this simply cannot be true. MaybeSet<T> is strictly a superset of Optional<T>, so, fundamentally, anything you're doing with the current Optional<T> you'll also be able to accomplish with MaybeSet<T>.

[ParadiseFallen]

also i want built in validation so if it is MaybeSet<string> it should throw validation error to graphql value optional but cant be nullable. only if i made MaybeSet<string?> it should accept also null.

also MaybeSet<string> will have nullable string? Value otherwise all who uses this feature forced for creating default values for that. it's easy whan it is scalar. but what with owned models? creating new value as default isn't good.

[ParadiseFallen]

also we could make it work like

record Inut 
{
  public MaybeSet<string>? OptionalNameButNotNullRequredForValidation {get;set;}
  public MaybeSet<string?>? OptionalNameAndCanBeSetToNull {get;set;}
}

[ParadiseFallen]

can we use custom directives to specify @NotNullWhenExplicitlySet in schema?

[glen-84]

I was discussing this with Michael the other day, and he brought up an important point – with the proposed solution, the schema indicates to the client that an optional/maybe-set field is nullable, but then immediately breaks that contract by throwing an error when it is. This is far from ideal.

One alternative would be to accept the null value, but not set it on the Optional/MaybeSet, so that HasValue/WasSet still returns false for non-nullable fields. It's slightly better, but still not perfect since the client may assume that the field's value will be set to null, when in reality nothing will actually happen.

[michaelstaib]

We are discussing a new PatchInputType<T> at the moment. The idea described here has to many issues we see, but we recognize the need for a higher level patch type.

I will describe it in more detail how we gonna tackle it with HotChocolate 14 in this issue in the next couple of days: https://github.com/ChilliCream/graphql-platform/issues/7097