Empty runtime variables with @include directive
zcei opened this issue · 2 comments
Hej folks,
when using the validation rule in combination with a query that provides a variable with a default value, the @include (and potentially also the @skip) directive is causing troubles.
GraphQLError: Argument "if" of required type "Boolean!" was provided the variable "$shouldSkip" which was not provided a runtime value.
I was able to reproduce this with a test case, and I'm wondering what the expected behavior is here.
it('should respect @include(if: false) via default variable value', () => {
const ast = parse(`
query Foo ($shouldSkip: Boolean = false) {
variableScalar(count: 10) @include(if: $shouldSkip)
}
`);
const complexity = getComplexity({
estimators: [
simpleEstimator({defaultComplexity: 1})
],
schema,
query: ast
});
expect(complexity).to.equal(0);
});I see that you're only using graphql-js's getDirectiveValues, which under the hood has a logic that throws the error here.
But to me it sounds like it should first check for the query document default variables.
I feel like we have two options here:
- obtain default variables from operation definition here and add these to
this.options.variables(or a derivate object of it). - try to convince
graphql-jsmaintainers that there's a flaw in their flow of obtaining runtime values.
As the graphql-js library does not seem to have issues with @include directives, I'm leaning towards (1), wdyt?
Thanks for bringing this up! Looks like the default query variables were not considered at all for the complexity and this showed up in the skip / include directives now. I now added the same logic to coerce the variables that is used in the graphql-js library for the execution phase. Your test is now passing. I also added one for field input arguments + default values.
Oh ma gee, that's awesome! I haven't even seen getVariableValues and was a bit 😞 that I would need to manually build it. Thanks so much ❤️