-
Notifications
You must be signed in to change notification settings - Fork 1.1k
RFC: OneOf Input Objects #825
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
base: main
Are you sure you want to change the base?
Changes from 25 commits
c385058
f6bd659
39e593c
b6741c3
d17d5ec
dca3826
7e02f5a
4111476
6754e0a
7c4c1a2
bb225f7
05fde06
e8f6145
08abf49
59cb12d
c470afb
99aa5d9
691087d
7109dbc
05ab541
6a6be52
de87d2f
57e2388
5a966f2
e78d2b5
c6cd857
d106233
87d0b22
d88d62a
a810aef
a1563a9
b45c0e4
0c9830e
c4d0b50
340594e
dbccf84
17f1304
d15f7bb
6310475
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,5 @@ words: | |
- tatooine | ||
- zuck | ||
- zuckerberg | ||
- brontie | ||
- oneOf |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1566,6 +1566,28 @@ define arguments or contain references to interfaces and unions, neither of | |
which is appropriate for use as an input argument. For this reason, input | ||
objects have a separate type in the system. | ||
|
||
**OneOf Input Objects** | ||
|
||
OneOf Input Objects are a special variant of Input Objects where the type system | ||
asserts that exactly one of the fields must be set and non-null, all others | ||
being omitted. This is useful for representing situations where an input may be | ||
one of many different options. | ||
|
||
When using the type system definition language, the `@oneOf` directive is used | ||
to indicate that an Input Object is a OneOf Input Object (and thus requires | ||
exactly one of its field be provided): | ||
|
||
```graphql | ||
input UserUniqueCondition @oneOf { | ||
id: ID | ||
username: String | ||
organizationAndEmail: OrganizationAndEmailInput | ||
} | ||
``` | ||
|
||
In schema introspection, the `__Type.isOneOf` field will return {true} for OneOf | ||
Input Objects, and {false} for all other Input Objects. | ||
|
||
**Circular References** | ||
|
||
Input Objects are allowed to reference other Input Objects as field types. A | ||
|
@@ -1659,6 +1681,21 @@ is constructed with the following rules: | |
does not provide a default value, the input object field definition's default | ||
value should be used. | ||
|
||
Further, if the input object is a OneOf Input Object, the following additional | ||
rules apply: | ||
|
||
- If the input object literal or unordered map does not contain exactly one | ||
entry an error must be thrown. | ||
|
||
- Within the input object literal or unordered map, if the single entry is | ||
{null} an error must be thrown. | ||
|
||
- If the coerced unordered map does not contain exactly one entry an error must | ||
be thrown. | ||
|
||
- If the value of the single entry in the coerced unordered map is {null} an | ||
error must be thrown. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between the first two conditions and the second two conditions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first two apply before coercion, the second two apply after coercion. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #825 (comment) discussion there still seems somewhat open, pending input from @leebyron |
||
|
||
Following are examples of input coercion for an input object type with a | ||
`String` field `a` and a required (non-null) `Int!` field `b`: | ||
|
||
|
@@ -1688,6 +1725,38 @@ input ExampleInputObject { | |
| `{ b: $var }` | `{ var: null }` | Error: {b} must be non-null. | | ||
| `{ b: 123, c: "xyz" }` | `{}` | Error: Unexpected field {c} | | ||
|
||
Following are examples of input coercion for a oneOf input object type with a | ||
`String` member field `a` and an `Int` member field `b`: | ||
|
||
```graphql example | ||
input ExampleInputTagged @oneOf { | ||
a: String | ||
b: Int | ||
} | ||
``` | ||
|
||
| Literal Value | Variables | Coerced Value | | ||
| ------------------------ | ----------------------- | --------------------------------------------------- | | ||
| `{ a: "abc", b: 123 }` | `{}` | Error: Exactly one key must be specified | | ||
| `{ a: null, b: 123 }` | `{}` | Error: Exactly one key must be specified | | ||
| `{ a: null }` | `{}` | Error: Value for member field {a} must be non-null | | ||
| `{ b: 123 }` | `{}` | `{ b: 123 }` | | ||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `{ a: $var, b: 123 }` | `{ var: null }` | Error: Exactly one key must be specified | | ||
| `{ a: $var, b: 123 }` | `{}` | Error: Exactly one key must be specified | | ||
| `{ b: $var }` | `{ var: 123 }` | `{ b: 123 }` | | ||
| `$var` | `{ var: { b: 123 } }` | `{ b: 123 }` | | ||
| `"abc123"` | `{}` | Error: Incorrect value | | ||
| `$var` | `{ var: "abc123" } }` | Error: Incorrect value | | ||
| `{ a: "abc", b: "123" }` | `{}` | Error: Exactly one key must be specified | | ||
| `{ b: "123" }` | `{}` | Error: Incorrect value for member field {b} | | ||
| `{ a: "abc" }` | `{}` | `{ a: "abc" }` | | ||
| `{ b: $var }` | `{}` | Error: Value for member field {b} must be specified | | ||
| `$var` | `{ var: { a: "abc" } }` | `{ a: "abc" }` | | ||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| `{ a: "abc", b: null }` | `{}` | Error: Exactly one key must be specified | | ||
| `{ b: $var }` | `{ var: null }` | Error: Value for member field {b} must be non-null | | ||
| `{ b: 123, c: "xyz" }` | `{}` | Error: Unexpected field {c} | | ||
| `{ a: $a, b: $b }` | `{ a: "abc" }` | Error: Exactly one key must be specified | | ||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
**Type Validation** | ||
|
||
1. An Input Object type must define one or more input fields. | ||
michaelstaib marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
@@ -1700,6 +1769,9 @@ input ExampleInputObject { | |
returns {true}. | ||
4. If input field type is Non-Null and a default value is not defined: | ||
- The `@deprecated` directive must not be applied to this input field. | ||
5. If the Input Object is a OneOf Input Object then: | ||
1. The type of the input field must be nullable. | ||
2. The input field must not have a default value. | ||
3. If an Input Object references itself either directly or through referenced | ||
Input Objects, at least one of the fields in the chain of references must be | ||
either a nullable or a List type. | ||
|
@@ -1727,6 +1799,12 @@ defined. | |
the original Input Object. | ||
4. Any non-repeatable directives provided must not already apply to the original | ||
Input Object type. | ||
5. The `@oneOf` directive must not be provided by an Input Object type | ||
extension. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Why is this the case? extend is useful for tooling to "create types" in a peicemeal fashion. Not everyone needs to know all the details but it needs to be validated at the end at actual schema creation time
and
The following once combined is valid surely? if we had an invalid combination say
and
then this can be validated at the time the actual runtime schema object is created. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Twofold: it’s because a ”OneOf Input Object” may be constructed in a different way to traditional input objects (might use a different class or similar), and because if this was not the case then when the directive was added you’d have to go back and validate all previously added fields since they have different “potential to be invalid” text. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm kinda feels like this is projecting an implementation into the spec situation.
I would argue that the spec should not care how its implemented - sure it could be a new OneOfInputType class at implementation time (I toyed with this in graphql-java but rejected it as too much impact) but the spec shouldn't care.
Again an implementation detail - not sure that the spec should care. Even merging them seems straightforward since they are a non repeating directive
And implementation can merge these easily enough. Right now the spec says about input types
in other words it allows you to put extra directives on the input types during extension I just feel like this is an unnecessary restriction There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree, I'd argue that implementing Input Objects and OneOf Input Objects as the same class in the implementation is an implementation detail that the spec doesn't need to worry about 😉 At one point, oneOf was going to be its own top-level type, like To me, a OneOf Input Object is a distinct type in GraphQL that happens to share a lot with Input Objects, but still has it's own distinct behaviors in many ways. As such, changing an Input Object into a OneOf Input Object would be like changing an Input Object into an Object for me - and that's not something I think the SDL should allow. This is one of the arguments against using the directive syntactical representation in my opinion, and encouraging using a keyword instead - to make the separation clearer.
This second point is not an implementation detail, it's a spec detail - specifically it relates to lines 1724-1726 in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Along the same thoughts that @benjie is pointing out, I personally would say a different syntactical representation in the SDL would make it appear as a "distinct type", whereas since it is a directive, it is simply additional validation rules on top of the existing input object type. Keep in mind that directives are described as such:
I think this definition fits
So the way this PR is written, I see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If directives were intended to merely annotate types, and not to modify their behavior, then perhaps There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Indeed, they are a special variant. In my opinion you wouldn't take a broad thing and then turn it into a special variant at a later stage - it needs to be a special variant from the start. But this is my opinion, I'm happy to test it against the broader working group. Imagine for a second that we didn't have input objects. You could imagine taking an object type and tagging it type UserInput @input {
id: ID!
name: String!
bio: String
} This syntactic change could indicate:
This is essentially the input object type at this point. But all we've done as annotated the existing object type with different validation logic and execution behaviors. Is the input object type really not distinct from the object type? Would turning an object type into an This, again, is one of the big issues that I have with using a directive to represent this distinct type. (I'm also in favour of using the directive because it's the smallest change, and allows it to be compatible with existing clients.) This constraint: that you can't add the directive in an extension, is my compromise - to allow us to leverage the benefits of using a directive without the footguns in terms of changing one thing into another at a later stage. |
||
6. If the original Input Object is a OneOf Input Object then: | ||
1. All fields of the Input Object type extension must be nullable. | ||
2. All fields of the Input Object type extension must not have default | ||
values. | ||
|
||
## List | ||
|
||
|
@@ -1949,6 +2027,9 @@ schema. | |
GraphQL implementations that support the type system definition language should | ||
provide the `@specifiedBy` directive if representing custom scalar definitions. | ||
|
||
GraphQL implementations that support the type system definition language should | ||
provide the `@oneOf` directive if representing OneOf Input Objects. | ||
|
||
When representing a GraphQL schema using the type system definition language any | ||
_built-in directive_ may be omitted for brevity. | ||
|
||
|
@@ -2162,3 +2243,20 @@ to the relevant IETF specification. | |
```graphql example | ||
scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122") | ||
``` | ||
|
||
### @oneOf | ||
|
||
```graphql | ||
directive @oneOf on INPUT_OBJECT | ||
``` | ||
|
||
The `@oneOf` directive is used within the type system definition language to | ||
indicate an Input Object is a OneOf Input Object. | ||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```graphql example | ||
input UserUniqueCondition @oneOf { | ||
id: ID | ||
username: String | ||
organizationAndEmail: OrganizationAndEmailInput | ||
} | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,10 @@ type Query { | |
findDog(searchBy: FindDogInput): Dog | ||
} | ||
|
||
type Mutation { | ||
addPet(pet: PetInput): Pet | ||
} | ||
|
||
enum DogCommand { | ||
SIT | ||
DOWN | ||
|
@@ -93,6 +97,23 @@ input FindDogInput { | |
name: String | ||
owner: String | ||
} | ||
|
||
input CatInput { | ||
name: String! | ||
nickname: String | ||
meowVolume: Int | ||
} | ||
|
||
input DogInput { | ||
name: String! | ||
nickname: String | ||
barkVolume: Int | ||
} | ||
|
||
input PetInput @oneOf { | ||
cat: CatInput | ||
dog: DogInput | ||
} | ||
``` | ||
|
||
## Documents | ||
|
@@ -1431,6 +1452,103 @@ arguments, an input object may have required fields. An input field is required | |
if it has a non-null type and does not have a default value. Otherwise, the | ||
input object field is optional. | ||
|
||
### OneOf Input Objects Have Exactly One Field | ||
|
||
**Formal Specification** | ||
|
||
- For each {operation} in {document}: | ||
- Let {oneofInputObjects} be all OneOf Input Objects transitively included in | ||
the {operation}. | ||
- For each {oneofInputObject} in {oneofInputObjects}: | ||
- Let {fields} be the fields provided by {oneofInputObject}. | ||
benjie marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- {fields} must contain exactly one entry. | ||
- Let {field} be the sole entry in {fields}. | ||
- Let {value} be the value of {field}. | ||
- {value} must not be the {null} literal. | ||
- If {value} is a variable: | ||
- Let {variableName} be the name of {variable}. | ||
- Let {variableDefinition} be the {VariableDefinition} named | ||
{variableName} defined within {operation}. | ||
- Let {variableType} be the expected type of {variableDefinition}. | ||
- {variableType} must be a non-null type. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if these lines relating to variable usage pre-empt the discussion around #1059 and should be pulled from this spec change (simplifying it). Variables must be only of the allowed type, but it seems that we should specify what that entails for all variables and types only in one place, i.e. the separate rule. So if we currently allow variables of nullable types to be used in non-null positions and throw a field error at runtime -- which we do -we should continue to do so irrespective of isOneOf, and if/when we make the change there, that should be done in a way that covers isOneOf as well. Encountered this while attempting to rebase graphql/graphql-js#3813 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are two arguments against this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think so? Technically all fields on a oneof are nullable, but you must specify one and it must be non-null, so this seems a very straightforward way to require that when it comes to a variable? #1059 handles non-null positions, but this is a nullable position according to the type system. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So it's a nullable type only because we want introspection to say that it's nullable because currently that's the only way of making something optional. But a null cannot be supplied, so in a sense it's by definition "a non-nullable position." So we would then have to introduce the concept of non-nullable positions that occur when (1) the type for the position is non-nullable or (2) the containing type is oneOf, and then the general rule about matching nullable and non-nullable would have to depend on this new "position" concept rather than the type itself. As I type this, I can see that this additional layer is a bad idea, and I appreciate the compromise that you have ended up with. On the other hand, in GraphQL 2.0 / TreeQL, we should definitely separate optionality and nullability, and remember to change oneOf to be better defined. (It really shouldn't be the case that you cannot use null at a nullable position.) |
||
|
||
**Explanatory Text** | ||
|
||
OneOf Input Objects require that exactly one field must be supplied and that | ||
field must not be {null}. | ||
|
||
An empty OneOf Input Object is invalid. | ||
|
||
```graphql counter-example | ||
mutation addPet { | ||
addPet(pet: {}) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
Multiple fields are not allowed. | ||
|
||
```graphql counter-example | ||
mutation addPet($cat: CatInput, $dog: DogInput) { | ||
addPet(pet: { cat: $cat, dog: $dog }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
```graphql counter-example | ||
mutation addPet($dog: DogInput) { | ||
addPet(pet: { cat: { name: "Brontie" }, dog: $dog }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
```graphql counter-example | ||
mutation addPet { | ||
addPet(pet: { cat: { name: "Brontie" }, dog: null }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
Variables used for OneOf Input Object fields must be non-nullable. | ||
|
||
```graphql example | ||
mutation addPet($cat: CatInput!) { | ||
addPet(pet: { cat: $cat }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
```graphql counter-example | ||
mutation addPet($cat: CatInput) { | ||
addPet(pet: { cat: $cat }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
If a field with a literal value is present then the value must not be {null}. | ||
|
||
```graphql example | ||
mutation addPet { | ||
addPet(pet: { cat: { name: "Brontie" } }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
```graphql counter-example | ||
mutation addPet { | ||
addPet(pet: { cat: null }) { | ||
name | ||
} | ||
} | ||
``` | ||
|
||
## Directives | ||
|
||
### Directives Are Defined | ||
|
Uh oh!
There was an error while loading. Please reload this page.