Fix bug in AstPrinter preventing query short form from being used

[kilink]

AstPrinter's operationDefinition method had a String comparison that would
always fail when checking the node's operation; the String contained in
the op variable was already lowercased, while the constant "QUERY" was
used in the comparison. The result was that query short form was
never used when printing queries.

[bbakerman]

I am not sure we want this behavior. I get that that printAstCompact might lead to { f } if there is no args etc.. (is more compact) but the normal printAst probably should use the slightly longer for.

@andimarek what are your thoughts?

[bbakerman]

This was ported from graphql.js originally so in a sense could do what they do

And they do this

src/language/printer.js

  OperationDefinition(node) {
    const op = node.operation;
    const name = node.name;
    const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')');
    const directives = join(node.directives, ' ');
    const selectionSet = node.selectionSet;
    // Anonymous queries with no directives or variable definitions can use
    // the query short form.
    return !name && !directives && !varDefs && op === 'query'
      ? selectionSet
      : join([op, join([name, varDefs]), directives, selectionSet], ' ');
  },

[bbakerman]

which is inline with what this PR does

[luiznaac]

@bbakerman I recently updated to the newest release (19.0) and found out that a test of mine was failing due to this modification. There's no parameter which we could pass nor a specific function we could call to get this behavior. IMHO, it's misleading to just assume the client wants the query in short form just because there're no names, vars nor directives.... shouldn't we introduce a more explicit way to achieve it?

[andimarek]

Thanks a lot @luiznaac for calling this out. We discussed this and decided that this is a change we want to make, but we didn't document this explicitly in the release notes. I just updated them to make this clear.

We also discussed introducing an option, but we decided against it as it is semantically the same.