-
Notifications
You must be signed in to change notification settings - Fork 1.1k
OneOf inhabitability #1211
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?
OneOf inhabitability #1211
Changes from 2 commits
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 | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1594,9 +1594,9 @@ Input Objects are allowed to reference other Input Objects as field types. A | |||||||||||||||||||
| circular reference occurs when an Input Object references itself either directly | ||||||||||||||||||||
| or through referenced Input Objects. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Circular references are generally allowed, however they may not be defined as an | ||||||||||||||||||||
| unbroken chain of Non-Null singular fields. Such Input Objects are invalid | ||||||||||||||||||||
| because there is no way to provide a legal value for them. | ||||||||||||||||||||
| Circular references are generally allowed, however they may not form cycles such | ||||||||||||||||||||
| that no finite value can be provided. Such Input Objects are invalid because | ||||||||||||||||||||
| there is no way to provide a legal value for them. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| This example of a circularly-referenced input type is valid as the field `self` | ||||||||||||||||||||
| may be omitted or the value {null}. | ||||||||||||||||||||
|
|
@@ -1642,6 +1642,22 @@ input Second { | |||||||||||||||||||
| } | ||||||||||||||||||||
| ``` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Because _OneOf Input Objects_ require exactly one field to be set and non-null, | ||||||||||||||||||||
| nullable fields do not allow a finite value to be provided as they do in other | ||||||||||||||||||||
| Input Objects. This example is invalid because providing a value for `First` | ||||||||||||||||||||
| requires a non-null `Second`, and constructing a `Second` requires a non-null | ||||||||||||||||||||
| `First`: | ||||||||||||||||||||
|
Member
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. Agree with @BoD; perhaps:
Suggested change
Contributor
Author
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. Thanks @benjie ! In applying this change, I re-read the nearby Circular References section and saw some opportunities to simplify:
My inclination is to simplify these examples by removing the As a counter-argument, the
Member
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 have no preference either way on the end result (that's more a question for @leebyron) but if you want to rework the existing examples please do so via a separate PR not this one.
Member
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. Scratch that I do have a preference: leave the existing circular references with the Changing the order of value/self in example 84 to be consistent would be a fine separate editorial PR if you were so inclined.
Contributor
Author
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. Great! I spun this out into another PR at #1214 |
||||||||||||||||||||
|
|
||||||||||||||||||||
| ```graphql counter-example | ||||||||||||||||||||
| input First @oneOf { | ||||||||||||||||||||
| second: Second | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| input Second { | ||||||||||||||||||||
| first: First! | ||||||||||||||||||||
| } | ||||||||||||||||||||
| ``` | ||||||||||||||||||||
|
|
||||||||||||||||||||
| **Result Coercion** | ||||||||||||||||||||
|
|
||||||||||||||||||||
| An input object is never a valid result. Input Object types cannot be the return | ||||||||||||||||||||
|
|
@@ -1726,9 +1742,7 @@ input ExampleInputObject { | |||||||||||||||||||
| 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. | ||||||||||||||||||||
| 3. {InputObjectCanBeProvidedAFiniteValue(inputObject)} must be {true}. | ||||||||||||||||||||
|
benjie marked this conversation as resolved.
Outdated
|
||||||||||||||||||||
| 4. {InputObjectDefaultValueHasCycle(inputObject)} must be {false}. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| InputObjectDefaultValueHasCycle(inputObject, defaultValue, visitedFields): | ||||||||||||||||||||
|
|
@@ -1768,6 +1782,40 @@ InputFieldDefaultValueHasCycle(field, defaultValue, visitedFields): | |||||||||||||||||||
| - Return {InputObjectDefaultValueHasCycle(namedFieldType, fieldDefaultValue, | ||||||||||||||||||||
| nextVisitedFields)}. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| InputObjectCanBeProvidedAFiniteValue(inputObject, visited): | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - If {visited} is not provided, initialize it to the empty set. | ||||||||||||||||||||
| - If {inputObject} is in {visited}: | ||||||||||||||||||||
| - Return {false}. | ||||||||||||||||||||
| - Let {nextVisited} be a new set containing {inputObject} and everything | ||||||||||||||||||||
| from {visited}. | ||||||||||||||||||||
| - If {inputObject} is a _OneOf Input Object_: | ||||||||||||||||||||
| - For each field {field} of {inputObject}: | ||||||||||||||||||||
| - Let {fieldType} be the type of {field}. | ||||||||||||||||||||
| - If {FieldTypeCanBeProvidedAFiniteValue(fieldType, nextVisited)}: | ||||||||||||||||||||
| - Return {true}. | ||||||||||||||||||||
| - Return {false}. | ||||||||||||||||||||
| - Otherwise: | ||||||||||||||||||||
| - For each field {field} of {inputObject}: | ||||||||||||||||||||
| - Let {fieldType} be the type of {field}. | ||||||||||||||||||||
| - If {fieldType} is Non-Null: | ||||||||||||||||||||
| - Let {innerType} be the type wrapped by {fieldType}. | ||||||||||||||||||||
| - If not {FieldTypeCanBeProvidedAFiniteValue(innerType, nextVisited)}: | ||||||||||||||||||||
|
benjie marked this conversation as resolved.
Outdated
|
||||||||||||||||||||
| - Return {false}. | ||||||||||||||||||||
| - Return {true}. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| FieldTypeCanBeProvidedAFiniteValue(fieldType, visited): | ||||||||||||||||||||
|
|
||||||||||||||||||||
| - If {fieldType} is a List type: | ||||||||||||||||||||
| - Return {true}. | ||||||||||||||||||||
| - If {fieldType} is a Non-Null type: | ||||||||||||||||||||
| - Let {innerType} be the type wrapped by {fieldType}. | ||||||||||||||||||||
| - Return {FieldTypeCanBeProvidedAFiniteValue(innerType, visited)}. | ||||||||||||||||||||
|
benjie marked this conversation as resolved.
Outdated
|
||||||||||||||||||||
| - Assert: {fieldType} is a named type. | ||||||||||||||||||||
| - If {fieldType} is not an Input Object type: | ||||||||||||||||||||
|
Member
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. Not sure if we need this added for clarity... To make it clear that OneOf Input Objects are included?
Suggested change
Contributor
Author
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 think this has the potential to add more confusion than it clarifies. "Input Object type", without an 'all variants' qualifier, is used consistently to describe OneOf and non-OneOf types. Adding "(including variants)" only here might make readers wonder whether the unqualified uses elsewhere are intentionally exclusive. That said, you know the spec's editorial voice better than I do, so happy to add it if you think it helps.
Member
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. Nah, I also don’t think it should be there, but I think it might warrant discussion. |
||||||||||||||||||||
| - Return {true}. | ||||||||||||||||||||
| - Return {InputObjectCanBeProvidedAFiniteValue(fieldType, visited)}. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ### OneOf Input Objects | ||||||||||||||||||||
|
|
||||||||||||||||||||
| :: A _OneOf Input Object_ is a special variant of _Input Object_ where exactly | ||||||||||||||||||||
|
|
||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the issue and the counter-example below, but am having trouble parsing/understanding the meaning of this sentence "nullable fields do not allow a finite value to be provided as they do in other Input Objects". Maybe it could be rephrased or maybe it's just me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair -- I was trying to describe that while a nullable field would normally provide an escape from infinite recursion, the field-value-may-not-be-null requirement of a OneOf prevents using this escape.