From 5c31fd325c72c645a4225c201d8a80c10038e839 Mon Sep 17 00:00:00 2001 From: Adrian Zgorzalek Date: Tue, 23 Oct 2018 14:58:27 +0100 Subject: [PATCH 1/2] Pretty format variable definitions for both operations and fragments. When there is more than a couple argument values, pretty formatting isn't doing a good job. Instead I would like to take the following approach: When there is one argument, print as is, meaning: query Q ($arg: Type = Default) { When there is more than one do the following: query Q ( $arg1: Type = Default, $arg2 ) { An alternative would be to have closing bracket be in the same line as final argument. I am fine with either, as changes which introduce another argument on last position always touch two lines - traiiling comma will have to be added. --- src/language/__tests__/printer-test.js | 18 ++++++++++++++++-- src/language/printer.js | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/language/__tests__/printer-test.js b/src/language/__tests__/printer-test.js index e03971132e..083c4c6863 100644 --- a/src/language/__tests__/printer-test.js +++ b/src/language/__tests__/printer-test.js @@ -157,11 +157,22 @@ describe('Printer: Query document', () => { fragment Foo($a: ComplexType, $b: Boolean = false) on TestType { id } + + fragment Simple($a: Boolean = true) on TestType { + id + } `, { experimentalFragmentVariables: true }, ); expect(print(fragmentWithVariable)).to.equal(dedent` - fragment Foo($a: ComplexType, $b: Boolean = false) on TestType { + fragment Foo( + $a: ComplexType, + $b: Boolean = false + ) on TestType { + id + } + + fragment Simple($a: Boolean = true) on TestType { id } `); @@ -174,7 +185,10 @@ describe('Printer: Query document', () => { expect(printed).to.equal( dedent(String.raw` - query queryName($foo: ComplexType, $site: Site = MOBILE) @onQuery { + query queryName( + $foo: ComplexType, + $site: Site = MOBILE + ) @onQuery { whoever123is: node(id: [123, 456]) { id ... on User @onInlineFragment { diff --git a/src/language/printer.js b/src/language/printer.js index 6ade5f6235..f3b397e50c 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -28,7 +28,7 @@ const printDocASTReducer = { OperationDefinition(node) { const op = node.operation; const name = node.name; - const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')'); + const varDefs = printVariableDefinitions(node.variableDefinitions); const directives = join(node.directives, ' '); const selectionSet = node.selectionSet; // Anonymous queries with no directives or variable definitions can use @@ -78,7 +78,7 @@ const printDocASTReducer = { }) => // Note: fragment variable definitions are experimental and may be changed // or removed in the future. - `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` + + `fragment ${name}${printVariableDefinitions(variableDefinitions)} ` + `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` + selectionSet, @@ -283,3 +283,17 @@ function printBlockString(value, isDescription) { ? `"""\n${isDescription ? escaped : indent(escaped)}\n"""` : `"""${escaped.replace(/"$/, '"\n')}"""`; } + +/** + * Prints variables one per line with a trailing comma, when there is more + * than one argument. Otherwise print inline. + */ +function printVariableDefinitions(args) { + if (!args || args.length === 0) { + return ''; + } + if (args.length === 1) { + return '(' + args[0] + ')'; + } + return '(\n' + indent(join(args, ',\n')) + '\n)'; +} From eb609150c6246fb5986f4a6ab15f1ccb72281819 Mon Sep 17 00:00:00 2001 From: Adrian Zgorzalek Date: Mon, 5 Nov 2018 17:09:00 +0100 Subject: [PATCH 2/2] Don't special case single argument and don't use to separate arguments --- src/language/__tests__/printer-test.js | 32 ++++++++++++++------------ src/language/printer.js | 8 ++----- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/language/__tests__/printer-test.js b/src/language/__tests__/printer-test.js index 083c4c6863..1f1f0186b9 100644 --- a/src/language/__tests__/printer-test.js +++ b/src/language/__tests__/printer-test.js @@ -60,7 +60,9 @@ describe('Printer: Query document', () => { 'query ($foo: TestType) @testDirective { id, name }', ); expect(print(queryAstWithArtifacts)).to.equal(dedent` - query ($foo: TestType) @testDirective { + query ( + $foo: TestType + ) @testDirective { id name } @@ -70,7 +72,9 @@ describe('Printer: Query document', () => { 'mutation ($foo: TestType) @testDirective { id, name }', ); expect(print(mutationAstWithArtifacts)).to.equal(dedent` - mutation ($foo: TestType) @testDirective { + mutation ( + $foo: TestType + ) @testDirective { id name } @@ -82,7 +86,9 @@ describe('Printer: Query document', () => { 'query ($foo: TestType = {a: 123} @testDirective(if: true) @test) { id }', ); expect(print(queryAstWithVariableDirective)).to.equal(dedent` - query ($foo: TestType = {a: 123} @testDirective(if: true) @test) { + query ( + $foo: TestType = {a: 123} @testDirective(if: true) @test + ) { id } `); @@ -96,7 +102,9 @@ describe('Printer: Query document', () => { }, ); expect(print(queryAstWithVariableDirective)).to.equal(dedent` - fragment Foo($foo: TestType @test) on TestType @testDirective { + fragment Foo( + $foo: TestType @test + ) on TestType @testDirective { id } `); @@ -157,24 +165,16 @@ describe('Printer: Query document', () => { fragment Foo($a: ComplexType, $b: Boolean = false) on TestType { id } - - fragment Simple($a: Boolean = true) on TestType { - id - } `, { experimentalFragmentVariables: true }, ); expect(print(fragmentWithVariable)).to.equal(dedent` fragment Foo( - $a: ComplexType, + $a: ComplexType $b: Boolean = false ) on TestType { id } - - fragment Simple($a: Boolean = true) on TestType { - id - } `); }); @@ -186,7 +186,7 @@ describe('Printer: Query document', () => { expect(printed).to.equal( dedent(String.raw` query queryName( - $foo: ComplexType, + $foo: ComplexType $site: Site = MOBILE ) @onQuery { whoever123is: node(id: [123, 456]) { @@ -217,7 +217,9 @@ describe('Printer: Query document', () => { } } - subscription StoryLikeSubscription($input: StoryLikeSubscribeInput) @onSubscription { + subscription StoryLikeSubscription( + $input: StoryLikeSubscribeInput + ) @onSubscription { storyLikeSubscribe(input: $input) { story { likers { diff --git a/src/language/printer.js b/src/language/printer.js index f3b397e50c..6fff2e9775 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -285,15 +285,11 @@ function printBlockString(value, isDescription) { } /** - * Prints variables one per line with a trailing comma, when there is more - * than one argument. Otherwise print inline. + * Prints variables one per line. */ function printVariableDefinitions(args) { if (!args || args.length === 0) { return ''; } - if (args.length === 1) { - return '(' + args[0] + ')'; - } - return '(\n' + indent(join(args, ',\n')) + '\n)'; + return '(\n' + indent(join(args, '\n')) + '\n)'; }