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/src/__tests__/signature.test.ts b/packages/apollo-graphql/src/__tests__/signature.test.ts index 161e07ef30f..aae270731a0 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('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. { @@ -104,7 +88,129 @@ describe('aggressive signature', () => { jkl } `, - output: '{user{name...Bar}}fragment Bar on User{asd}', + output: 'fragment Bar on User{asd}{user{name...Bar}}', + }, + { + name: 'fragments out of order', + operationName: '', + input: gql` + fragment Baz on User { + jkl + } + + { + user { + name + ...Bar + ...Baz + } + } + + fragment Bar on User { + asd + } + `, + output: + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + '{user{name...Bar...Baz}}', + }, + { + name: 'fragments in order', + operationName: '', + input: gql` + { + user { + name + ...Bar + ...Baz + } + } + + fragment Bar on User { + asd + } + + fragment Baz on User { + jkl + } + `, + output: + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + '{user{name...Bar...Baz}}', + }, + { + name: 'with multiple operations (and no operation name specified)', + operationName: '', + input: gql` + fragment Bar on User { + asd + } + + fragment Baz on User { + jkl + } + + query C { + dfg + } + + query D { + dfg + } + + subscription A { + user { + name + ...Bar + ...Baz + } + } + + mutation B { + abc + } + `, + output: + '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: 'TheMutation', + input: gql` + mutation TheMutation { + user { + name + ...Bar + ...Baz + } + } + fragment Baz on User { + jkl + } + fragment Bar on User { + asd + } + `, + output: + 'fragment Bar on User{asd}fragment Baz on User{jkl}' + + 'mutation TheMutation{user{name...Bar...Baz}}', }, { name: 'full test', @@ -136,13 +242,15 @@ describe('aggressive signature', () => { } `, 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 }) => { test(name, () => { - expect(aggressive(input, operationName)).toEqual(output); + expect(defaultEngineReportingSignature(input, operationName)).toEqual( + 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); + }); + }); +}); 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 c8565f5c37f..b4b6ba3f7b5 100644 --- a/packages/apollo-graphql/src/transforms.ts +++ b/packages/apollo-graphql/src/transforms.ts @@ -16,11 +16,8 @@ import { } 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 @@ -82,25 +79,32 @@ 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; -} - -// 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 { +// 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 { 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 { 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; +} 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,