Make variableDefinitions not nullable
:sparkles: This is an old work account. Please reference @brandonchinn178 for all future communication :sparkles:
It seems like variableDefinitions will always be a non-null array.
https://github.com/graphql/graphql-js/blob/f07ec241d51d7c3fb0263bb3ade1975dc87babc3/src/language/parser.js#L258-L285 https://github.com/graphql/graphql-js/blob/f07ec241d51d7c3fb0263bb3ade1975dc87babc3/src/language/parser.js#L307
Hi @brandon-leapyear
Thanks for PR 👍 AST format should have a healthy balance between the ability to easily process but at the same to easily construct AST nodes. For example, at the moment you can easily extract part of the query into separate query:
return {
kind: Kind.OPERATION_DEFINITION,
selectionSet: someSelectionSet,
};
And you don't need to add a bunch of empty arrays, moreover, it makes code less fragile when we add new properties to AST nodes.
But I agree it looks weird and creates complications, for example with testing see #2203
Personally I think a good long term solution would be to always return undefined instead of empty arrays since it's will improve performance and memory consumption.
Also, it would be great if someone researches what other JS parsers do e.g. Babel's parser, ESLint parser, etc.