[validation] Allow safe divergence #229
Merged
+151
−22
Conversation
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
leebyron
force-pushed
the
allow-safe-divergence
branch
from
c9c56f2 to
88ab1e7
Compare
|
yeah this makes a lot of sense #shipit |
leebyron
force-pushed
the
allow-safe-divergence
branch
2 times, most recently
from
ffc9bec to
4eaf6fd
Compare
As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as a potential conflict. This change loosens this rule, allowing fields which can never both apply in a frame of execution to diverge.
leebyron
force-pushed
the
allow-safe-divergence
branch
from
4eaf6fd to
d71e063
Compare
leebyron
added a commit
that referenced
this pull request
Nov 13, 2015
[validation] Allow safe divergence
leebyron
added a commit
to graphql/graphql-spec
that referenced
this pull request
Nov 13, 2015
This loosens the rules for fields of the same response name which occur in the same selection set. Implemented in JS by graphql/graphql-js#230 and graphql/graphql-js#229 This allows for fields of the same response name to differ in a few conditions deemed to be safe where previously the validation was too conservative. The currently supported directives: @Skip and @include do not create an ambiguous or erroneous query when used on conflicting fields in a divergent fashion. In fact, this may be intended when many different conditions should result in the same outcome. For example: ```graphql { field @include(if: $firstCondition) field @include(if: $secondCondition) } ``` This example could be considered as the intent of fetching `field` given `$firstCondition || $secondCondition`. While this example is contrived, there are more complex examples where such fields are nested within fragments that this condition is reasonable. The following example is now legal, since no object can be both the concrete object type "Cat" and "Dog" at the same time, so the response shape is still unambiguous provided the types are known. ```graphql { ... on Cat { name } ... on Dog { name: nickname } } ``` However the following is still not legal, since it results in ambiguity: ```graphql { name ... on Dog { name: nickname } } ```
andimarek
added a commit
to graphql-java/graphql-java
that referenced
this pull request
Dec 17, 2015
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.
As pointed out in #53, our validation is more conservative than it needs to be in some cases. Particularly, two fields which could not overlap are still treated as a potential conflict.
This change loosens this rule, allowing fields which can never both apply in a frame of execution to diverge.