Skip to content

Support graphql oneOf directive without composeDirective#306

Open
jdolle wants to merge 3 commits into
mainfrom
one-of-native-support
Open

Support graphql oneOf directive without composeDirective#306
jdolle wants to merge 3 commits into
mainfrom
one-of-native-support

Conversation

@jdolle

@jdolle jdolle commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Apollo doesn't support oneOf because they are using an out dated version of graphql.

This PR adds functionality to keep the oneOf directives through composition and adds a test to ensure this works.

This avoids having to unnecessarily add @composeDirective for @oneOf. @composeDirective should be reserved for custom directives instead of native directives.

@jdolle jdolle requested a review from kamilkisiela July 10, 2026 19:06
@jdolle jdolle self-assigned this Jul 10, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for composing the built-in @oneOf directive on input object types during supergraph composition. It updates the subgraph state builder to parse @oneOf on input objects, propagates this state through the supergraph composition AST, and outputs the directive in the resulting supergraph and public SDLs. Additionally, a skipApolloBuildCheck option was added to the test kit to bypass Apollo Gateway validation when needed. Feedback on the changes highlights two issues: a leftover test.only modifier in the new test file that would cause other tests to be skipped, and a leftover console.log debugging statement in validate-state.ts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread __tests__/composition.spec.ts Outdated
Comment thread src/subgraph/validation/validate-state.ts
@jdolle jdolle force-pushed the one-of-native-support branch from fc88b3d to 362d87c Compare July 10, 2026 19:31
Comment thread __tests__/composition.spec.ts Outdated
});

test("guild composition supports composing built-in directive (@oneOf)", () => {
api.runIf("guild", () => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

only run for guild b/c others dont support atm

Comment thread src/subgraph/state.ts Outdated
) {
getOrCreateInputObjectType(state, typeName).isOneOf =
true;
const oneOfDirective = getOrCreateDirective(state, 'oneOf');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

must ensure the definition is added or else Supergraph.build fails because the internal version of graphql it's using is out of date

@kamilkisiela

Copy link
Copy Markdown
Contributor

Few things:

  • What will happen if subgraph A already has @oneOf defined via @composeDirective, and after we introduce native support for it, subgraph B adds @oneOf without @composeDirective
  • Let's add a few scenarios where you try to "break" it, like two subgraphs defining the same input type, a case where the input type is defined twice in two subgraphs, but fields differ, where one defines @oneOf and the other does not etc

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants