Query level nullability review and drafting thread

[twof]

Creating this PR so folks can talk about the RFC and suggest additions and changes. If you make any changes to the document, feel free to add yourself to the top of the doc!

Rendered RFC here

Want to contribute? Here are some things we need:

• Additional use cases. How would you use this feature?
• Additional information about alternatives. How are you working around this limitation of GraphQL
• Prior art. Have similar things been proposed in the past?
• Pros and cons of making schema fields non-nullable.

We want to get feedback from a broad range of people since the impact of this RFC is somewhat large. People we would like additional feedback from:

• Someone at an organization that is using a client other than Apollo
• People who work in frontend and backend web

[aprilrd]

Thank you for writing this up! I much prefer ! over @nonNull.

The current proposal does not describe how the server should behave when a ! marked field is null. Shouldn't that be part of the proposal?

[lizjakubowski]

great work so far compiling all of the information from various docs 👏 some nits, some more high level stuff -- I can definitely help rework this stuff!

[aprilrd]

just a thought, shouldn't the proposal be named Operation Nonnullability or something similar? The operator actually adds non-nullability and it should function in any operation type, not just a query.

[twof]

@wxue Do you remember why we didn't want this?

[lizjakubowski]

I think we should remove this as an alternative, I don't think we understand it well enough to vouch for it

[twof]

TODO: Flesh this out with things about versioning, breaking changes, and the impact it has on partial results

[captbaritone]

Hey! We shipped something like this in Relay last half. We haven’t yet documented it in open source, but it’s in wide use inside Facebook. I’ll share a description of what we built, and our observations below.

API

Our approach was to include a custom directive, @required(action: LOG | NONE | THROW) where actions have the following behavior:

• THROW: Relay throws an error when this field is read. Relay supports per-component fragments, so we are able to throw when the component which requires the field actually renders. Combined with React’s Error Boundaries, we are able to contain that error to a subtree of the UI. For example, if a required field is missing in a single item in a list, we can just replace that individual item with an error UI, or perhaps choose to simply omit that single list item.
• LOG: Is intended for a case where the client code can recover from the missing data, but the recovery UI is not expected to be needed. When this is encountered, Relay “bubbles” the nullness up to the parent node (so if user.name is null, you would get a null user) and logs the fact that it encountered a null value back to the server for alerting purposes.
• NONE: This is intended for cases where the child field is required in order for the parent object to “make sense” but the client can render a sensible UI even without the parent object. It simply bubbles the nullness to the parent node.

In Relay these the custom @required directive is compiled away and replaced with additional metadata in our runtime artifacts which tell us how to read the server response.

Observations

• The “required-ness” of a field is not just a client concern but, ideally, a view/component level concern
  • You may have a single query which provides data for a large complex view. In one subtree of that UI the field may be required, but in a separate subtree it may not.
  • It’s possible that you fetch a large query with a field that is required by one view, but that view never ends up rendering.
  • Even if the view/component that requires the field renders, it may be possible to contain the error state to just that sub-view.
• A single field may be referenced in multiple places in the same fragment
  • With inline fragments it’s possible to reference the same field in multiple places. If one instance of the field is required and the other is not, the expected behavior can be ambiguous.
  • In the Relay compiler we make it a compiler error to have multiple references to the same field with different @requried status.
• Inline fragments lead to some odd edge cases
  • What does it mean for a field to be required within an inline fragment within a type condition? If the type condition matches, and the field is missing, we error?
  • This may lead to a case where your field may never be null, but it could still be missing. The client codegen would need to account for this.
  • In Relay we made it a compiler error to use @required in an inline fragment’s selection. We have not had any pushback/confusion on this choice internally.
• This feature is popular
  • As you’ve called out in your RFC, handling null values in the client is a major source of frustration for engineers and after introducing the new directive we saw explosive adoption.
  • The THROW variant (which is most similar to your proposal) is far and away the most popular action, accounting for ~80% of instances.
  • However, while handling null values is a pain for developers, erroring results in a bad experience for users. This makes explosive adoptions dangerous. We attempted to mitigate this by minimizing the impact of errors (scoping them to a UI subtree) and ensuring thrown errors were correctly logged/alerted and attributed back to the query/fragment which marked them as required.

