graphql / graphql-spec

GraphQL is a query language and execution engine tied to any backend service.
https://spec.graphql.org
14.31k stars 1.13k forks source link

RFC: SemanticNonNull type (null only on error) #1065

Open benjie opened 1 year ago

benjie commented 1 year ago

TL;DR: Introduces a new type wrapper, Semantic-Non-Null, which represents that a value will not be null unless an error happens, and if an error does happen then this null does not bubble.

The problem

GraphQL schema designers must use non-nullable types sparingly because if a non-nullable type were to raise an error then the entire selection set it is within will be destroyed, leading to clients receiving less usable data and making writing the results to a normalized cache a dangerous action. Because of this, nullable-by-default is a best practice in GraphQL, and non-null type wrappers should only be used for fields that the schema designer is confident will never raise an error - not just in the current schema, but in all future schemas.

Many GraphQL consumers choose to ignore the entire response from the server when any error happens, one reason for this is because the null bubbling behavior makes writing to normalized caches dangerous. For these users, when an error doesn't happen, the nullable fields they are dealing with can be frustrating because their type generation requires them to handle the null case even if it may never happen in practice, which can lead to a lot of unnecessary code that will never execute. There is currently no way for the type generators to know that a field will never be null unless there's an associated error.

The solution

We can categorise that there are effectively two types of null:

This PR introduces a new wrapper type in addition to List and Non-Null, called Semantic-Non-Null. The Semantic-Non-Null type indicates that the field will never be a semantic null - it will not be null in the normal course of business, but can be null only if accompanied by an error in the errors list (i.e. an "error null"). Thus a client that throws out all responses with errors will never see a null in this position. Also, critically, any null raised by this field will not bubble and thus if an error is found with the exact path to this null then it is safe to store the result (including the error) into a normalized cache.

In SDL the Semantic-Non-Null wrapper is currently represented by a ! prefix (as opposed to the ! suffix for a strict Non-Null).

Thus we have the following:

# Type description Syntax Result values
1 Unadorned String String string, or error null, or semantic null
2 Semantic-Non-Null String !String string, or error null
3 (Strict-)Non-Null String String! string

Note that 1 and 3 above are exactly the same as in the current GraphQL specification, this PR introduces 2 which sits in the middle.

Backwards compatibility

All existing schemas are automatically supported because the meaning of String and String! is unchanged.

To ensure that all existing clients are automatically supported, this PR introduces the includeSemanticNonNull argument on __Field.type which defaults to false. Clients that do not pass includeSemanticNonNull: true will see all Semantic-Non-Null types stripped, which will have the effect of making them appear as if they were the unadorned types. This is safe, since it means these clients will need to handle both error nulls and semantic nulls (as they traditionally would have) even though we know that a semantic null will never happen in practice.

All existing GraphQL documentation, tutorials, examples, and everything else we've built over the last 8 years remains valid since the meaning of String and String! are unchanged.

History

This PR is almost identical to #1048, but it changes the name of the new type wrapper from Null-Only-On-Error to Semantic-Non-Null, and changes the syntax from String* to !String. It addresses the True Nullability Schema discussion raised by @captbaritone and incorporates/adapts some of the terminology from @leebyron's Strict Semantic Nullability proposal.

netlify[bot] commented 1 year ago

Deploy Preview for graphql-spec-draft ready!

Name Link
Latest commit bd038f24720b0bf9dc7d9e2eb3edf4f4498a4759
Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/6595e04ffa373d0008cbff71
Deploy Preview https://deploy-preview-1065--graphql-spec-draft.netlify.app/draft
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

captbaritone commented 9 months ago

One outstanding question regarding this approach is "will we be able to find an additive syntax to denote 'null only on error' that is palatable?".

Some folks from the Nullability Working Group met in an ad-hoc meeting to brainstorm the options available and itentify viable syntax candidates. (Meeting notes can be found here under the heading "Ad-hoc discussion on syntax")

