Skip to content

gateway(feat): Allow complete SDL - follow-up to #4209 #4228

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

Merged
merged 7 commits into from
Jun 11, 2020

Conversation

trevor-scheer
Copy link
Contributor

@trevor-scheer trevor-scheer commented Jun 9, 2020

This PR adds additional normalization to composition inputs in order to prevent duplicate directive errors when constructing the schema.

The oversight from #4209 that's being corrected by this PR is to also remove specified directive definitions (@deprecated, @skip, @include, and @specifiedBy) from composition inputs. Failure to remove them during composition results in duplicate definitions and graphql validation errors if they are included in the composition inputs.

This commit adds additional normalization to composition inputs in order to
prevent duplicate directive errors when constructing the schema.

The oversight from #4209 that's being corrected by this commit is to _also_
remove specified directive definitions (to be included by the composition
work itself). Failure to remove them during composition results in duplicate
definitions and graphql validation errors.
@trevor-scheer trevor-scheer force-pushed the trevor/normalization-strip-default-directives branch from 0f9d9dc to a97a502 Compare June 9, 2020 22:35
@tapaderster
Copy link

What about common custom directives in federation schema?
Let's say I have a custom type directive named @exampleDirective in all of my federated service sdls. Would that work or do I need to strip those custom schema definitions from the sdl manually?

@trevor-scheer
Copy link
Contributor Author

@tapaderster a good question! Composition actually already handles this, if you take a look at this #3536. Currently the type system directives are ignored by composition altogether (meaning they're supported, just implemented at the service level). See also #3464 for details on executable directives, which are also supported by the gateway but with some caveats.

@trevor-scheer trevor-scheer merged commit 672fb1b into master Jun 11, 2020
@trevor-scheer trevor-scheer deleted the trevor/normalization-strip-default-directives branch June 11, 2020 20:03
@abernix abernix added this to the Release 2.14.4 milestone Jun 12, 2020
abernix pushed a commit to apollographql/federation that referenced this pull request Sep 4, 2020
…-server#4209 (apollographql/apollo-server#4228)

This commit adds additional normalization to composition inputs in order to
prevent duplicate directive errors when constructing the schema.

The oversight from apollographql/apollo-server#4209 that's being corrected by this PR is to also remove
specified directive definitions (@deprecated, @Skip, @include, and
@SpecifiedBy) from composition inputs. Failure to remove them during
composition results in duplicate definitions and graphql validation errors if
they are included in the composition inputs.
Apollo-Orig-Commit-AS: apollographql/apollo-server@672fb1b
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants