-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] Client controlled nullability operator #867
Comments
I love this, and to elaborate a bit and try to apply some definitions and following thoughts: This is client control to transform a Nullable to a Non-Null, but I would like to see the opposite appear in this same RFC - converting a Non-Null to a Nullable. Terms for those:
Some thoughts on validation:
|
Maintainer of the Apollo GraphQL iOS library here. I am very much in favor of this proposal. As @twof stated, in Swift, dealing with the nullability of most fields becomes very cumbersome. Most developers end up explicitly (ie. unsafely) unwrapping fields all over the place or having to wrapping our generated models in their own view models, where they do nullability checks and validation. We've been mulling this idea around internally for a while now, but never got around to making a proposal. I'm glad someone else got around to making the proposal. @leebyron I'm not sold on the idea of converting Non-Null fields to Nullable. As you describe, it adds a lot of complexity and edge cases. But I have a hard time seeing compelling uses cases that would warrant providing the functionality. If you have example use cases, I'd love to hear them! |
Regarding making a Non-Nullable field nullable (e.g. via I imagine a use case for |
Since making a nullable field non-nullable is a non-breaking change, we must allow Strictly speaking the same situation cannot arise for |
(To be clear, I don't currently think |
Same motivations as marking a field as required with Excuse the poor data modeling for this entirely contrived example, but hopefully this makes the design intent clear. In this schema every hero has a craft (non-null), ...but not every craft is capable of a top speed (nullable) type Hero {
name: String
craft: StarShip!
}
type StarShip {
topSpeed: String
}
# Queries our Hero, but only includes its craft if it has a topSpeed. Includes the hero's name regardless.
fragment Test on Hero {
name
craft? {
topSpeed!
}
} You can see how in that example if our required
I imagine the exact same errors would exist in the errors response, it would just alter where the null-replacement behavior applies. An error below is of course captured in the errors response, and would TL;DR: No error swallowing, all existing errors continue to report, just
IMHO that's a secondary use case, but still a valid one. As I mentioned before, I think a primary use case is determining where to stop error bubbling in the case an ancestor field uses
Seems likely not. I think putting I think the validation rules around these are open questions which tradeoff between improved clarity of intent and back-compat resiliency. Though after thinking about this a bit, it seems weird if I don't think I'd advocate for introducing error boundary optional fields ( Obviously imperfect corollary, but IMHO introducing |
That example really clears things up; I see your intent now and it definitely seems to make sense 👍 (I didn’t think about it before because all my relations tend to be nullable anyway.) Interestingly this does add a reason to allow (without even a lint error) |
Could the non-nullability information somehow be made available as schema extensions (schema level) in addition to query fields (query level)? From my understanding, a lot of the nullability friction comes from:
For an example, the star wars API has nullable type PeopleEdge {
cursor: String!
node: Person
} This comes up super confusing to an app developer as there's not really much to be done if I think there would be some value in allowing codegen clients to augment the schema on their side and do some "impedance matching" with what the server exposes. This could be done using the current grammar with a directive and type extensions only known to the client: # client/schema-extensions.graphqls
extend type PeopleEdge @nonnull(fields: ["node"]) Or using new syntax elements and validation rules: # client/schema-extensions.graphqls
extend type PeopleEdge {
node: Person!
} In larger apps, this would allow to manage nullability in a central place without each developer having to opt-in the non-nullability manually. |
One more consideration: The currently proposed syntax does not have an obvious answer for the nullable list items. @martinbonnin's second suggested syntax (copied below) lets you mark the list items not nullable. But it would be too aggressive for components with different nullability requirements as suggested by comments like this: twof#1 (comment) # @martinbonnin's second suggested syntax
extend type PeopleEdge {
node: Person!
}
# a way to mark list items non-nullable
extend type PeopleConnection {
edges: [PeopleEdge!]!
} |
To solve the nullable list items, what do folks think about this syntax? To extend the example above: query {
peopleConnection {
node {
name
}
}! # Notice "!" here. "!" makes list items non-nullable.
}
|
In theory, you could have an arbitrary number of nested type Transformation {
# here everything is nullable
rotationMatrix: [[Int]]
# here everything is non-nullable
translationMatrix: [[Int!]!]!
} Adding a single |
Two major drawbacks:
I'm not entirely convinced we need to extend this concept down to items within Lists, but if we do, I'd suggest just allowing the modifier to be a sequence of modifiers: fragment T on Transformation {
# just requires that the matrix itself exist, inner item nullability still applies
a: rotationMatrix!
# requires that the matrix exist and items of the first list are non-null
b: rotationMatrix!!
# requires that the matrix, items, and nested items are non-null
c: rotationMatrix!!!
# requires that items and nested items are non-null, but catches that error at the field level (doesnt bubble the error)
d: rotationMatrix?!!
} |
I like Lee’s solution above, but worry that it may complicate us adding another wrapping type (in addition to List and NonNull) in future. No idea what such a wrapping type may be though, and we haven’t needed one yet… |
Hey there! Quick update for everyone. I got pulled into another project at work and haven't been able to focus on this, but I'm hoping to wrap it up in the next couple of weeks, and then I'll be able to focus on this full time. We've started on an implementation, and we're looking to make sure we've got all of our corner cases covered next. You can follow our progress here: aprilrd/graphql-js#5 We're currently implementing both the non-null operator (!) and its inverse (?). We haven't dealt with list items yet. |
The ["Nullability RFC" for GraphQL](graphql/graphql-wg#694) allows fields to individually be marked as optional or required in a query by the client-side. ([See Strawman Proposal](graphql/graphql-spec#867)) If a field is marked as optional then it's allowed to be missing and `null`, which can control where missing values cascade to: ```graphql query { me { name? } } ``` If a field is marked as required it may never be allowed to become `null` and must cascade if it otherwise would have been set to `null`: ```graphql query { me { name! } } ```
The ["Nullability RFC" for GraphQL](graphql/graphql-wg#694) allows fields to individually be marked as optional or required in a query by the client-side. ([See Strawman Proposal](graphql/graphql-spec#867)) If a field is marked as optional then it's allowed to be missing and `null`, which can control where missing values cascade to: ```graphql query { me { name? } } ``` If a field is marked as required it may never be allowed to become `null` and must cascade if it otherwise would have been set to `null`: ```graphql query { me { name! } } ``` In Graphcache, we imagine that the nullable field — which would be marked with `required: 'optional'` — can allow Graphcache to make more data nullable and hence partial, which enhances schema awareness, even if it's not actively used. The required fields — which would be marked with `required: 'required'` — would force Graphcache to include this data, regardless of what schema awareness may say, which also enhances partial data in the presence of schema awareness, since it increases what the cache may deliver. In other words, it guarantees a "forced outcome" in both cases, without having to look up whether a field is nullable in the schema. In the future, we may even derive the `RequiredStatus` of a `FieldNode` in an external place and never call `isFieldNullable` with a schema in the query traversal.
@twof Whats the update on this, there has been no discussion for years and I cannot find a reason the PR was closed? |
|
You can read a fuller version of the RFC here.
TL;DR:
This RFC proposes new syntax for GraphQL clients to express that fields in an operation are non-null.
Problem Statement
The official best practice for schema design is to make all fields nullable by default. If “null” is not an appropriate value for a failed field, the guidance recommends making it nonNull in the SDL.
The problem with the SDL nonNull (
!
) is that it eliminates the possibility of partial failure on a given type. This forces the schema 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 every possible use case.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.
🧑💻 Proposed Solution
Three new operators are introduced to the query language:
!
which marks a field Non-Nullable?
which marks a field Nullable[]
which allows the above two operators to be applied to the elements of a list, or the list as a whole.🎬 Behavior
The proposed query-side Non-Null designator and SDL-defined Non-Null would have slightly different behavior. When null is resolved for a SDL Non-Nullable field, the executor propagates until it finds the nearest Nullable parent, and sets that to null.
This proposal also introduces a second "optional designator",
?
, which designates that the field is Nullable, even if it is Non-Nullable in the schema. When null is resolved for a field marked with a required designator (!
) the executor instead propagates null to the nearest field marked with an optional designator (?
). If there is no optional designator to propagate to, then the response'sdata
field will become null ie the entire response will be lost. Because of that, in most cases we expect that required designators will be paired with optional designators to preserve partial responses.✏️ Proposed syntax -
!
The client can express that a schema field is required by using the
!
syntax in the query definition:We have chosen
!
because!
is already being used in the GraphQL spec to indicate that a field in the schema is non-nullable.?
chosen because other languages use it to indicate optionality.[]
was used because it's traditionally been used to indicate a list.If a field is a list, designators can either be applied directly to the field ie
twoDListField!
or to the elements of the list with list operators matching the number of dimensions of the list ietwoDListField[[!]]!
.✨ Use cases
Improve the developer experience using GraphQL client codegen types
Handling nullable values on the client is a major source of frustration for engineers, especially when using GraphQL codegen types in strongly-typed languages.
The proposed nonNull designator would allow GraphQL clients to generate codegen types with more precise nullability requirements for a particular feature. For example, using a GraphQL client like Apollo GraphQL on mobile, the following query
would codegen to the following type in Swift.
If a null business name is not acceptable for the feature executing this query, this generated type is more ergonomic to use since the developer does not need to unwrap the value each time it’s accessed.
The text was updated successfully, but these errors were encountered: