Handle operation parsing error GraphQL #2434
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
CodSpeed Performance ReportMerging #2434 will not alter performanceComparing Summary
|
vohoanglong0107
force-pushed
the
feat-graphql-error-parse-operation
branch
from
5cc07f8
to
79e3871
Compare
vohoanglong0107
force-pushed
the
feat-graphql-error-parse-operation
branch
from
79e3871
to
83949bf
Compare
vohoanglong0107
force-pushed
the
feat-graphql-error-parse-operation
branch
from
83949bf
to
797b369
Compare
| GraphqlBogus { | ||
| items: [ | ||
| GraphqlBogus { | ||
| items: [ |
There was a problem hiding this comment.
This doesn't seem right: two bogus nodes with two items
Plus, we already have GraphqlBogusSelection. I am not sure what's the recovery strategy at the moment
There was a problem hiding this comment.
The reason there is a nested bogus nodes here was because in this case:
{
hero @deprecated(: "Deprecated")
}The parser tried to parse (: "Deprecated") as an GraphqlArgumentList, but since this argument list has an invalid argument : "Deprecated" the entire list was turn into a bogus node, which caused @deprecated(: "Deprecated") to turn from a GraphqlDirective to a bogus node (as it contained an bogus node instead of an argument list), which turned hero @deprecated(: "Deprecated") from a GraphqlField to a bogus node, caused a series of nested bogus nodes. Do you have any suggestions on how to improve this?
| GraphqlBogus { | ||
| items: [ | ||
| L_PAREN@50..51 "(" [] [], | ||
| GraphqlBogus { | ||
| items: [ | ||
| GraphqlBogus { | ||
| items: [ |
There was a problem hiding this comment.
Same here. Ideally we should have a better recovering strategy
| }, | ||
| GraphqlBogus { | ||
| items: [ | ||
| GraphqlBogus { | ||
| items: [ | ||
| AT@192..193 "@" [] [], | ||
| GraphqlBogus { | ||
| items: [ | ||
| L_PAREN@193..194 "(" [] [], | ||
| GraphqlBogus { | ||
| items: [ | ||
| GraphqlBogus { | ||
| items: [ |
There was a problem hiding this comment.
Maybe we should have smaller cases to better evaluate the recovery strategy. The parser seems to not know that is in recovery mode and keeps creating bogus nodes.
There was a problem hiding this comment.
It the same issue with the above but with this case:
{
hero @(: "Deprecated"
}
Summary
Handle parsing for operation in GraphQL parser
Test Plan
All the tests should pass