Skip to content

Aligned the OneOf implementation with the specification #8352

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

glen-84
Copy link
Collaborator

@glen-84 glen-84 commented Jun 10, 2025

Summary of the changes (Less than 80 chars)

  • Aligned the OneOf implementation with the specification.

  • Updated the OneOf directive description.
  • Renamed Oneof/oneof to OneOf.
  • Renamed __Type.oneOf to __Type.isOneOf.
  • Added test for { a: null }.
  • Updated tests.
  • (WIP) Moved validation from OneOf-specific rule to VariablesInAllowedPositions.

❓ Questions:

  • How should we handle this change? (see Slack)
  • "The @oneOf directive must not be provided by an Input Object type extension."
    • How should we handle this? Is this only for "real" extensions? Do we even support that? Does it apply to HC type extensions?
  • ✅ I'm not sure how to implement VariableVisitor.IsNonNullPosition based on the spec.
    • Note: 3 tests currently skipped.
  • Do we need to change something here?
  • ✅ Should we add it as a built-in directive to all "BuiltIns"-like classes? (ref)
  • Should we "unflag" it (remove config, on by default)? (seems to be the case in JS/Ruby and maybe Java).

@glen-84 glen-84 marked this pull request as draft June 10, 2025 14:00
@glen-84 glen-84 added this to the HC-16.0.0 milestone Jun 10, 2025
@glen-84 glen-84 force-pushed the gai/one-of-alignment branch from 4b27509 to 3e96bca Compare June 13, 2025 10:19
@glen-84 glen-84 force-pushed the gai/one-of-alignment branch from 6229b24 to f4b1b5a Compare June 13, 2025 13:18
@glen-84 glen-84 marked this pull request as ready for review June 13, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant