Support directives on the schema - #824
Merged
Merged
Conversation
Directives on `schema @foo { ... }` and `extend schema @foo` were never read, so e.g. Apollo Federation v2's `extend schema @link(...)` was missing from the GraphQLSchema and from federation-jvm's _service.sdl. They're now added as schema applied directives, the schema definition's first, then its extensions'. RootTypeInfo also took the last SchemaDefinition, which can be an extension, so `schema { query: MyQuery }` followed by `extend schema @foo` lost the custom root type. Operation types are now merged across the definition and its extensions. Only applied directives are set on the schema: graphql-java doesn't resolve type references in the deprecated legacy schema directives, so an enum argument would fail validation. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- New SchemaParserOptions.allowUndeclaredDirectives, off by default - Undeclared directives are kept as applied directives only, with argument types guessed from their values - Fails with a SchemaError when an argument type can't be guessed
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #717
Fixes #729
Checklist
Description
Directives on
schema @foo { ... }andextend schema @foowere never read, so they didn't end up inGraphQLSchema.getSchemaAppliedDirectives(). For Apollo Federation v2 this meansextend schema @link(...)was missing from federation-jvm's_service.sdl, and the gateway treated the subgraph as Federation 1. They're now added as schema applied directives, the schema definition's first, then its extensions' (same order as graphql-java'sSchemaGenerator).Also fixed
RootTypeInfotaking the lastSchemaDefinition.SchemaExtensionDefinitionis a subclass of it, soschema { query: MyQuery }followed byextend schema @foolost the custom root type and failed with "Type definition for root query type 'Query' not found". Operation types are now merged across the definition and its extensions.Only applied directives are set on the schema, not the deprecated legacy
withSchemaDirectives. graphql-java doesn't resolve type references in that list, so a directive with an enum argument (e.g.@link(..., for: SECURITY)) failed schema validation with aClassCastException.getSchemaDirectives()returned nothing before this change either.SchemaObjectsgets aschemaAppliedDirectivesproperty, defaulting to empty.@JvmOverloadsand a hiddencopy()overload keep the old constructor andcopy()signatures, so code compiled against 14.0.x still works.Checked end to end with federation-jvm 6.2.1 (on graphql-java 25, since that's what it's built against): the
_service.sdlnow has the@link, and on master it didn't.Behaviour change: a directive on
schema/extend schemathat isn't declared, or uses an argument its definition doesn't have, now throws aSchemaErrorlike it does everywhere else. Before, it was silently dropped. This also means federation users who declared@keyetc. but not@linkwill need to declare@linktoo. The existingSchemaClassScannerTestfederation fixture had exactly that problem (it appliedimport:without declaring it), so I fixed it.Undeclared directives, like the
@link-imported ones in the last comment on #717, still fail by default. For #729 there's a newallowUndeclaredDirectivesoption: directives without a definition are then added as applied directives, with argument types guessed from their values like in 13.0.1. They're not added as legacy directives, since graphql-java rejects those without a definition. Enum and object values can't be guessed, so those still throw and ask to declare the directive.🤖 Generated with Claude Code