[aprilrd]

Thank you so much for your attention and participation. I am excited to hear that Facebook has run into the same issue and implemented a solution that works at scale. I have a couple of questions to understand your comment better:

• You described how Relay uses the directive. Does your internal API change its behavior based on the @required directive? If not, could you explain why?
• A single field may be referenced in multiple places in the same fragment. I see the ambiguity if a fragment aliases fields without arguments. But there is no ambiguity for fields with arguments. Did I understand correctly?
• I fail to understand the issue with an inline fragment with a type condition. Given this schema and this query, I believe a codegen tool can correctly generate the returned type. Am I missing anything?

type Cat {
  name: String
}

type Fish {
  weight: String
}

union AnimalResult = Cat | Fish

type Query {
  animal: AnimalResult
}

query getAnimal {
  animal {
    ... on Cat {
      name @required(action: THROW)
    }

    ... on Fish {
      weight
    }
  }
}

type GetAnimalQuery = { __typename: 'Cat', name: string } | { __typename: 'Fish', weight: string | null; } | null

[kassens]

Does your internal API change its behavior based on the @required directive? If not, could you explain why?

No. There might be a potential small win in terms of transfer size or server resources by not computing some other fields, but this hasn't stood out as a large enough potential win yet.

issue with an inline fragment with a type condition

The type generation gets more weird when union or interface types are involved: ... on Mammal { name }. Relay forces name to be an optional key in this case even if name is a String! in the schema.

[josephsavona]

You described how Relay uses the directive. Does your internal API change its behavior based on the @required directive? If not, could you explain why?

No, our implementation is currently purely on the client, and Relay Compiler removes the @required directive before persisting queries to the server. We have considered implementing support for this directive on the server but have not done so yet. In general we imagine this as an optimization: if the server can determine that one fragment cannot be fulfilled due to a missing required field, the server could avoid further processing of that specific fragment - but other fragments may need overlapping data and should continue evaluation if they are unaffected by the missing required data.

One consideration here is that servers generally flatten away fragments during execution, but preserving the semantics of our @required implementation requires treating fragments as boundary points. Ie, a missing required field may bubble-up a null to the nearest nullable parent field or nearest fragment boundary, whichever is nearest.

A single field may be referenced in multiple places in the same fragment. I see the ambiguity if a fragment aliases fields without arguments. But there is no ambiguity for fields with arguments. Did I understand correctly?

Fields can have ambiguous required-ness regardless of the presence of arguments. Consider:

# BAD: this case is ambiguous, though there's a plausible reason you may want to do this:
fragment Preview on Media {
  # possible intent: for general media the thumbnail is optional
  thumbnail(size: 32)

  ... on Photo {
    # but if its a photo we require a thumbnail to exist
    thumbnail(size: 32) @required(action: NONE)
  }
}

Note that in this case the problem is (like in the previous point) the fact that (inline) fragments are spread into parents. If the results of ... on Photo { thumbnail(...) } appeared under a different key in the response - e.g. media.onPhoto.thumbnail - then the above example would not be ambiguous.

However, the following is clearly ambiguous and should be considered invalid:

# BAD: this can reasonably be considered invalid code
fragment Preview on Media {
  thumbnail(size: 32)
  thumbnail(size: 32) @required(action: NONE)
}

I fail to understand the issue with an inline fragment with a type condition. Given this schema and this query, I believe a codegen tool can correctly generate the returned type. Am I missing anything?

The challenge is inline fragments on abstract types - because the results of inline fragments are flattened into the outer result object, developers have to check for the presence of keys to see whether the object matched the abstract type. Consider a variation of your example:

type Cat implements Animal, Node {
  id: ID!
  name: String
}

# ... other types implementing Animal

interface Animal extends Node {
  species: String
}
interface Node {
  id: ID!
}

type Query {
  node: Node
}

query getAnimal {
  node {
    ... on Animal {
      species @required
    }
  }
}
type GetAnimalQuery = 
  # node exists, is an Animal, has species: everything is non-nullable
  {
    node: {
      species: string, 
    }
  }
  # or node doesn't exist
  # or node exists and is an Animal, but species was null so the null bubbled-up and nulled out the node
  | {
    node: null
  }
  # or the node exists but is not an Animal, nothing to select
  | { }
  # or node had an error, which nulled out the parent
  | null

Here is is possible to define types that represent the possible outcomes, but the possible combinations grow quickly as users add more required fields within more different type conditions (inline fragments).

[josephsavona]

Can you expand here? My reading of this is that if the value of a field with ! is null at runtime, then the GraphQL executor must treat that identically to the case that the field resolver raised an exception, including returning an entry in the top-level errors list and nulling out the parent field (which may need to then bubble up an error if that field was non-nullable in the schema, or also had ! applied).

This would mean that in a query such as the following, other_media_field being null at runtime would cause the entire query to return effectively no data (just {data: {media: null}, errors: [...]}). This would prevent any part of the UI from rendering because a single field was missing for one part:

query MediaQuery {
  media(id: "...") {
     name
     thumbnail
     ...MediaFragment
  }
}
fragment MediaFragment on Media {
  other_media_field!
}

How are you thinking about handling cases such as this?

[kassens]

I read this as any field in the response that would be null and has a ! on the field in the operation would cause the whole operation to return an error and no data. Even if that field is in some named fragment.

I think this would limit the usefulness extremely, basically to very simple queries. To take Yelp as an example, I don't think it'd be a good idea to error a search query if a single search result item failed to generate a star rating (assuming that's required to render a search result item).

It would be good to clarify what the behavior is if the ! is in a named fragment.

[fotoetienne]

At Netflix, we have implemented this on the server (using a directive syntax) and defined it to have identical semantics as the existing SDL Non-Null behavior. This has worked well.

I've proposed a clarification to specify this.

[josephsavona]

@fotoetienne Thanks for adding the clarification, that confirms my interpretation and brings us back to my original question:

How are you thinking about the fact that a single fragment with a ! field can cause data for other fragments (or in the extreme, an entire request) to be nulled out? As @captbaritone mentioned above, one of our goals / requirements when designing @required was that a "missing" required value for one part of a UI cannot cause data for other parts of the UI to be nulled out. The design here violates that constraint (it breaks fragment modularity).

[fotoetienne]

@josephsavona That's a great question. Our current server logic will make a field act as Non-Null is there any Non-Null designation for that field in the query. This query example:

fragment Preview on Media {
  thumbnail(size: 32)

  ... on Photo {
    thumbnail(size: 32) @nonNull
  }
}

would essentially be interpreted by the server as:

fragment Preview on Media {
  thumbnail(size: 32) @nonNull

  ... on Photo {
    thumbnail(size: 32) @nonNull
  }
}

It is at least non-ambiguous, but I can see how it is a challenge for fragment modularity.

[josephsavona]

@fotoetienne Thanks for clarifying. That seems like a severe limitation that would make applications significantly less robust than they can and arguably should be. How do you manage this in practice? Do you encourage fields with ! to be given unique aliases so that you prevent removing data for other parts of a UI?

[fotoetienne]

Short answer - it hasn't come up for us yet, but it's an important edge-case to consider.

Long answer - Fragment modularity is particularly crucial for clients that rely heavily on components tied to individually defined Fragments that are compiled into large queries. We don't currently have many teams using that approach.

I agree that this is an important edge-case that we should think through some more. We want a design that will lead UI developers to fall into "the pit of success" rather than allowing surprising behavior like you've described.

[fotoetienne]

An initial thought, probably flawed: reversing the server-defined logic might help. Only honor nonNull if all references to that field specify it. This would, however, require client codegen to be aware of an entire query and only make types nonNull if all fragments in the query specify nonNull. Thoughts?

[fotoetienne]

Making conflicting definitions between fragments be a validation error (like you did with @requires) is the safest approach. I like that. It's probably an acceptable limitation that you just can't have differing nullability for the same field.

[kassens]

@joesavona even unique aliases would still null out the object by bubbling up to the parent. It's really the same as any specific field being non-nullable in the server schema and ending up null which bubbles up.

[fotoetienne]

Great work! Made a couple revisions/suggestions

[fotoetienne]

Defaults can be quite useful from a resilience standpoint. It is relevant to this proposal in that the ! syntax would't be as amenable to specifying a default, whereas a directive (e.g. @nonNull(default: false)) would open us up to that.

[benjie]

Though replacing ! with ! = “default” like in variable defaults could work. This would either have to be evaluated client side (like in Relay) or the same default would have to apply to all references at the same location (whether from fragments or directly).

[fotoetienne]

I would consider reducing the focus on specific client languages.

The crux of the problem is that the SDL nonNull (!) eliminates the possibility of partial failure on a given type. This forces the SDL author to decide for which fields partial failure is acceptable. A GraphQL schema author may not be in the best position to decide whether partial failure is acceptable for a given canvas. Additionally, the answer to whether a missing value is acceptable may vary between UI canvases. Specifying nonNull (aka. required) in the query gives the UI developer the power to decide where partial failure is acceptable, and the flexibility for different canvases to have different requirements in this regard.

The pains are more noticeable in languages with first-class null safety, but are relevant to any client.

[mjmahone]

This will be true even with custom syntax. For composability of fragments, it would be worrisome if this new syntax made it impossible to have these two fragments in the same application:

fragment UserOkWithoutName on User {
  name
}
fragment UserMustHaveName on User {
  name!
}

With consistency, I could end up re-rendering a component built with UserMustHaveName based on a response fetched using only UserOkWithoutName.

And in fact it gets worse:

fragment BestFriendWithoutName on User {
  best_friend {
    id
  }
}
fragment BestFriendWithName on User {
  best_friend {
    id
    name!
  }
}

If my best friend, when I fetched BestFriendWithName, is User:1, and then suddenly, after fetching BestFriendWithoutName is User:2, I no longer am capable of rendering the component relying on BestFriendWithName (because name is unset on User:2). If the client library doesn't know what to do in that case, you can end up with consistency updates causing crashes.

[aprilrd]

As you said, the client library will need to understand this new syntax/directive. I am not familiar with GraphQL client implementations, so I am not sure how difficult my imagined behavior will be to implement. Doesn't this behavior solve the issue you outlined?

If a GraphQL client finds that a component relies on BestFriendWithName , but the cached data doesn't have name, it should not return any of the cached data. To the client, it is like a state before fetching started. Depending on fetch-policy (to use Apollo's terminology), the client can start a new request to fetch the missing field data.

