-
Notifications
You must be signed in to change notification settings - Fork 2k
Descriptions on executable documents #4430
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: 16.x.x
Are you sure you want to change the base?
Descriptions on executable documents #4430
Conversation
Enhance GraphQL parser and printer to support descriptions for operations, variables, and fragments This commit is based on graphql#3402 , rebased onto graphql-js 16 and with added tests Implements graphql/graphql-spec#1170
src/language/parser.ts
Outdated
if (hasDescription) { | ||
throw syntaxError( | ||
this._lexer.source, | ||
this._lexer.token.start, | ||
'Unexpected description, descriptions are supported only on type definitions.', | ||
); | ||
} | ||
|
||
switch (keywordToken.value) { | ||
case 'query': | ||
case 'mutation': | ||
case 'subscription': | ||
return this.parseOperationDefinition(); | ||
case 'fragment': | ||
return this.parseFragmentDefinition(); | ||
case 'extend': | ||
if (hasDescription) { | ||
throw syntaxError( | ||
this._lexer.source, | ||
this._lexer.token.start, | ||
'Unexpected description, descriptions are not supported on type extensions.', | ||
); | ||
} |
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.
Currently this changes the behavior of the this.unexpected()
fallback lower down. I think we should restore the previous format - two switch statements with a "no descriptions" assertion between them.
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.
I took a stab at addressing this feedback slightly differently to better preserve existing behavior, please see
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.
Looks good! We should have a test to ensure that a description on an anonymous query results in a sensible parse error if we don't already have one, plus the comment I made before. Other than that, I think this is good to go!
"""Invalid"""
{ __typename }
c70433e
to
c573130
Compare
Enhance GraphQL parser and printer to support descriptions for operations, variables, and fragments
This commit is based on #3402 , rebased onto graphql-js 16 and with added tests
Implements graphql/graphql-spec#1170