Make variableDefinitions not nullable

[brandon-leapyear]

✨ This is an old work account. Please reference @brandonchinn178 for all future communication ✨

---

It seems like variableDefinitions will always be a non-null array.

graphql-js/src/language/parser.js

Lines 258 to 285 in f07ec24

	parseOperationDefinition(): OperationDefinitionNode {
	const start = this._lexer.token;
	if (this.peek(TokenKind.BRACE_L)) {
	return {
	kind: Kind.OPERATION_DEFINITION,
	operation: 'query',
	name: undefined,
	variableDefinitions: [],
	directives: [],
	selectionSet: this.parseSelectionSet(),
	loc: this.loc(start),
	};
	}
	const operation = this.parseOperationType();
	let name;
	if (this.peek(TokenKind.NAME)) {
	name = this.parseName();
	}
	return {
	kind: Kind.OPERATION_DEFINITION,
	operation,
	name,
	variableDefinitions: this.parseVariableDefinitions(),
	directives: this.parseDirectives(false),
	selectionSet: this.parseSelectionSet(),
	loc: this.loc(start),
	};
	}

graphql-js/src/language/parser.js

Line 307 in f07ec24

parseVariableDefinitions(): Array<VariableDefinitionNode> {

[IvanGoncharov]

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.

[yaacovCR]

closing this PR in favor of #4206 based on suggestion above in #2405 (comment)