[mjmahone]

Doesn't this behavior solve the issue you outlined?

That's how Apollo solves it, but that strategy isn't true universally. For instance that's not how Relay solves it. The client I work on can't solve it this way, as our consistency subscriptions are at the response level, for queries that include hundreds of fragments (whether that's a good choice or not is irrelevant: it's how things work for the largest GraphQL client at Facebook): if missing name meant the entire consistency update should not be served, whenever a product chose to mark their field with !, they could cause entire unrelated surfaces to lose consistency.

Another valid strategy would also be to return the data, but with best_friend being null: all the other data in the fragment/consistency subscription would still be delivered (this is Relay's strategy for @required(action: NONE)). Or requiring the product to null check every piece of data, always, regardless of its nullability annotation.

[aprilrd]

Do you mean that Relay requires field selections on the same id and type to behave the same globally? If so, I can see how my suggested behavior won't work. If not:

Another valid strategy would also be to return the data, but with best_friend being null: all the other data in the fragment/consistency subscription would still be delivered (this is Relay's strategy for @required(action: NONE))

This is what I imagined. The client would nullify up until the most immediate nullable parent.

[cometkim]

Here's one of the cases where this would be needed.

GatsbyJS projects use GraphQL to handle build-time data dependencies, and provide a file system route API that can automatically build pages from specific types.

Creating a file called src/pages/{BlogPost.slug}.jsx creates pages from all BlogPost nodes and injects page context. { id: string, slug: string }

user can use the page context as an argument to fetch additional data.

# $id is automatically injected by Gatsby
query BlogPostQuery($id: String!) {
  # The type of blogPost is nullable, but the result is guaranteed to non-null by Gatsby
  blogPost(id: { eq: $id }) {
    title
    tags
  }
}

This can be done with additional static analysis in plugin layer. But the proposal can be a simple solution.

Even if It's not clear in production cases that can customize schema, but it seems a huge advantage for ecosystem players using SDL and code-generation.

[aprilrd]

Thank you for sharing another use case!

[josephsavona]

One general thought is that clients generally use GraphQL to fulfill at least two purposes in an application.

• To describe the local data dependencies of an individual unit of UI in order read that data from the local cache (and subscribe for updates).
• To describe the aggregated data dependencies of an entire user interface in order to fetch those dependencies from the server.

An example of the distinction is pagination:

• The user interface wants to read (from the local cache) all the items that have been fetched so far.
• The query sent to the server should always be for a page at a time: applications generally don't want to keep refetching from the beginning of the list (1-10, then 1-20, etc) but rather fetch pages (1-10, then 11-20, etc).

The same distinction is relevant when considering the "required-ness" of a field: Individual components want to describe their data requirements - if a given component must have user.name, then it should be easy to express that dependency. However, when data dependencies of multiple components are aggregated that should occur in a way that allows fetching data to fulfill as much of the user interface as possible.

This proposal makes it difficult to use the same GraphQL to describe an individual component's data dependencies and to aggregate the dependencies for an entire user interface. Instead, the likely outcome of this proposal is that "sophisticated" clients (ie those that use AOT query compilation and also that use a client-side cache as a source of truth) would use ! as both a user-facing feature and as a compiler target. In other words:

• When reading data for a component from the cache, ! would allow a component to express that a field is required and have the cache return "null" for an entire fragment (or part of one) if a required field was missing in the cache.
• When sending queries to the server, the client would generally remove ! unless it could prove that a field was always used with !, or more generally that the inclusion of ! on a field would not break fragment composition.

Considering this last point, it is likely that under this proposal many instances of ! fields would likely end up pruned by client compilers, losing information about potential optimizations that could be applied. A canonical failure case is below (adapted from @mjmahone's comment above):

query UserSummary {
  me {
    ...Header
    ...Timeline
   }
 }
 
fragment Header on User {
  name!
  profile_picture
  ...SomeExpensiveFragment
}

fragment Timeline on User {
  name
  posts(first: 10)! { 
    title!  # exclude posts that don't have a title
  }
}

fragment SomeExpensiveFragment on User { ... }

Imagine that the user interface is capable of rendering Header and Timeline independently: even if data for one is missing the application should render the section whose data is available. If me.name is null at runtime, then we would ideally do the following:

• Continue to resolve all the selections of Timeline: its data dependencies can still be fulfilled w a null name
• Avoid (or cancel) loading SomeExpensiveFragment, since we know that Header cannot be fulfilled.

In practice, a client compiler would have to transform the query as follows:

...snip...

fragment Header on User {
  name # <--- remove `!` here because keeping it could cause `Timeline` to fail
  ...snip...
}
fragment Timeline on User {
  ...snip...
  posts(first: 10) {  # <-- remove `!` here because keeping it could `Header` to fail
    ...snip...
  }
}
...snip...

In other words: client compilers would have to remove both instances of ! in the version of the query that they send to the server. At the same time, there is no way for the client to describe its real intent, which is that if Header.name cannot be fulfilled, the remaining selections of Header can be skipped, and similarly if Timeline.posts is null, that the other selections of Timeline can be skipped. I'd encourage thinking about ways to retain the ability to declare "required-ness" of a field but potentially change the semantics to accommodate these cases.

[twof]

Thanks everyone for your comments! Earlier today the champions of this proposal attended a GraphQL Working Group meeting and officially made it to the first RFC stage. You can read the notes from the meeting here, and I'll update again when the recording of the presentation is online.

We'll be moving this over to a PR in the main repo, and I'll try so summarize the existing comments as best I can. We definitely still want to address them.