From 6f3313ed25ff73c3df677d39b601364369b65fbc Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 7 Feb 2019 15:19:46 -0800 Subject: [PATCH 01/10] Commit failing test --- .../src/__tests__/signature.test.ts | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/apollo-graphql/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index 161e07ef30f..560bf4652b7 100644 --- a/packages/apollo-graphql/src/__tests__/signature.test.ts +++ b/packages/apollo-graphql/src/__tests__/signature.test.ts @@ -13,7 +13,7 @@ import { // breaks if you turn it off in tests. disableFragmentWarnings(); -describe('aggressive signature', () => { +describe.only('aggressive signature', () => { function aggressive(ast: DocumentNode, operationName: string): string { return printWithReducedWhitespace( removeAliases( @@ -106,6 +106,29 @@ describe('aggressive signature', () => { `, output: '{user{name...Bar}}fragment Bar on User{asd}', }, + { + name: 'fragments out of order', + operationName: '', + input: gql` + fragment Bar on User { + asd + } + + { + user { + name + ...Bar + ...Baz + } + } + + fragment Baz on User { + jkl + } + `, + output: + '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', + }, { name: 'full test', operationName: 'Foo', From a494e948ad471da8de283b327d5283d2b9454ad8 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 7 Feb 2019 15:27:39 -0800 Subject: [PATCH 02/10] Add one more passing test case to show that order of fragments matters --- .../src/__tests__/signature.test.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/apollo-graphql/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index 560bf4652b7..b7c27f65cb8 100644 --- a/packages/apollo-graphql/src/__tests__/signature.test.ts +++ b/packages/apollo-graphql/src/__tests__/signature.test.ts @@ -129,6 +129,29 @@ describe.only('aggressive signature', () => { output: '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', }, + { + name: 'fragments in order', + operationName: '', + input: gql` + { + user { + name + ...Bar + ...Baz + } + } + + fragment Bar on User { + asd + } + + fragment Baz on User { + jkl + } + `, + output: + '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', + }, { name: 'full test', operationName: 'Foo', From ec695b926a64f467492b4ba15cf79443c57681e2 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 7 Feb 2019 17:45:38 -0800 Subject: [PATCH 03/10] Add top-level definition sorting to sortAST Sort document definitions array first by operation, followed by an alphabetized list of fragments. This change adds one more layer of "determinism" to how we normalize document signatures. This is critical for properly registering operations, as well as comparing incoming operations against the manifest. --- packages/apollo-graphql/src/transforms.ts | 27 ++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/apollo-graphql/src/transforms.ts b/packages/apollo-graphql/src/transforms.ts index c8565f5c37f..c7d7ee60df0 100644 --- a/packages/apollo-graphql/src/transforms.ts +++ b/packages/apollo-graphql/src/transforms.ts @@ -13,6 +13,7 @@ import { FragmentDefinitionNode, ObjectValueNode, ListValueNode, + ASTNode, } from 'graphql/language/ast'; import { print } from 'graphql/language/printer'; import { separateOperations } from 'graphql/utilities'; @@ -94,13 +95,37 @@ function sorted( return undefined; } +// Predicate for filtering and type checking operation definitions (query, mutation, subscription) +// Similar to graphql's isExecutableDefinitionNode +function isOperationDefinitionNode( + node: ASTNode, +): node is OperationDefinitionNode { + return node.kind === 'OperationDefinition'; +} + +// Predicate for filtering and type checking fragment definitions +// Similar to graphql's isExecutableDefinitionNode +function isFragmentDefinitionNode( + node: ASTNode, +): node is FragmentDefinitionNode { + return node.kind === 'FragmentDefinition'; +} + // sortAST sorts most multi-child nodes alphabetically. Using this as part of // your signature calculation function may make it easier to tell the difference // between queries that are similar to each other, and if for some reason your // GraphQL client generates query strings with elements in nondeterministic // order, it can make sure the queries are treated as identical. export function sortAST(ast: DocumentNode): DocumentNode { - return visit(ast, { + const [operation] = ast.definitions.filter(isOperationDefinitionNode); + const fragments = ast.definitions.filter(isFragmentDefinitionNode); + + const astWithSortedDefinitions = { + ...ast, + definitions: [operation, ...sortBy(fragments, 'name.value')], + }; + + return visit(astWithSortedDefinitions, { OperationDefinition( node: OperationDefinitionNode, ): OperationDefinitionNode { From 24dcd99e3a746dc43e82b72eb33e720c3aef89cf Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Thu, 7 Feb 2019 17:50:23 -0800 Subject: [PATCH 04/10] Add tests and fix broken example --- .../src/__tests__/signature.test.ts | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/apollo-graphql/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index b7c27f65cb8..6d68d4c3548 100644 --- a/packages/apollo-graphql/src/__tests__/signature.test.ts +++ b/packages/apollo-graphql/src/__tests__/signature.test.ts @@ -110,10 +110,29 @@ describe.only('aggressive signature', () => { name: 'fragments out of order', operationName: '', input: gql` + fragment Baz on User { + jkl + } + + { + user { + name + ...Bar + ...Baz + } + } + fragment Bar on User { asd } - + `, + output: + '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', + }, + { + name: 'fragments in order', + operationName: '', + input: gql` { user { name @@ -122,6 +141,10 @@ describe.only('aggressive signature', () => { } } + fragment Bar on User { + asd + } + fragment Baz on User { jkl } @@ -152,6 +175,52 @@ describe.only('aggressive signature', () => { output: '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', }, + { + name: 'with a subscription', + operationName: '', + input: gql` + fragment Bar on User { + asd + } + + fragment Baz on User { + jkl + } + + subscription A { + user { + name + ...Bar + ...Baz + } + } + `, + output: + 'subscription A{user{name...Bar...Baz}}' + + 'fragment Bar on User{asd}fragment Baz on User{jkl}', + }, + { + name: 'with a mutation', + operationName: '', + input: gql` + mutation A { + user { + name + ...Bar + ...Baz + } + } + fragment Baz on User { + jkl + } + fragment Bar on User { + asd + } + `, + output: + 'mutation A{user{name...Bar...Baz}}' + + 'fragment Bar on User{asd}fragment Baz on User{jkl}', + }, { name: 'full test', operationName: 'Foo', From 1b70cd365f07ca80d218738f9e6156e0c0390b84 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 15:55:57 -0800 Subject: [PATCH 05/10] Create utilities file, move / remove utilities. --- packages/apollo-graphql/src/transforms.ts | 6 +----- packages/apollo-graphql/src/utilities.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 5 deletions(-) create mode 100644 packages/apollo-graphql/src/utilities.ts diff --git a/packages/apollo-graphql/src/transforms.ts b/packages/apollo-graphql/src/transforms.ts index c7d7ee60df0..3998c1d56b7 100644 --- a/packages/apollo-graphql/src/transforms.ts +++ b/packages/apollo-graphql/src/transforms.ts @@ -13,15 +13,11 @@ import { FragmentDefinitionNode, ObjectValueNode, ListValueNode, - ASTNode, } from 'graphql/language/ast'; import { print } from 'graphql/language/printer'; import { separateOperations } from 'graphql/utilities'; -// We'll only fetch the `ListIteratee` type from the `@types/lodash`, but get -// `sortBy` from the modularized version of the package to avoid bringing in -// all of `lodash`. -import { ListIteratee } from 'lodash'; import sortBy from 'lodash.sortby'; +import { sorted } from './utilities'; // Replace numeric, string, list, and object literals with "empty" // values. Leaves enums alone (since there's no consistent "zero" enum). This diff --git a/packages/apollo-graphql/src/utilities.ts b/packages/apollo-graphql/src/utilities.ts new file mode 100644 index 00000000000..638363bd0dc --- /dev/null +++ b/packages/apollo-graphql/src/utilities.ts @@ -0,0 +1,17 @@ +// We'll only fetch the `ListIteratee` type from the `@types/lodash`, but get +// `sortBy` from the modularized version of the package to avoid bringing in +// all of `lodash`. +import { ListIteratee } from 'lodash'; +import sortBy from 'lodash.sortby'; + +// Like lodash's sortBy, but sorted(undefined) === undefined rather than []. It +// is a stable non-in-place sort. +export function sorted( + items: ReadonlyArray | undefined, + ...iteratees: Array> +): Array | undefined { + if (items) { + return sortBy(items, ...iteratees); + } + return undefined; +} From d1aeb3aeb75c2d47156ddc4e5a77e2a049d76a88 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 15:57:56 -0800 Subject: [PATCH 06/10] Update test to use defaultEngineSignature function instead of a slightly different equivalent (same functions, but composed differently). This is strictly for consistency - if the composition changes, this should be reflected in the tests. The output of defaultEngineSignature is what we're most concerned with. --- .../src/__tests__/signature.test.ts | 24 ++++--------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/packages/apollo-graphql/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index 6d68d4c3548..7fe5d42e0a6 100644 --- a/packages/apollo-graphql/src/__tests__/signature.test.ts +++ b/packages/apollo-graphql/src/__tests__/signature.test.ts @@ -1,27 +1,11 @@ -import { DocumentNode } from 'graphql'; import { default as gql, disableFragmentWarnings } from 'graphql-tag'; - -import { - printWithReducedWhitespace, - hideLiterals, - dropUnusedDefinitions, - sortAST, - removeAliases, -} from '../transforms'; +import { defaultEngineReportingSignature } from '../signature'; // The gql duplicate fragment warning feature really is just warnings; nothing // breaks if you turn it off in tests. disableFragmentWarnings(); -describe.only('aggressive signature', () => { - function aggressive(ast: DocumentNode, operationName: string): string { - return printWithReducedWhitespace( - removeAliases( - hideLiterals(sortAST(dropUnusedDefinitions(ast, operationName))), - ), - ); - } - +describe('defaultEngineReportingSignature', () => { const cases = [ // Test cases borrowed from optics-agent-js. { @@ -257,7 +241,9 @@ describe.only('aggressive signature', () => { ]; cases.forEach(({ name, operationName, input, output }) => { test(name, () => { - expect(aggressive(input, operationName)).toEqual(output); + expect(defaultEngineReportingSignature(input, operationName)).toEqual( + output, + ); }); }); }); From 2ad998b927ae2c1514514c536d65bfdd2b7b6e84 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 16:00:42 -0800 Subject: [PATCH 07/10] Rename sortAST -> lexicographicSortOperations Update sorting logic to sort by: 1) node type (operation / fragment) 2) operation type (query / mutation / subscription) 3) operation name 4) a stringified version of the node (covers edge case of anonymous operations) --- packages/apollo-graphql/src/signature.ts | 8 ++-- packages/apollo-graphql/src/transforms.ts | 57 ++++++++--------------- 2 files changed, 23 insertions(+), 42 deletions(-) diff --git a/packages/apollo-graphql/src/signature.ts b/packages/apollo-graphql/src/signature.ts index 8ddc71fe9f2..c1fe9444488 100644 --- a/packages/apollo-graphql/src/signature.ts +++ b/packages/apollo-graphql/src/signature.ts @@ -22,8 +22,8 @@ // - hideLiterals, which replaces all numeric and string literals as well // as list and object input values with "empty" values // - removeAliases, which removes field aliasing from the query -// - sortAST, which sorts the children of most multi-child nodes -// consistently +// - lexicographicSortOperations, which sorts the children of most multi-child +// nodes consistently // - printWithReducedWhitespace, a variant on graphql-js's 'print' // which gets rid of unneeded whitespace // @@ -49,7 +49,7 @@ import { printWithReducedWhitespace, dropUnusedDefinitions, removeAliases, - sortAST, + lexicographicSortOperations, hideLiterals, } from './transforms'; @@ -61,7 +61,7 @@ export function defaultEngineReportingSignature( operationName: string, ): string { return printWithReducedWhitespace( - sortAST( + lexicographicSortOperations( removeAliases(hideLiterals(dropUnusedDefinitions(ast, operationName))), ), ); diff --git a/packages/apollo-graphql/src/transforms.ts b/packages/apollo-graphql/src/transforms.ts index 3998c1d56b7..f0a2b590fda 100644 --- a/packages/apollo-graphql/src/transforms.ts +++ b/packages/apollo-graphql/src/transforms.ts @@ -79,46 +79,27 @@ export function dropUnusedDefinitions( return separated; } -// Like lodash's sortBy, but sorted(undefined) === undefined rather than []. It -// is a stable non-in-place sort. -function sorted( - items: ReadonlyArray | undefined, - ...iteratees: Array> -): Array | undefined { - if (items) { - return sortBy(items, ...iteratees); - } - return undefined; -} - -// Predicate for filtering and type checking operation definitions (query, mutation, subscription) -// Similar to graphql's isExecutableDefinitionNode -function isOperationDefinitionNode( - node: ASTNode, -): node is OperationDefinitionNode { - return node.kind === 'OperationDefinition'; -} - -// Predicate for filtering and type checking fragment definitions -// Similar to graphql's isExecutableDefinitionNode -function isFragmentDefinitionNode( - node: ASTNode, -): node is FragmentDefinitionNode { - return node.kind === 'FragmentDefinition'; -} - -// sortAST sorts most multi-child nodes alphabetically. Using this as part of -// your signature calculation function may make it easier to tell the difference -// between queries that are similar to each other, and if for some reason your -// GraphQL client generates query strings with elements in nondeterministic -// order, it can make sure the queries are treated as identical. -export function sortAST(ast: DocumentNode): DocumentNode { - const [operation] = ast.definitions.filter(isOperationDefinitionNode); - const fragments = ast.definitions.filter(isFragmentDefinitionNode); - +// lexicographicSortOperations sorts most multi-child nodes alphabetically. +// Using this as part of your signature calculation function may make it easier +// to tell the difference between queries that are similar to each other, and if +// for some reason your GraphQL client generates query strings with elements in +// nondeterministic order, it can make sure the queries are treated as identical. +export function lexicographicSortOperations(ast: DocumentNode): DocumentNode { + // Sort by kind (fragment/operation), operation type (query/mutation/subscription), then name. + // + // XXX + // The last sorting iteratee deterministically sorts unnamed operations. + // Caveat: The sorting value is just the stringified version of the non-sorted + // subtree. For our use case, this should be ok. const astWithSortedDefinitions = { ...ast, - definitions: [operation, ...sortBy(fragments, 'name.value')], + definitions: sortBy( + ast.definitions, + 'kind', + 'operation', + 'name.value', + value => JSON.stringify(value), + ), }; return visit(astWithSortedDefinitions, { From 602eb661032a10124908b13b4cd5eb60df703e62 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 16:02:55 -0800 Subject: [PATCH 08/10] Update and create new test cases --- .../src/__tests__/signature.test.ts | 71 ++++--- .../src/__tests__/transforms.test.ts | 188 +++++++++++++++++- 2 files changed, 226 insertions(+), 33 deletions(-) diff --git a/packages/apollo-graphql/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index 7fe5d42e0a6..aae270731a0 100644 --- a/packages/apollo-graphql/src/__tests__/signature.test.ts +++ b/packages/apollo-graphql/src/__tests__/signature.test.ts @@ -88,7 +88,7 @@ describe('defaultEngineReportingSignature', () => { jkl } `, - output: '{user{name...Bar}}fragment Bar on User{asd}', + output: 'fragment Bar on User{asd}{user{name...Bar}}', }, { name: 'fragments out of order', @@ -111,7 +111,8 @@ describe('defaultEngineReportingSignature', () => { } `, output: - '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + '{user{name...Bar...Baz}}', }, { name: 'fragments in order', @@ -134,20 +135,13 @@ describe('defaultEngineReportingSignature', () => { } `, output: - '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + '{user{name...Bar...Baz}}', }, { - name: 'fragments in order', + name: 'with multiple operations (and no operation name specified)', operationName: '', input: gql` - { - user { - name - ...Bar - ...Baz - } - } - fragment Bar on User { asd } @@ -155,20 +149,13 @@ describe('defaultEngineReportingSignature', () => { fragment Baz on User { jkl } - `, - output: - '{user{name...Bar...Baz}}fragment Bar on User{asd}fragment Baz on User{jkl}', - }, - { - name: 'with a subscription', - operationName: '', - input: gql` - fragment Bar on User { - asd + + query C { + dfg } - fragment Baz on User { - jkl + query D { + dfg } subscription A { @@ -178,16 +165,36 @@ describe('defaultEngineReportingSignature', () => { ...Baz } } + + mutation B { + abc + } `, output: - 'subscription A{user{name...Bar...Baz}}' + - 'fragment Bar on User{asd}fragment Baz on User{jkl}', + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + 'mutation B{abc}' + + 'query C{dfg}query D{dfg}' + + 'subscription A{user{name...Bar...Baz}}', + }, + { + name: 'with multiple queries, and one specified', + operationName: 'A', + input: gql` + { + asd + } + + query A { + jkl + } + `, + output: 'query A{jkl}', }, { name: 'with a mutation', - operationName: '', + operationName: 'TheMutation', input: gql` - mutation A { + mutation TheMutation { user { name ...Bar @@ -202,8 +209,8 @@ describe('defaultEngineReportingSignature', () => { } `, output: - 'mutation A{user{name...Bar...Baz}}' + - 'fragment Bar on User{asd}fragment Baz on User{jkl}', + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + 'mutation TheMutation{user{name...Bar...Baz}}', }, { name: 'full test', @@ -235,8 +242,8 @@ describe('defaultEngineReportingSignature', () => { } `, output: - 'query Foo($a:Boolean,$b:Int){user(age:0,name:""){name tz...Bar...on User{bee hello}}}' + - 'fragment Bar on User{age@skip(if:$a)...Nested}fragment Nested on User{blah}', + 'fragment Bar on User{age@skip(if:$a)...Nested}fragment Nested on User{blah}' + + 'query Foo($a:Boolean,$b:Int){user(age:0,name:""){name tz...Bar...on User{bee hello}}}', }, ]; cases.forEach(({ name, operationName, input, output }) => { diff --git a/packages/apollo-graphql/src/__tests__/transforms.test.ts b/packages/apollo-graphql/src/__tests__/transforms.test.ts index e0ad5e471d3..b8917973bd7 100644 --- a/packages/apollo-graphql/src/__tests__/transforms.test.ts +++ b/packages/apollo-graphql/src/__tests__/transforms.test.ts @@ -1,6 +1,11 @@ import { default as gql, disableFragmentWarnings } from 'graphql-tag'; -import { printWithReducedWhitespace, hideLiterals } from '../transforms'; +import { + printWithReducedWhitespace, + hideLiterals, + lexicographicSortOperations, + dropUnusedDefinitions, +} from '../transforms'; // The gql duplicate fragment warning feature really is just warnings; nothing // breaks if you turn it off in tests. @@ -75,3 +80,184 @@ describe('hideLiterals', () => { }); }); }); + +describe('dropUnusedDefinitions', () => { + const cases = [ + { + name: + 'demonstrates that separateOperations can only accommodate one anonymous operation', + operationName: '', + input: gql` + { + abc + } + { + def + } + `, + output: '{def}', + }, + { + name: + 'gets a document with a single, specified operation and only the fragments it needs', + operationName: 'TheMutation', + input: gql` + query IgnoreMe { + abc + ...DropThisFragment + } + + fragment DropThisFragment on SomeType { + def + } + + mutation TheMutation { + ...KeepThisFragment + } + + fragment KeepThisFragment on AnotherType { + abc + ...KeepThisNestedFragment + } + + fragment KeepThisNestedFragment on AnotherType { + ghi + } + `, + output: + 'mutation TheMutation{...KeepThisFragment}' + + 'fragment KeepThisFragment on AnotherType{abc...KeepThisNestedFragment}' + + 'fragment KeepThisNestedFragment on AnotherType{ghi}', + }, + ]; + + cases.forEach(({ name, operationName, input, output }) => { + test(name, () => { + expect( + printWithReducedWhitespace(dropUnusedDefinitions(input, operationName)), + ).toEqual(output); + }); + }); +}); + +describe('lexicographicSortOperations', () => { + const cases = [ + { + name: 'sorts fields with respect to each other', + input: gql` + { + def + abc { + z + a + } + } + `, + output: '{abc{a z}def}', + }, + { + name: + 'sorts nameless operations deterministically, relative to each other (pt. 1)', + input: gql` + { + abc + def + ghi + } + { + abc + } + + { + def + } + `, + output: '{abc def ghi}{abc}{def}', + }, + { + name: + 'sorts nameless operations deterministically, relative to each other (pt. 2)', + input: gql` + { + def + } + { + abc + def + ghi + } + { + abc + } + `, + output: '{abc def ghi}{abc}{def}', + }, + { + name: + 'sorts various (named / unnamed) operations and fragments, relative to each other (pt. 1)', + input: gql` + query Thing1 { + asdf + } + subscription Thing2 { + lkjkl + ...A + } + query { + fhg + ...A + } + fragment A on B { + abc + def + } + mutation { + abc + def + } + `, + output: + 'fragment A on B{abc def}' + + 'mutation{abc def}' + + 'query Thing1{asdf}{fhg...A}' + + 'subscription Thing2{lkjkl...A}', + }, + { + name: + 'sorts various (named / unnamed) operations and fragments, relative to each other (pt. 2)', + input: gql` + fragment A on B { + abc + def + } + query Thing1 { + asdf + } + mutation { + abc + def + } + subscription Thing2 { + lkjkl + ...A + } + query { + fhg + ...A + } + `, + output: + 'fragment A on B{abc def}' + + 'mutation{abc def}' + + 'query Thing1{asdf}{fhg...A}' + + 'subscription Thing2{lkjkl...A}', + }, + ]; + cases.forEach(({ name, input, output }) => { + test(name, () => { + expect( + printWithReducedWhitespace(lexicographicSortOperations(input)), + ).toEqual(output); + }); + }); +}); From 077e2c7621ea02a52ea3aa2a4f5416e5287829a0 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 16:15:53 -0800 Subject: [PATCH 09/10] Add no-focused-test tslint rule, added tslint config for apollo-graphql --- package-lock.json | 64 ++++++++++----- package.json | 1 + packages/apollo-engine-reporting/tslint.json | 4 + packages/apollo-graphql/tslint.json | 83 ++++++++++++++++++++ packages/graphql-extensions/tslint.json | 4 + 5 files changed, 137 insertions(+), 19 deletions(-) create mode 100644 packages/apollo-graphql/tslint.json diff --git a/package-lock.json b/package-lock.json index 943c5fbd54b..a54629fb597 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1384,14 +1384,6 @@ } } }, - "@types/koa-bodyparser": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/@types/koa-bodyparser/-/koa-bodyparser-5.0.1.tgz", - "integrity": "sha512-nL9JGPAHTnFb7zDm9u0bdXEMsbTgCJWcyHJRRPTSCkErzDSyeuSum7IemS697NTpWDwQw+Y9d/CYzvWKlAsMRQ==", - "requires": { - "@types/koa": "*" - } - }, "@types/koa-compose": { "version": "3.2.2", "resolved": "https://registry.npmjs.org/@types/koa-compose/-/koa-compose-3.2.2.tgz", @@ -2309,6 +2301,15 @@ "koa-bodyparser": "^3.0.0", "koa-router": "^7.4.0", "type-is": "^1.6.16" + }, + "dependencies": { + "@types/koa-bodyparser": { + "version": "4.2.1", + "bundled": true, + "requires": { + "@types/koa": "*" + } + } } }, "apollo-server-lambda": { @@ -5450,7 +5451,8 @@ "version": "2.1.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-2.1.1.tgz", "integrity": "sha1-w7M6te42DYbg5ijwRorn7yfWVN8=", - "dev": true + "dev": true, + "optional": true }, "aproba": { "version": "1.2.0", @@ -5474,13 +5476,15 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz", "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c=", - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz", "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==", "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -5497,19 +5501,22 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/code-point-at/-/code-point-at-1.1.0.tgz", "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz", "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=", - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/console-control-strings/-/console-control-strings-1.1.0.tgz", "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -5640,7 +5647,8 @@ "version": "2.0.3", "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.3.tgz", "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -5654,6 +5662,7 @@ "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz", "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -5670,6 +5679,7 @@ "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.0.4.tgz", "integrity": "sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==", "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -5678,13 +5688,15 @@ "version": "0.0.8", "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=", - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.2.4", "resolved": "https://registry.npmjs.org/minipass/-/minipass-2.2.4.tgz", "integrity": "sha512-hzXIWWet/BzWhYs2b+u7dRHlruXhwdgvlTMDKC6Cb1U7ps6Ac6yQlR39xsbjWJE377YTCtKwIXIpJ5oP+j5y8g==", "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.1", "yallist": "^3.0.0" @@ -5705,6 +5717,7 @@ "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -5793,7 +5806,8 @@ "version": "1.0.1", "resolved": "https://registry.npmjs.org/number-is-nan/-/number-is-nan-1.0.1.tgz", "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -5807,6 +5821,7 @@ "resolved": "https://registry.npmjs.org/once/-/once-1.4.0.tgz", "integrity": "sha1-WDsap3WWHUsROsF9nFC6753Xa9E=", "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -5902,7 +5917,8 @@ "version": "5.1.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.1.tgz", "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==", - "dev": true + "dev": true, + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -5944,6 +5960,7 @@ "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -5965,6 +5982,7 @@ "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "dev": true, + "optional": true, "requires": { "ansi-regex": "^2.0.0" } @@ -6013,13 +6031,15 @@ "version": "1.0.2", "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", - "dev": true + "dev": true, + "optional": true }, "yallist": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/yallist/-/yallist-3.0.2.tgz", "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=", - "dev": true + "dev": true, + "optional": true } } }, @@ -13019,6 +13039,12 @@ } } }, + "tslint-no-focused-test": { + "version": "0.5.0", + "resolved": "https://registry.npmjs.org/tslint-no-focused-test/-/tslint-no-focused-test-0.5.0.tgz", + "integrity": "sha512-YK0PSY5XAdJaTzVIXxnUGyvB5VAi+H9yTc3e40YVtu8Ix3+zLSz4ufvX6rXT3nWpim0DR6fxXoL/Zk8JI641Vg==", + "dev": true + }, "tsutils": { "version": "2.29.0", "resolved": "https://registry.npmjs.org/tsutils/-/tsutils-2.29.0.tgz", diff --git a/package.json b/package.json index 443d9a554cd..aeeba743ee6 100644 --- a/package.json +++ b/package.json @@ -128,6 +128,7 @@ "test-listen": "1.1.0", "ts-jest": "23.10.5", "tslint": "5.12.1", + "tslint-no-focused-test": "^0.5.0", "typescript": "3.3.1", "ws": "6.1.3", "yup": "0.26.10" diff --git a/packages/apollo-engine-reporting/tslint.json b/packages/apollo-engine-reporting/tslint.json index fff5bbb3192..437dd051ea7 100644 --- a/packages/apollo-engine-reporting/tslint.json +++ b/packages/apollo-engine-reporting/tslint.json @@ -1,4 +1,7 @@ { + "rulesDirectory": [ + "tslint-no-focused-test" + ], "rules": { "ban": false, "class-name": true, @@ -43,6 +46,7 @@ "no-duplicate-variable": true, "no-empty": true, "no-eval": true, + "no-focused-test": true, "no-inferrable-types": false, "no-internal-module": true, "no-null-keyword": false, diff --git a/packages/apollo-graphql/tslint.json b/packages/apollo-graphql/tslint.json new file mode 100644 index 00000000000..437dd051ea7 --- /dev/null +++ b/packages/apollo-graphql/tslint.json @@ -0,0 +1,83 @@ +{ + "rulesDirectory": [ + "tslint-no-focused-test" + ], + "rules": { + "ban": false, + "class-name": true, + "eofline": true, + "forin": true, + "interface-name": [ + true, + "never-prefix" + ], + "jsdoc-format": true, + "label-position": true, + "member-access": true, + "member-ordering": [ + true, + { + "order": [ + "static-field", + "instance-field", + "constructor", + "public-instance-method", + "protected-instance-method", + "private-instance-method" + ] + } + ], + "no-any": false, + "no-arg": true, + "no-bitwise": true, + "no-conditional-assignment": true, + "no-consecutive-blank-lines": false, + "no-console": [ + true, + "log", + "debug", + "info", + "time", + "timeEnd", + "trace" + ], + "no-construct": true, + "no-debugger": true, + "no-duplicate-variable": true, + "no-empty": true, + "no-eval": true, + "no-focused-test": true, + "no-inferrable-types": false, + "no-internal-module": true, + "no-null-keyword": false, + "no-require-imports": false, + "no-shadowed-variable": true, + "no-switch-case-fall-through": true, + "no-trailing-whitespace": true, + "no-unused-expression": true, + "no-var-keyword": true, + "no-var-requires": true, + "object-literal-sort-keys": false, + "radix": true, + "switch-default": true, + "triple-equals": [ + true, + "allow-null-check" + ], + "typedef": [ + false, + "call-signature", + "parameter", + "arrow-parameter", + "property-declaration", + "variable-declaration", + "member-variable-declaration" + ], + "variable-name": [ + true, + "check-format", + "allow-leading-underscore", + "ban-keywords" + ] + } +} diff --git a/packages/graphql-extensions/tslint.json b/packages/graphql-extensions/tslint.json index fff5bbb3192..437dd051ea7 100644 --- a/packages/graphql-extensions/tslint.json +++ b/packages/graphql-extensions/tslint.json @@ -1,4 +1,7 @@ { + "rulesDirectory": [ + "tslint-no-focused-test" + ], "rules": { "ban": false, "class-name": true, @@ -43,6 +46,7 @@ "no-duplicate-variable": true, "no-empty": true, "no-eval": true, + "no-focused-test": true, "no-inferrable-types": false, "no-internal-module": true, "no-null-keyword": false, From d341e08691879704c429dea42da0d6f9220efec3 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 8 Feb 2019 16:46:50 -0800 Subject: [PATCH 10/10] Leverage the visitor for sorting top level nodes as well --- packages/apollo-graphql/src/transforms.ts | 38 ++++++++++++----------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/apollo-graphql/src/transforms.ts b/packages/apollo-graphql/src/transforms.ts index f0a2b590fda..b4b6ba3f7b5 100644 --- a/packages/apollo-graphql/src/transforms.ts +++ b/packages/apollo-graphql/src/transforms.ts @@ -85,24 +85,26 @@ export function dropUnusedDefinitions( // for some reason your GraphQL client generates query strings with elements in // nondeterministic order, it can make sure the queries are treated as identical. export function lexicographicSortOperations(ast: DocumentNode): DocumentNode { - // Sort by kind (fragment/operation), operation type (query/mutation/subscription), then name. - // - // XXX - // The last sorting iteratee deterministically sorts unnamed operations. - // Caveat: The sorting value is just the stringified version of the non-sorted - // subtree. For our use case, this should be ok. - const astWithSortedDefinitions = { - ...ast, - definitions: sortBy( - ast.definitions, - 'kind', - 'operation', - 'name.value', - value => JSON.stringify(value), - ), - }; - - return visit(astWithSortedDefinitions, { + return visit(ast, { + Document(node: DocumentNode): DocumentNode { + // Sort by kind (fragment/operation), operation type (query/mutation/subscription), then name. + // + // XXX + // The last sorting iteratee deterministically sorts unnamed operations. + // Caveat: The sorting value is just the stringified version of the non-sorted + // subtree. For our use case, this should be ok. + return { + ...node, + // Use sortBy because 'definitions' is not optional. + definitions: sortBy( + ast.definitions, + 'kind', + 'operation', + 'name.value', + definition => JSON.stringify(definition), + ), + }; + }, OperationDefinition( node: OperationDefinitionNode, ): OperationDefinitionNode {