We arrived at a top three which seemed to pull away from the rest as clear leaders. ~! is the favorite of the group, but all three are worth sharing with a larger group. All three have a basis in written English in which the additive symbol qualifies the ! in some way.

The three leaders, including the properties that make them more or less desirable:

bignimbus commented 9 months ago

These are all really thoughtful. The only thing I have to add is that !^ reminds me of bitwise XOR, which feels conceptually adjacent to semantic non-null in a way that I can't fully express on a Friday afternoon.

michaelstaib commented 9 months ago

Also "Bang carrot" sounds kind of great :D

glen-84 commented 9 months ago

friends: [User~!]~!

You won't make any friends with syntax like that. 🙃

Adding a multi-character affix would be a big mistake, in my opinion.

With @leebyron's proposal, this would simply be friends: [User], right? How can that not be the goal?

benjie commented 9 months ago

With Lee's proposal, this would simply be friends: [User], right?

Along with schema @strictNullablility, yes.

How can that not be the goal?

There's a number of reasons why using the unadorned type to represent a semantically non-null type can not be the goal; here's a few:

  1. The unadorned type currently means nullable. Changing its meaning (even if that's correlated with a directive added to the schema) is a HUGE shift:
    • Every single SDL file that exists currently may have it's meaning changed if somewhere in the pipeline the schema gains a @strictNullability directive
      • When people add @strictNullability to their schema declaration they would need to change every single Foo type in their SDL to be Foo? in order to be safe; this is a huge shift and something very likely to have fields missed due to merge conflicts/etc as a codebase continues to evolve whilst this transition is going on
      • If they miss this transition in one place, that place is now forever semantically non-nullable because transitioning back to "nullable" is a breaking change.
    • Every piece of documentation that the community has written in the last 9 years which says the unadorned type is nullable will need to be revised (a frankly impossible task)
  2. The default behavior should be the most forward-compatible behavior.
    • Moving from nullable to non-nullable for output types is non-breaking, so starting with User (nullable) and later changing to User! (non-nullable) is an easy and safe transition.
    • Moving from non-nullable to semantically-non-nullable is not breaking. Moving from semantically-non-nullable to strict non-nullable is not breaking.
    • With Lee's proposal, the unadorned type now means "semantically non-nullable" (but only if the @strictNullability directive is present). If you realise that this is in fact a semantically nullable field, you need to change the type to User? which is a breaking change.
    • We should make it as easy as possible for people to write and maintain schemas in a versionless way, and defaulting to types that can be iterated is safer and, I'd argue, better for the ecosystem.
    • In this proposal (#1065) the unadorned type is the most forwards-compatible type - you can move to semantically non-nullable or strict-non-nullable in a non-breaking way
  3. Having User mean two almost opposite things, "nullable" and "semantically non-nullable", depending on the presence of a directive which could be in a completely different folder of the project seems hugely confusing and undesirable.
    • In my opinion, this will increase the complexity of GraphQL and make it much harder for people to learn, reducing adoption

My personal opinion is that Type and Type! should continue to mean what they have always meant, and the new type we're introducing, the semantically-non-nullable type, should get a different syntax. So far, Type~! is the preferred syntax for this, but if you have a better suggestion please post it here.

glen-84 commented 9 months ago

The unadorned type currently means nullable.

And that's a mistake IMO.

Changing its meaning (even if that's correlated with a directive added to the schema) is a HUGE shift

... in the right direction.

When people add @strictNullability to their schema declaration they would need to change every single Foo type in their SDL to be Foo? in order to be safe; this is a huge shift and something very likely to have fields missed due to merge conflicts/etc as a codebase continues to evolve whilst this transition is going on

This is covered in Lee's proposal ("How to adopt this incrementally?"). You don't enable @strictNullability until you're done adding the semantically-nullable modifier (?).

If they miss this transition in one place, that place is now forever semantically non-nullable because transitioning back to "nullable" is a breaking change.

Then they best be careful. 🙂

Every piece of documentation that the community has written in the last 9 years which says the unadorned type is nullable will need to be revised (a frankly impossible task)

So nothing can ever be improved because people have written about GraphQL before? The language will become progressively worse while trying to achieve infinite BC. Is versioning really an impossibility here?

Moving from nullable to non-nullable for output types is non-breaking, so starting with User (nullable) and later changing to User! (non-nullable) is an easy and safe transition.

It means a client can continue to use optional chaining, sure, but how does the API suddenly start returning data that doesn't exist, if the field was previously nullable?

If you realise that this is in fact a semantically nullable field, you need to change the type to User? which is a breaking change.

This is a migration concern. It's an issue once.

We should make it as easy as possible for people to write and maintain schemas in a versionless way, and defaulting to types that can be iterated is safer and, I'd argue, better for the ecosystem.

Part of the reason for these proposals is clients having to write code like a?.b?.c?.d – if nullable is the default, that just means more of this?

In this proposal (RFC: SemanticNonNull type (null only on error) #1065) the unadorned type is the most forwards-compatible type - you can move to semantically non-nullable or strict-non-nullable in a non-breaking way

Again, I'm curious how this works in practice, and more importantly, how often it happens. What data do you return for a field that has been changed to non-nullable? A confusing default like example@example.com? An empty string?

Having User mean two almost opposite things, "nullable" and "semantically non-nullable", depending on the presence of a directive which could be in a completely different folder of the project seems hugely confusing and undesirable.

You're thinking of a scenario where the two coexist. You do the migration once, and align your mental model with many other languages, then you forget about the inconsistent syntax that GraphQL currently has (in relation to these languages and common conventions).

Also, the behaviour might be toggled via a setting, and not necessarily via a schema directive hidden deep inside your project.

In my opinion, this will increase the complexity of GraphQL and make it much harder for people to learn, reducing adoption

To the contrary, something like friends: [User~!]~! (aka ASCII art) is more likely to alienate developers. If anything, aligning the syntax with common language conventions (which are far cleaner and more intuitive) is more likely to do the opposite.

My personal opinion is that Type and Type! should continue to mean what they have always meant, and the new type we're introducing, the semantically-non-nullable type, should get a different syntax.

This is adding more cognitive overhead – instead of just picking between nullable and non-nullable, we have to consider "semantically non-nullable" as well. How will that be less confusing?

So far, Type~! is the preferred syntax for this, but if you have a better suggestion please post it here.

Just Don't Do It™

It will make by-hand schema authoring a confusing mess. Multi-character affixes are awful. There exists a convention used commonly – adopt that, and align GraphQL with a common way of thinking. Accept the short-term pain for longer-term simplicity.

If anything, involve the community – poll developers on:

tobias-tengler commented 9 months ago

@benjie I see your point regarding backwards compatibility and already produced GraphQL content. I have to agree with @glen-84 though. If we make changes to this, it should make things easier long-term.

? makes sense to every developer and having the shortest syntax for the most common case (null-only-on-error) is also nice.

type User {
  name: String
  communityName: String?
}

The above is easily understood without knowing the historic context.

type User {
  id: ID!
  name: String~!
  communityName: String
}

This is not.

The introduction of the String? and the changed meaning of String wouldn't break existing consumers, since null-on-error is already handled by the type being nullable previously. The only breaking change would be String! to String, but this would also be the case with String! to String~!.

Similarly to how String~! would return String for old introspection queries, String? could also return String.

Besides the point that learning material would be outdated, I don't see a real reason why ~! should be preferred over ?. The outdated info that String means nullable wouldn't cause any harm, because even if you write queries with this assumption, you're handling the null-on-error case.

For me this would be an okay trade-off for an easier understood nullable syntax and shorter syntax for the common case. Contrary to you I think the new syntax would make adoption easier. For existing projects it's an optional, one time schema migration and for new projects you get an easier understood syntax out-of-the-box.