-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix for #64: return before error on missing fields #73
Fix for #64: return before error on missing fields #73
Conversation
@rachsmithcodes: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
@@ -175,7 +175,7 @@ export function NoMissingClientDirectives(context: ValidationContext) { | |||
const fieldDef = context.getFieldDef(); | |||
|
|||
// if we don't have a type to check then we can early return | |||
if (!parentType) return; | |||
if (!parentType || fieldDef) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps reverse the check on fieldDef here? !fieldDef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree it looks like we need to flip this condition, went ahead and made that change in 9d95cac
78e9cc4
to
552dba3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rachsmithcodes!
Fixed broken test in a follow-on commit: bf80011 |
I am getting the error described in #64.
As far as I can tell, it is happening in when a GraphQL document includes a field that doesn't exist on the server schema. In this scenario the validation should add an error like this:
But when it gets run though the
NoMissingClientDirectives
rule,context.getFieldDef()
returnsundefined
, causing the extension to error on line 103 ofvalidation.js
.Adding an early return if the
fieldDef
doesn't exist prevents the error.┆Issue is synchronized with this Jira Task by Unito