Skip to content

Conversation

asiandrummer
Copy link
Contributor

Addresses #288. Simply inserts another check to see OperationType is query and only omit the keyword in that case. For mutation (and subscription) the query string should still contain OperationType.

@@ -34,6 +34,23 @@ describe('Printer', () => {
);
});

it('correctly prints non-query operations without name', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test could potentially be within kitchen sink test, but I felt like I'd slow down the tests using it. Let me know if you think otherwise.

@leebyron
Copy link
Contributor

leebyron commented Feb 9, 2016

Sweet, thanks so much for digging into this!

@asiandrummer asiandrummer force-pushed the printer branch 2 times, most recently from 5540f64 to bf9e385 Compare February 9, 2016 01:54
@@ -31,7 +31,8 @@ const printDocASTReducer = {
const defs = wrap('(', join(node.variableDefinitions, ', '), ')');
const directives = join(node.directives, ' ');
const selectionSet = node.selectionSet;
return !name ? selectionSet :
return !name ? !directives && !defs && (!op || op === `query`) ?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leebyron thanks for the good catch - especially that query with directives/variable defs would ignore the artifacts and only show the selectionSet, treating it as a short-handed query. Added explicit checks for that exact condition, as well as tests for it.

leebyron added a commit that referenced this pull request Feb 9, 2016
Provides a correct OperationType without name in GraphQLPrinter
@leebyron leebyron merged commit 76e3ac1 into master Feb 9, 2016
@leebyron leebyron deleted the printer branch February 9, 2016 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants