support @include/@skip directives #23
Conversation
zimuliu
force-pushed
the
support-include-skip
branch
2 times, most recently
from
9839dbe to
a974258
Compare
| @@ -183,6 +191,29 @@ export default class QueryComplexity { | |||
| (total: number, childNode: FieldNode | FragmentSpreadNode | InlineFragmentNode) => { | |||
| let nodeComplexity = 0; | |||
|
|
|||
| let includeNode = true; | |||
| let skipNode = false; | |||
There was a problem hiding this comment.
We only need one variable to determine if the node should be skipped/included, this avoids contradictory (temporary) values...
There was a problem hiding this comment.
These two variables were created for a better alignment with the GraphQL standard:
Neither @Skip nor @include has precedence over the other. In the case that both the @Skip and @include directives are provided in on the same the field or fragment, it must be queried only if the @Skip condition is false and the @include condition is true. Stated conversely, the field or fragment must not be queried if either the @Skip condition is true or the @include condition is false.
from: http://spec.graphql.org/June2018/#sec--include
We can technically use a single variable, but it would be a bit harder to follow.
| let includeNode = true; | ||
| let skipNode = false; | ||
|
|
||
| childNode.directives.forEach((directive: DirectiveNode) => { |
There was a problem hiding this comment.
In case a server makes excessive use of directives it would iterate over all the directives, extract values, etc. regardless of whether it was already determined if we include or skip the field.
For example:
{
field @skip(if: true) @otherDirective @otherDirective2 @etc
}In this case, we could skip the evaluation of the last 3 directives. I'd use Array.find instead and find the directives that result in the exclusion of the field.
There was a problem hiding this comment.
@skip and @include can only be used in queries/mutations from the client, and not in schema definitions on the server side. The chance of multiple directives on a query is really rare. Also, as the directives are not sorted, the big O complexity is the same for both forEach and find. If we use forEach, we just need to scan directives in one pass to get the job done.
Directives from the standard:
directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
|
Thanks for reviewing. Responded in the comments to explain the decisions made. Please re-review. |
|
This makes sense now, thanks for the explanations. |
Support these two directives defined in the latest GraphQL standards.
No need to compute the complexity if any of the following is found:
@include(if: false)@skip(if: true)