Skip to content

OneOf inhabitability#1211

Open
jbellenger wants to merge 2 commits intographql:mainfrom
jbellenger:jbellenger-oneof-inhabitability
Open

OneOf inhabitability#1211
jbellenger wants to merge 2 commits intographql:mainfrom
jbellenger:jbellenger-oneof-inhabitability

Conversation

@jbellenger
Copy link
Contributor

@jbellenger jbellenger commented Feb 17, 2026

OneOf types can form graphs for which finite values cannot be created.

The simplest example of this is a self-recursing OneOf type:

input A @oneOf { a:A }

While this is the simplest form of the issue, we can combine this with other kinds of input object types to create other uninhabited types:

input A @oneOf { b:B }
input B { a:A! }

These kinds of types are considered valid on two fronts today:

  1. They are valid as OneOf types because all field types are nullable and there are no default values
  2. They are valid as a circular input object because there are no unbreakable chains of non-null non-list fields

While this follows the letter of the Circular References part of the spec, it violates the spirit of the spec in that there is no way to create a finite, legal value for these types. I propose that these topologies be declared invalid.

I took a stab at reworking the Circular References section to consider OneOfs and added a more formal specification framed around finite values.

@netlify
Copy link

netlify bot commented Feb 17, 2026

Deploy Preview for graphql-spec-draft ready!

Name Link
🔨 Latest commit 3d521c2
🔍 Latest deploy log https://app.netlify.com/projects/graphql-spec-draft/deploys/69a0a6f22520c900080d4b3e
😎 Deploy Preview https://deploy-preview-1211--graphql-spec-draft.netlify.app
📱 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 project configuration.

@jbellenger jbellenger marked this pull request as ready for review February 17, 2026 00:17
@benjie benjie added the 💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md) label Feb 17, 2026
@benjie
Copy link
Member

benjie commented Feb 17, 2026

Excellent catch! I think we can and should fast track this. Once you have the GraphQL.js implementation in place either add it to the next GraphQL WG agenda or let me know if you’d rather I represent it on your behalf.

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the named type to an assertion for clarity

Comment on lines 1813 to 1819
- Let {namedFieldType} be the underlying named type of {fieldType}.
- If {namedFieldType} is not an Input Object type:
- Return {true}.
- If {namedFieldType} is not a _OneOf Input Object_:
- Return {true}.
- If {OneOfInputObjectCanBeProvidedAFiniteValue(namedFieldType, nextVisited)}:
- Return {true}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Let {namedFieldType} be the underlying named type of {fieldType}.
- If {namedFieldType} is not an Input Object type:
- Return {true}.
- If {namedFieldType} is not a _OneOf Input Object_:
- Return {true}.
- If {OneOfInputObjectCanBeProvidedAFiniteValue(namedFieldType, nextVisited)}:
- Return {true}.
- Assert: {fieldType} is a named type.
- If {fieldType} is not an Input Object type:
- Return {true}.
- If {fieldType} is not a _OneOf Input Object_ type:
- Return {true}.
- If {OneOfInputObjectCanBeProvidedAFiniteValue(fieldType, nextVisited)}:
- Return {true}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

viaductbot pushed a commit to airbnb/viaduct that referenced this pull request Feb 17, 2026
viaduct.arbitrary.graphql is a suite of generators used to property test Viaduct and other internal airbnb systems. The generators in this package use a model for GraphQL values, which can be applied to GraphQL objects that require them. Examples of this include the default values for a graphql argument definition, the arguments/variables used in a GraphQL document, or the result of executing a selection set.

This value model has evolved over time -- the initial `RawValue` family of types has been gradually replaced with an `IR.Value` model. Currently, both models exist in the code base and are used for slightly different jobs.

This PR completely removes the `RawValue` model and updates all systems to use `IR.Value`. Along the way it also updates most components to operate on a ViaductSchema rather than graphql-java's GraphQLSchema.

**Better Edgecase Coverage**
Before this change, the schema generator would assign default values and applied directives as it generated each type.
This created complexity in other parts of the system (eg value generators had to know how to not generate a value for a type that hadn't been defined yet), and made it impossible for the system to generate some kinds of edge cases.

This PR changes this behavior to insert default values and applied directives as additional passes, after all types have been generated. This simplifies value generation and allows for more kinds of edge cases to be generated.

This ability to generate more edge cases has surfaced some novel issues, such as this potential spec issue with OneOf types: graphql/graphql-spec#1211

Github-Change-Id: 998226
GitOrigin-RevId: 6ae10823b358faf51f883d593d33d6767933ee8b
reframe the issue as as general class of input object habitation, not
just a special edge case around pure OneOf trees. This allows handling
the case of mixed OneOf/non-OneOf types, like:

```graphql
input A @OneOf { b: B }
input B { a: A! }
```

To support this, I've also rewritten the previously-proposed
`OneOfInputObjectCanBeProvidedAFiniteValue` function as
`InputObjectCanBeProvidedAFiniteValue`, which handles both pure inputs,
pure OneOfs, and mixed input/OneOfs validation.
@jbellenger
Copy link
Contributor Author

@benjie after looking at this more, I realized that my previous proposal only dealt with closed OneOf trees, but that one can also define uninhabited types by mixing OneOf and non-OneOfs.

For example, this cycle is considered valid by both the current spec and my original proposal:

input A @oneOf { b:B }
input B { a:A! }

I've updated this PR to fold the overly-specific OneOf rules I'd originally written into the Cyclic References section, capturing the general shape of the problem for both flavors of input objects.

This is implemented in graphql-js PR graphql/graphql-js#4564

I was going to plan on being at the graphql-wg call on March 5th, though with the slightly wider scope of this PR I'd be fine to catch a different meeting.

Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! For Non-Null type we normally say "Let {innerType} be the inner type of {...}" or "Let {nullableType} be the unwrapped nullable type of", but these are just editorial nits really - the structure looks correct to me.

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}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit... Maybe:

Suggested change
3. {InputObjectCanBeProvidedAFiniteValue(inputObject)} must be {true}.
3. {InputObjectHasUnbreakableCycle(inputObject)} must be {false}.

Comment on lines +1802 to +1803
- Let {innerType} be the type wrapped by {fieldType}.
- If not {FieldTypeCanBeProvidedAFiniteValue(innerType, nextVisited)}:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Let {innerType} be the type wrapped by {fieldType}.
- If not {FieldTypeCanBeProvidedAFiniteValue(innerType, nextVisited)}:
- Let {nullableType} be the unwrapped nullable type of {fieldType}.
- If not {FieldTypeCanBeProvidedAFiniteValue(nullableType, nextVisited)}:

Comment on lines +1812 to +1813
- Let {innerType} be the type wrapped by {fieldType}.
- Return {FieldTypeCanBeProvidedAFiniteValue(innerType, visited)}.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Let {innerType} be the type wrapped by {fieldType}.
- Return {FieldTypeCanBeProvidedAFiniteValue(innerType, visited)}.
- Let {nullableType} be the unwrapped nullable type of {fieldType}.
- Return {FieldTypeCanBeProvidedAFiniteValue(nullableType, visited)}.

- Let {innerType} be the type wrapped by {fieldType}.
- Return {FieldTypeCanBeProvidedAFiniteValue(innerType, visited)}.
- Assert: {fieldType} is a named type.
- If {fieldType} is not an Input Object type:
Copy link
Member

Choose a reason for hiding this comment

The 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
- If {fieldType} is not an Input Object type:
- If {fieldType} is not an Input Object type (including variants):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

💭 Strawman (RFC 0) RFC Stage 0 (See CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants