From e8482a2807a2240ad1a6e9fa262ce5eeb7fd8274 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Wed, 12 Dec 2018 18:42:53 +0000 Subject: [PATCH 01/25] chore(deps): update dependency @types/graphql to v14 --- package-lock.json | 14 +++++++------- package.json | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/package-lock.json b/package-lock.json index e3d33e4db95..c119859f4d1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1409,9 +1409,9 @@ "dev": true }, "@types/graphql": { - "version": "0.12.7", - "resolved": "https://registry.npmjs.org/@types/graphql/-/graphql-0.12.7.tgz", - "integrity": "sha512-xuf/9ShwOOlxoV+J5ts4vAoPbL3nmcFU3z/Ad6jrsiB99hB/Wrs2fl3qJga5XJ1euAasMN/FsNPdHMV6+OV5vw==", + "version": "14.0.3", + "resolved": "https://registry.npmjs.org/@types/graphql/-/graphql-14.0.3.tgz", + "integrity": "sha512-TcFkpEjcQK7w8OcrQcd7iIBPjU0rdyi3ldj6d0iJ4PPSzbWqPBvXj9KSwO14hTOX2dm9RoiH7VuxksJLNYdXUQ==", "dev": true }, "@types/isomorphic-fetch": { @@ -6486,12 +6486,12 @@ "dev": true }, "graphql": { - "version": "0.13.2", - "resolved": "https://registry.npmjs.org/graphql/-/graphql-0.13.2.tgz", - "integrity": "sha512-QZ5BL8ZO/B20VA8APauGBg3GyEgZ19eduvpLWoq5x7gMmWnHoy8rlQWPLmWgFvo1yNgjSEFMesmS4R6pPr7xog==", + "version": "14.0.2", + "resolved": "https://registry.npmjs.org/graphql/-/graphql-14.0.2.tgz", + "integrity": "sha512-gUC4YYsaiSJT1h40krG3J+USGlwhzNTXSb4IOZljn9ag5Tj+RkoXrWp+Kh7WyE3t1NCfab5kzCuxBIvOMERMXw==", "dev": true, "requires": { - "iterall": "^1.2.1" + "iterall": "^1.2.2" } }, "graphql-anywhere": { diff --git a/package.json b/package.json index 77ebd8b095e..9c8c0ddd231 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,7 @@ "devDependencies": { "@octokit/rest": "15.18.0", "@types/benchmark": "1.0.31", - "@types/graphql": "0.12.7", + "@types/graphql": "14.0.3", "@types/isomorphic-fetch": "0.0.34", "@types/jest": "23.3.10", "@types/lodash": "4.14.119", @@ -105,7 +105,7 @@ "danger": "4.4.10", "fetch-mock": "7.2.5", "flow-bin": "0.88.0", - "graphql": "0.13.2", + "graphql": "14.0.2", "graphql-tag": "2.10.0", "isomorphic-fetch": "2.2.1", "jest": "23.6.0", From d81f876b177e85eccda31b6ba5ebbe7c95a801a9 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 17 Dec 2018 11:20:59 -0500 Subject: [PATCH 02/25] Set new selection during FieldNode creation This will help avoid typescript errors when using `graphql` 14.x. --- packages/apollo-cache/src/utils.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/apollo-cache/src/utils.ts b/packages/apollo-cache/src/utils.ts index d22b17407a0..7d88d0b9315 100644 --- a/packages/apollo-cache/src/utils.ts +++ b/packages/apollo-cache/src/utils.ts @@ -71,21 +71,17 @@ function selectionSetFromObj(obj: any): SelectionSetNode { const selections: FieldNode[] = []; Object.keys(obj).forEach(key => { + const nestedSelSet: SelectionSetNode = selectionSetFromObj(obj[key]); + const field: FieldNode = { kind: 'Field', name: { kind: 'Name', value: key, }, + selectionSet: nestedSelSet || undefined, }; - // Recurse - const nestedSelSet: SelectionSetNode = selectionSetFromObj(obj[key]); - - if (nestedSelSet) { - field.selectionSet = nestedSelSet; - } - selections.push(field); }); From 76ff754e9315576072869558b89b347d0e9334a4 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 17 Dec 2018 11:23:27 -0500 Subject: [PATCH 03/25] Convert `GraphQLError[]` use to `ReadonlyArray` Required by `graphql` 14.x, unless of course we want to parse the incoming read only array, and convert it into a `GraphQLError` array. If keeping things aligned with `graphql` (by using a read only error array) proves to be problematic, especially with regards to backwards compatibility, we might have to change this. --- .../apollo-client/src/core/ObservableQuery.ts | 24 +++++++++++-------- packages/apollo-client/src/core/types.ts | 2 +- packages/apollo-client/src/data/queries.ts | 4 ++-- .../apollo-client/src/errors/ApolloError.ts | 4 ++-- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/apollo-client/src/core/ObservableQuery.ts b/packages/apollo-client/src/core/ObservableQuery.ts index 16286a41711..ad43ceee3c4 100644 --- a/packages/apollo-client/src/core/ObservableQuery.ts +++ b/packages/apollo-client/src/core/ObservableQuery.ts @@ -22,7 +22,7 @@ import { QueryStoreValue } from '../data/queries'; export type ApolloCurrentResult = { data: T | {}; - errors?: GraphQLError[]; + errors?: ReadonlyArray; loading: boolean; networkStatus: NetworkStatus; error?: ApolloError; @@ -228,7 +228,8 @@ export class ObservableQuery< public isDifferentFromLastResult(newResult: ApolloQueryResult) { const { lastResultSnapshot: snapshot } = this; return !( - snapshot && newResult && + snapshot && + newResult && snapshot.networkStatus === newResult.networkStatus && snapshot.stale === newResult.stale && isEqual(snapshot.data, newResult.data) @@ -356,7 +357,9 @@ export class ObservableQuery< // XXX the subscription variables are separate from the query variables. // if you want to update subscription variables, right now you have to do that separately, // and you can only do it by stopping the subscription and then subscribing again with new variables. - public subscribeToMore(options: SubscribeToMoreOptions) { + public subscribeToMore( + options: SubscribeToMoreOptions, + ) { const subscription = this.queryManager .startGraphQLSubscription({ query: options.document, @@ -366,13 +369,14 @@ export class ObservableQuery< next: (subscriptionData: { data: TSubscriptionData }) => { if (options.updateQuery) { this.updateQuery((previous, { variables }) => - (options.updateQuery as UpdateQueryFn)( - previous, - { - subscriptionData, - variables, - }, - ), + (options.updateQuery as UpdateQueryFn< + TData, + TVariables, + TSubscriptionData + >)(previous, { + subscriptionData, + variables, + }), ); } }, diff --git a/packages/apollo-client/src/core/types.ts b/packages/apollo-client/src/core/types.ts index bda09331a3d..4df7e21bf9c 100644 --- a/packages/apollo-client/src/core/types.ts +++ b/packages/apollo-client/src/core/types.ts @@ -18,7 +18,7 @@ export type PureQueryOptions = { export type ApolloQueryResult = { data: T; - errors?: GraphQLError[]; + errors?: ReadonlyArray; loading: boolean; networkStatus: NetworkStatus; stale: boolean; diff --git a/packages/apollo-client/src/data/queries.ts b/packages/apollo-client/src/data/queries.ts index f7a3134988f..9e120f4c18c 100644 --- a/packages/apollo-client/src/data/queries.ts +++ b/packages/apollo-client/src/data/queries.ts @@ -10,7 +10,7 @@ export type QueryStoreValue = { previousVariables?: Object | null; networkStatus: NetworkStatus; networkError?: Error | null; - graphQLErrors?: GraphQLError[]; + graphQLErrors?: ReadonlyArray; metadata: any; }; @@ -78,7 +78,7 @@ export class QueryStore { networkStatus = NetworkStatus.loading; } - let graphQLErrors: GraphQLError[] = []; + let graphQLErrors: ReadonlyArray = []; if (previousQuery && previousQuery.graphQLErrors) { graphQLErrors = previousQuery.graphQLErrors; } diff --git a/packages/apollo-client/src/errors/ApolloError.ts b/packages/apollo-client/src/errors/ApolloError.ts index 46979df234b..396202238b8 100644 --- a/packages/apollo-client/src/errors/ApolloError.ts +++ b/packages/apollo-client/src/errors/ApolloError.ts @@ -31,7 +31,7 @@ const generateErrorMessage = (err: ApolloError) => { export class ApolloError extends Error { public message: string; - public graphQLErrors: GraphQLError[]; + public graphQLErrors: ReadonlyArray; public networkError: Error | null; // An object that can be used to provide some additional information @@ -48,7 +48,7 @@ export class ApolloError extends Error { errorMessage, extraInfo, }: { - graphQLErrors?: GraphQLError[]; + graphQLErrors?: ReadonlyArray; networkError?: Error | null; errorMessage?: string; extraInfo?: any; From 61fa175bf5390982fb20f2dd38fa86e1768d0148 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 17 Dec 2018 11:27:00 -0500 Subject: [PATCH 04/25] Add `es7` lib to the typescript config We're using a few es7 level functions, like `Array.prototype.includes`. --- packages/apollo-utilities/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-utilities/tsconfig.json b/packages/apollo-utilities/tsconfig.json index 5ec9779f580..e30d7a994fa 100644 --- a/packages/apollo-utilities/tsconfig.json +++ b/packages/apollo-utilities/tsconfig.json @@ -3,7 +3,7 @@ "compilerOptions": { "rootDir": "src", "outDir": "lib", - "lib": ["es6", "dom", "es2017.object"] + "lib": ["es6", "dom", "es7"] }, "include": ["src/**/*.ts"], "exclude": ["src/**/__tests__/*.ts"] From 516413446623b5b6452583b34e0f7960faddc92b Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Mon, 17 Dec 2018 11:29:52 -0500 Subject: [PATCH 05/25] Rewrite transformation utils that were writing into the graphql AST `graphql` 14.x has made most parts of the AST readonly, which means we can't upgrade Apollo Client to use `graphql` 14.x, until the `apollo-utilities` transformation helpers are modified to stop writing back into the original AST. This commit replaces all cloning / re-writing with a node visitor approach, provided by `graphql`'s `visit` function. --- .../src/__tests__/transform.ts | 6 +- packages/apollo-utilities/src/transform.ts | 712 +++++++----------- 2 files changed, 259 insertions(+), 459 deletions(-) diff --git a/packages/apollo-utilities/src/__tests__/transform.ts b/packages/apollo-utilities/src/__tests__/transform.ts index 3f8a423dd65..71a375632a0 100644 --- a/packages/apollo-utilities/src/__tests__/transform.ts +++ b/packages/apollo-utilities/src/__tests__/transform.ts @@ -534,7 +534,7 @@ describe('query transforms', () => { `; const expectedQueryStr = print(expectedQuery); - expect(expectedQueryStr).toBe(print(newQueryDoc)); + expect(print(newQueryDoc)).toBe(expectedQueryStr); }); it('should not add duplicates', () => { @@ -565,7 +565,7 @@ describe('query transforms', () => { `; const expectedQueryStr = print(expectedQuery); - expect(expectedQueryStr).toBe(print(newQueryDoc)); + expect(print(newQueryDoc)).toBe(expectedQueryStr); }); it('should not screw up on a FragmentSpread within the query AST', () => { @@ -1167,8 +1167,6 @@ describe('getDirectivesFromDocument', () => { const expected = gql` fragment client on ClientData { hi @client - bye @storage - bar } query Mixed { diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index efe9db06846..cbead229cca 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -2,7 +2,6 @@ import { DocumentNode, SelectionNode, SelectionSetNode, - DefinitionNode, OperationDefinitionNode, FieldNode, DirectiveNode, @@ -11,10 +10,9 @@ import { FragmentSpreadNode, VariableDefinitionNode, VariableNode, + visit, } from 'graphql'; -import { cloneDeep } from './util/cloneDeep'; - import { checkDocument, getOperationDefinitionOrDie, @@ -89,243 +87,179 @@ function getDirectiveMatcher( }; } -function addTypenameToSelectionSet( - selectionSet: SelectionSetNode, - isRoot = false, -) { - if (selectionSet.selections) { - if (!isRoot) { - const alreadyHasThisField = selectionSet.selections.some(selection => { - return ( - selection.kind === 'Field' && - (selection as FieldNode).name.value === '__typename' - ); - }); - - if (!alreadyHasThisField) { - selectionSet.selections.push(TYPENAME_FIELD); - } - } - - selectionSet.selections.forEach(selection => { - // Must not add __typename if we're inside an introspection query - if (selection.kind === 'Field') { - if ( - selection.name.value.lastIndexOf('__', 0) !== 0 && - selection.selectionSet - ) { - addTypenameToSelectionSet(selection.selectionSet); - } - } else if (selection.kind === 'InlineFragment') { - if (selection.selectionSet) { - addTypenameToSelectionSet(selection.selectionSet); +export function removeDirectivesFromDocument( + directives: RemoveDirectiveConfig[], + doc: DocumentNode, +): DocumentNode | null { + let variablesInUse: string[] = []; + let variablesToRemove: RemoveArgumentsConfig[] = []; + + let fragmentSpreadsInUse: string[] = []; + let fragmentSpreadsToRemove: RemoveFragmentSpreadConfig[] = []; + + let modifiedDoc = visit(doc, { + Variable: { + enter(node, _, parent) { + // Store each variable that's referenced as part of an argument + // (excluding operation definition variables), so we know which + // variables are being used. If we later want to remove a variable + // we'll fist check to see if it's being used, before continuing with + // the removal. + if ((parent as VariableDefinitionNode).kind !== 'VariableDefinition') { + variablesInUse.push(node.name.value); } - } - }); - } -} - -function getSelectionsMatchingDirectiveFromSelectionSet( - directives: GetDirectiveConfig[], - selectionSet: SelectionSetNode, - invert: boolean = false, - fieldsOnly: boolean = false, -): SelectionNode[] { - return selectionSet.selections - .map(selection => { - if ( - selection.kind !== 'Field' || - !(selection as FieldNode) || - !selection.directives - ) { - return fieldsOnly ? null : selection; - } + }, + }, - let isMatch: boolean; - const directiveMatcher = getDirectiveMatcher(directives); - selection.directives = selection.directives.filter(directive => { - const shouldKeep = !directiveMatcher(directive); + Field: { + enter(node) { + // If `remove` is set to true for a directive, and a directive match + // is found for a field, remove the field as well. + const shouldRemoveField = directives.some( + directive => directive.remove, + ); - if (!isMatch && !shouldKeep) { - isMatch = true; + if (shouldRemoveField) { + const matcher = getDirectiveMatcher(directives); + let removeField = false; + node.directives.forEach(directive => { + if (matcher(directive)) { + removeField = true; + return; + } + }); + + if (removeField) { + if (node.arguments) { + // Store field argument variables so they can be removed + // from the operation definition. + variablesToRemove = variablesToRemove.concat( + node.arguments + .filter(arg => arg.value.kind === 'Variable') + .map(arg => ({ + name: (arg.value as VariableNode).name.value, + })), + ); + } + + if (node.selectionSet) { + // Store fragment spread names so they can be removed from the + // docuemnt. + const fragSpreads = getAllFragmentSpreadsFromSelectionSet( + node.selectionSet, + ); + fragmentSpreadsToRemove = fragmentSpreadsToRemove.concat( + fragSpreads.map(frag => ({ + name: frag.name.value, + })), + ); + } + + // Remove the field. + return null; + } } + }, + }, - return shouldKeep; - }); - - return isMatch && invert ? null : selection; - }) - .filter(s => !!s); -} - -function removeDirectivesFromSelectionSet( - directives: RemoveDirectiveConfig[], - selectionSet: SelectionSetNode, -): SelectionSetNode { - if (!selectionSet.selections) return selectionSet; - // if any of the directives are set to remove this selectionSet, remove it - const agressiveRemove = directives.some( - (dir: RemoveDirectiveConfig) => dir.remove, - ); - - selectionSet.selections = getSelectionsMatchingDirectiveFromSelectionSet( - directives, - selectionSet, - agressiveRemove, - ); + FragmentSpread: { + enter(node) { + // Keep track of referenced fragment spreads. This is used to + // determine if top level fragment definitions should be removed. + fragmentSpreadsInUse.push(node.name.value); + }, + }, - selectionSet.selections.forEach(selection => { - if ( - (selection.kind === 'Field' || selection.kind === 'InlineFragment') && - selection.selectionSet - ) { - removeDirectivesFromSelectionSet(directives, selection.selectionSet); - } + Directive: { + enter(node) { + // If a matching directive is found, remove it. + const directiveFound = directives.some(directive => { + if (directive.name && directive.name === node.name.value) return true; + if (directive.test && directive.test(node)) return true; + return false; + }); + if (directiveFound) { + // Remove the directive. + return null; + } + }, + }, }); - return selectionSet; -} - -export function removeDirectivesFromDocument( - directives: RemoveDirectiveConfig[], - doc: DocumentNode, -): DocumentNode | null { - let docClone = cloneDeep(doc); - let removedArguments: RemoveArgumentsConfig[] = []; - let removedFragments: RemoveFragmentSpreadConfig[] = []; - const aggressiveRemove = directives.some(directive => directive.remove); - - docClone.definitions.forEach((definition: DefinitionNode) => { - const operationDefinition = definition as OperationDefinitionNode; - const originalSelectionSet = cloneDeep(operationDefinition.selectionSet); - - const newSelectionSet = removeDirectivesFromSelectionSet( - directives, - operationDefinition.selectionSet, + // If we've removed fields with arguments, make sure the associated + // variables are also removed from the rest of the document, as long as they + // aren't being used elsewhere. + if (variablesToRemove) { + variablesToRemove = variablesToRemove.filter( + variable => !variablesInUse.includes(variable.name), ); - - if (aggressiveRemove && !!docClone) { - const matchingSelections = getSelectionsMatchingDirectiveFromSelectionSet( - directives.map(config => ({ - name: config.name, - test: config.test, - })), - originalSelectionSet, - ); - - const remainingArguments = getAllArgumentsFromSelectionSet( - newSelectionSet, - ); - - removedArguments = [ - ...removedArguments, - ...matchingSelections - .map(getAllArgumentsFromSelection) - .reduce( - (allArguments, selectionArguments) => [ - ...allArguments, - ...selectionArguments, - ], - [], - ) - .filter( - removedArg => - !remainingArguments.some(remainingArg => { - if ( - remainingArg.value.kind !== 'Variable' || - !(remainingArg.value as VariableNode) - ) - return false; - if ( - removedArg.value.kind !== 'Variable' || - !(removedArg.value as VariableNode) - ) - return false; - return ( - remainingArg.value.name.value === removedArg.value.name.value - ); - }), - ) - .map(argument => { - if ( - argument.value.kind !== 'Variable' || - !(argument.value as VariableNode) - ) - return null; - return { - name: argument.value.name.value, - remove: aggressiveRemove, - }; - }) - .filter(node => !!node), - ]; - - const remainingFragmentSpreads = getAllFragmentSpreadsFromSelectionSet( - newSelectionSet, - ); - - removedFragments = [ - ...removedFragments, - ...matchingSelections - .map(getAllFragmentSpreadsFromSelection) - .reduce( - (allFragments, selectionFragments) => [ - ...allFragments, - ...selectionFragments, - ], - [], - ) - .filter( - removedFragment => - !remainingFragmentSpreads.some( - remainingFragment => - remainingFragment.name.value === removedFragment.name.value, - ), - ) - .map(fragment => ({ - name: fragment.name.value, - remove: aggressiveRemove, - })), - ]; - } - }); - - if (!docClone) { - return null; + modifiedDoc = removeArgumentsFromDocument(variablesToRemove, modifiedDoc); } - if (removedFragments.length > 0) { - docClone = removeFragmentSpreadFromDocument(removedFragments, docClone); - if (!docClone) { - return null; - } - } - - if (removedArguments.length > 0) { - docClone = removeArgumentsFromDocument(removedArguments, docClone); - if (!docClone) { - return null; - } + // If we've removed selection sets with fragment spreads, make sure the + // associated fragment definitions are also removed from the rest of the + // document, as long as they aren't being used elsewhere. + if (fragmentSpreadsToRemove) { + fragmentSpreadsToRemove = fragmentSpreadsToRemove.filter( + fragSpread => !fragmentSpreadsInUse.includes(fragSpread.name), + ); + modifiedDoc = removeFragmentSpreadFromDocument( + fragmentSpreadsToRemove, + modifiedDoc, + ); } - const operation = getOperationDefinitionOrDie(docClone); - const fragments = createFragmentMap(getFragmentDefinitions(docClone)); - - return isNotEmpty(operation, fragments) ? docClone : null; + return modifiedDoc; } export function addTypenameToDocument(doc: DocumentNode) { checkDocument(doc); - const docClone = cloneDeep(doc); - docClone.definitions.forEach((definition: DefinitionNode) => { - const isRoot = definition.kind === 'OperationDefinition'; - addTypenameToSelectionSet( - (definition as OperationDefinitionNode).selectionSet, - isRoot, - ); + let isRoot = false; + const modifiedDoc = visit(doc, { + OperationDefinition: { + enter(node) { + isRoot = true; + }, + }, + + SelectionSet: { + enter(node) { + // Don't add __typename to OperationDefinitions. + if (isRoot) { + isRoot = false; + return undefined; + } + + // No changes if no selections. + const { selections } = node; + if (!selections) { + return undefined; + } + + // If selections already have a __typename, or are part of an + // introspection query, do nothing. + const skip = selections.some(selection => { + return ( + selection.kind === 'Field' && + ((selection as FieldNode).name.value === '__typename' || + (selection as FieldNode).name.value.lastIndexOf('__', 0) === 0) + ); + }); + if (skip) { + return undefined; + } + + // Create and return a new SelectionSet with a __typename Field. + return { + ...node, + selections: [...selections, TYPENAME_FIELD], + }; + }, + }, }); - return docClone; + + return modifiedDoc; } const connectionRemoveConfig = { @@ -389,57 +323,47 @@ function hasDirectivesInSelection( ); } -function getDirectivesFromSelectionSet( - directives: GetDirectiveConfig[], - selectionSet: SelectionSetNode, -) { - selectionSet.selections = selectionSet.selections - .filter(selection => { - return hasDirectivesInSelection(directives, selection, true); - }) - .map(selection => { - if (hasDirectivesInSelection(directives, selection, false)) { - return selection; - } - if ( - (selection.kind === 'Field' || selection.kind === 'InlineFragment') && - selection.selectionSet - ) { - selection.selectionSet = getDirectivesFromSelectionSet( - directives, - selection.selectionSet, - ); - } - - return selection; - }); - return selectionSet; -} - export function getDirectivesFromDocument( directives: GetDirectiveConfig[], doc: DocumentNode, - includeAllFragments = false, ): DocumentNode | null { checkDocument(doc); - const docClone = cloneDeep(doc); - docClone.definitions = docClone.definitions.map(definition => { - if ( - (definition.kind === 'OperationDefinition' || - (definition.kind === 'FragmentDefinition' && !includeAllFragments)) && - definition.selectionSet - ) { - definition.selectionSet = getDirectivesFromSelectionSet( - directives, - definition.selectionSet, - ); - } - return definition; + + let parentPath: string; + const modifiedDoc = visit(doc, { + SelectionSet: { + enter(node, _, __, path) { + const currentPath = path.join('-'); + + if ( + !parentPath || + currentPath === parentPath || + !currentPath.startsWith(parentPath) + ) { + if (node.selections) { + const selectionsWithDirectives = node.selections.filter(selection => + hasDirectivesInSelection(directives, selection), + ); + + if (hasDirectivesInSelectionSet(directives, node, false)) { + parentPath = currentPath; + } + + return { + ...node, + selections: selectionsWithDirectives, + }; + } else { + return null; + } + } + }, + }, }); - const operation = getOperationDefinitionOrDie(docClone); - const fragments = createFragmentMap(getFragmentDefinitions(docClone)); - return isNotEmpty(operation, fragments) ? docClone : null; + const operation = getOperationDefinitionOrDie(modifiedDoc); + const fragments = createFragmentMap(getFragmentDefinitions(modifiedDoc)); + return isNotEmpty(operation, fragments) ? modifiedDoc : null; } function getArgumentMatcher(config: RemoveArgumentsConfig[]) { @@ -490,213 +414,91 @@ function hasArgumentsInSelection( ); } -function getAllArgumentsFromSelectionSet( - selectionSet: SelectionSetNode, -): ArgumentNode[] { - return selectionSet.selections - .map(getAllArgumentsFromSelection) - .reduce((allArguments, selectionArguments) => { - return [...allArguments, ...selectionArguments]; - }, []); -} - -function getAllArgumentsFromSelection( - selection: SelectionNode, -): ArgumentNode[] { - if (selection.kind !== 'Field' || !(selection as FieldNode)) { - return []; - } - - return selection.arguments || []; -} - export function removeArgumentsFromDocument( config: RemoveArgumentsConfig[], - query: DocumentNode, + doc: DocumentNode, ): DocumentNode | null { - const docClone = cloneDeep(query); - - docClone.definitions.forEach((definition: DefinitionNode) => { - const operationDefinition = definition as OperationDefinitionNode; - const removeVariableConfig = config - .filter(aConfig => !!aConfig.name) - .map(aConfig => ({ - name: aConfig.name, - remove: aConfig.remove, - })); - - removeArgumentsFromSelectionSet(config, operationDefinition.selectionSet); - removeArgumentsFromOperationDefinition( - removeVariableConfig, - operationDefinition, - ); - }); - - const operation = getOperationDefinitionOrDie(docClone); - const fragments = createFragmentMap(getFragmentDefinitions(docClone)); - return isNotEmpty(operation, fragments) ? docClone : null; -} - -function removeArgumentsFromOperationDefinition( - config: RemoveVariableDefinitionConfig[], - definition: OperationDefinitionNode, -): OperationDefinitionNode { - if (!definition.variableDefinitions) return definition; - // if any of the config is set to remove this argument, remove it - const aggressiveRemove = config.some( - (aConfig: RemoveVariableDefinitionConfig) => aConfig.remove, - ); - - let remove: boolean; - definition.variableDefinitions = definition.variableDefinitions.filter( - aDefinition => { - const shouldKeep = !config.some(aConfig => { - if (aConfig.name === aDefinition.variable.name.value) return true; - if (aConfig.test && aConfig.test(aDefinition)) return true; - return false; - }); - - if (!remove && !shouldKeep && aggressiveRemove) { - remove = true; - } - - return shouldKeep; + const modifiedDoc = visit(doc, { + OperationDefinition: { + enter(node) { + // Remove matching top level variables definitions. + const variableDefinitions = node.variableDefinitions.filter( + varDef => + !config.some(arg => arg.name === varDef.variable.name.value), + ); + return { + ...node, + variableDefinitions, + }; + }, }, - ); - - return definition; -} -function removeArgumentsFromSelectionSet( - config: RemoveArgumentsConfig[], - selectionSet: SelectionSetNode, -): SelectionSetNode { - if (!selectionSet.selections) return selectionSet; - // if any of the config is set to remove this selectionSet, remove it - const aggressiveRemove = config.some( - (aConfig: RemoveArgumentsConfig) => aConfig.remove, - ); - - selectionSet.selections = selectionSet.selections - .map(selection => { - if ( - selection.kind !== 'Field' || - !(selection as FieldNode) || - !selection.arguments - ) { - return selection; - } - - let remove: boolean; - const argumentMatcher = getArgumentMatcher(config); - selection.arguments = selection.arguments.filter(argument => { - const shouldKeep = !argumentMatcher(argument); - if (!remove && !shouldKeep && aggressiveRemove) { - remove = true; + Field: { + enter(node) { + // If `remove` is set to true for an argument, and an argument match + // is found for a field, remove the field as well. + const shouldRemoveField = config.some(argConfig => argConfig.remove); + + if (shouldRemoveField) { + const argMatcher = getArgumentMatcher(config); + let argMatchCount = 0; + node.arguments.forEach(arg => { + if (argMatcher(arg)) { + argMatchCount += 1; + } + }); + if (argMatchCount === 1) { + return null; + } } + }, + }, - return shouldKeep; - }); - - return remove ? null : selection; - }) - .filter(x => !!x); - - selectionSet.selections.forEach(selection => { - if ( - (selection.kind === 'Field' || selection.kind === 'InlineFragment') && - selection.selectionSet - ) { - removeArgumentsFromSelectionSet(config, selection.selectionSet); - } + Argument: { + enter(node) { + // Remove all matching arguments. + const argMatcher = getArgumentMatcher(config); + if (argMatcher(node)) { + return null; + } + }, + }, }); - return selectionSet; -} - -function hasFragmentSpreadInSelection( - config: RemoveFragmentSpreadConfig[], - selection: SelectionNode, -): boolean { - if ( - selection.kind !== 'FragmentSpread' || - !(selection as FragmentSpreadNode) - ) { - return false; - } - - return config.some(aConfig => { - if (aConfig.name === selection.name.value) return true; - if (aConfig.test && aConfig.test(selection)) return true; - return false; - }); + const operation = getOperationDefinitionOrDie(modifiedDoc); + const fragments = createFragmentMap(getFragmentDefinitions(modifiedDoc)); + return isNotEmpty(operation, fragments) ? modifiedDoc : null; } export function removeFragmentSpreadFromDocument( config: RemoveFragmentSpreadConfig[], - query: DocumentNode, + doc: DocumentNode, ): DocumentNode | null { - const docClone = cloneDeep(query); - - docClone.definitions.forEach((definition: DefinitionNode) => { - removeFragmentSpreadFromSelectionSet( - config, - (definition as OperationDefinitionNode).selectionSet, - ); - }); - - docClone.definitions = removeFragmentSpreadFromDefinitions( - config - .filter(aConfig => !!aConfig.name) - .map(aConfig => ({ name: aConfig.name })), - docClone.definitions, - ); - - const operation = getOperationDefinitionOrDie(docClone); - const fragments = createFragmentMap(getFragmentDefinitions(docClone)); - return isNotEmpty(operation, fragments) ? docClone : null; -} - -function removeFragmentSpreadFromDefinitions( - config: RemoveFragmentDefinitionConfig[], - definitions: DefinitionNode[], -): DefinitionNode[] { - return definitions.filter(definition => { - if ( - definition.kind !== 'FragmentDefinition' || - !(definition as FragmentDefinitionNode) - ) { - return true; - } - - return !config.some(aConfig => { - if (aConfig.name && aConfig.name === definition.name.value) return true; - if (aConfig.test && aConfig.test(definition)) return true; - return false; - }); - }); -} - -function removeFragmentSpreadFromSelectionSet( - config: RemoveFragmentSpreadConfig[], - selectionSet: SelectionSetNode, -): SelectionSetNode { - if (!selectionSet.selections) return selectionSet; - - selectionSet.selections = selectionSet.selections.filter( - selection => !hasFragmentSpreadInSelection(config, selection), - ); + const modifiedDoc = visit(doc, { + FragmentSpread: { + enter(node) { + const fragSpreadFound = config.some( + fragSpread => fragSpread.name === node.name.value, + ); + if (fragSpreadFound) { + return null; + } + }, + }, - selectionSet.selections.forEach(selection => { - if ( - (selection.kind === 'Field' || selection.kind === 'InlineFragment') && - selection.selectionSet - ) { - removeFragmentSpreadFromSelectionSet(config, selection.selectionSet); - } + FragmentDefinition: { + enter(node) { + const fragDefFound = config.some( + fragDef => fragDef.name && fragDef.name === node.name.value, + ); + if (fragDefFound) { + return null; + } + }, + }, }); - return selectionSet; + return modifiedDoc; } function getAllFragmentSpreadsFromSelectionSet( From 30b7b31d4c71ca426625cb02ce233fb4f52c4747 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 20 Dec 2018 07:21:28 -0500 Subject: [PATCH 06/25] Remove `includes` and tsconfig `es7` use Let's avoid adding in a polyfill. --- packages/apollo-utilities/src/transform.ts | 4 ++-- packages/apollo-utilities/tsconfig.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index cbead229cca..b35886c86c9 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -191,7 +191,7 @@ export function removeDirectivesFromDocument( // aren't being used elsewhere. if (variablesToRemove) { variablesToRemove = variablesToRemove.filter( - variable => !variablesInUse.includes(variable.name), + variable => variablesInUse.indexOf(variable.name) === -1, ); modifiedDoc = removeArgumentsFromDocument(variablesToRemove, modifiedDoc); } @@ -201,7 +201,7 @@ export function removeDirectivesFromDocument( // document, as long as they aren't being used elsewhere. if (fragmentSpreadsToRemove) { fragmentSpreadsToRemove = fragmentSpreadsToRemove.filter( - fragSpread => !fragmentSpreadsInUse.includes(fragSpread.name), + fragSpread => fragmentSpreadsInUse.indexOf(fragSpread.name) === -1, ); modifiedDoc = removeFragmentSpreadFromDocument( fragmentSpreadsToRemove, diff --git a/packages/apollo-utilities/tsconfig.json b/packages/apollo-utilities/tsconfig.json index e30d7a994fa..163b2777956 100644 --- a/packages/apollo-utilities/tsconfig.json +++ b/packages/apollo-utilities/tsconfig.json @@ -3,7 +3,7 @@ "compilerOptions": { "rootDir": "src", "outDir": "lib", - "lib": ["es6", "dom", "es7"] + "lib": ["es6", "dom"] }, "include": ["src/**/*.ts"], "exclude": ["src/**/__tests__/*.ts"] From 67efcaa8c22ce42a7cfacf5cc4c44b6b9c5cd366 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 20 Dec 2018 07:27:18 -0500 Subject: [PATCH 07/25] Pull `visit` in via a direct nested import --- packages/apollo-utilities/src/transform.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index b35886c86c9..2ae066ca7e1 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -10,8 +10,8 @@ import { FragmentSpreadNode, VariableDefinitionNode, VariableNode, - visit, } from 'graphql'; +import { visit } from 'graphql/language/visitor'; import { checkDocument, From f4e064e5b0a0779dfc08fa71791fcd6436ec0a09 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 20 Dec 2018 10:30:55 -0500 Subject: [PATCH 08/25] Code cleanup to address outstanding code review comments --- packages/apollo-utilities/src/transform.ts | 100 +++++++++------------ 1 file changed, 43 insertions(+), 57 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 2ae066ca7e1..8fba4ad408d 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -76,7 +76,7 @@ function isNotEmpty( function getDirectiveMatcher( directives: (RemoveDirectiveConfig | GetDirectiveConfig)[], ) { - return function directiveMatcher(directive: DirectiveNode): Boolean { + return function directiveMatcher(directive: DirectiveNode): boolean { return directives.some( (dir: RemoveDirectiveConfig | GetDirectiveConfig) => { if (dir.name && dir.name === directive.name.value) return true; @@ -91,22 +91,22 @@ export function removeDirectivesFromDocument( directives: RemoveDirectiveConfig[], doc: DocumentNode, ): DocumentNode | null { - let variablesInUse: string[] = []; + const variablesInUse: Record = Object.create(null); let variablesToRemove: RemoveArgumentsConfig[] = []; - let fragmentSpreadsInUse: string[] = []; + const fragmentSpreadsInUse: Record = Object.create(null); let fragmentSpreadsToRemove: RemoveFragmentSpreadConfig[] = []; let modifiedDoc = visit(doc, { Variable: { - enter(node, _, parent) { + enter(node, _key, parent) { // Store each variable that's referenced as part of an argument // (excluding operation definition variables), so we know which // variables are being used. If we later want to remove a variable // we'll fist check to see if it's being used, before continuing with // the removal. if ((parent as VariableDefinitionNode).kind !== 'VariableDefinition') { - variablesInUse.push(node.name.value); + variablesInUse[node.name.value] = true; } }, }, @@ -119,45 +119,36 @@ export function removeDirectivesFromDocument( directive => directive.remove, ); - if (shouldRemoveField) { - const matcher = getDirectiveMatcher(directives); - let removeField = false; - node.directives.forEach(directive => { - if (matcher(directive)) { - removeField = true; - return; - } - }); - - if (removeField) { - if (node.arguments) { - // Store field argument variables so they can be removed - // from the operation definition. - variablesToRemove = variablesToRemove.concat( - node.arguments - .filter(arg => arg.value.kind === 'Variable') - .map(arg => ({ - name: (arg.value as VariableNode).name.value, - })), - ); - } + if ( + shouldRemoveField && + node.directives.some(getDirectiveMatcher(directives)) + ) { + if (node.arguments) { + // Store field argument variables so they can be removed + // from the operation definition. + node.arguments + .filter(arg => arg.value.kind === 'Variable') + .forEach(arg => { + variablesToRemove.push({ + name: (arg.value as VariableNode).name.value, + }); + }); + } - if (node.selectionSet) { - // Store fragment spread names so they can be removed from the - // docuemnt. - const fragSpreads = getAllFragmentSpreadsFromSelectionSet( - node.selectionSet, - ); - fragmentSpreadsToRemove = fragmentSpreadsToRemove.concat( - fragSpreads.map(frag => ({ + if (node.selectionSet) { + // Store fragment spread names so they can be removed from the + // docuemnt. + getAllFragmentSpreadsFromSelectionSet(node.selectionSet).forEach( + frag => { + fragmentSpreadsToRemove.push({ name: frag.name.value, - })), - ); - } - - // Remove the field. - return null; + }); + }, + ); } + + // Remove the field. + return null; } }, }, @@ -166,7 +157,7 @@ export function removeDirectivesFromDocument( enter(node) { // Keep track of referenced fragment spreads. This is used to // determine if top level fragment definitions should be removed. - fragmentSpreadsInUse.push(node.name.value); + fragmentSpreadsInUse[node.name.value] = true; }, }, @@ -191,7 +182,7 @@ export function removeDirectivesFromDocument( // aren't being used elsewhere. if (variablesToRemove) { variablesToRemove = variablesToRemove.filter( - variable => variablesInUse.indexOf(variable.name) === -1, + variable => !variablesInUse[variable.name], ); modifiedDoc = removeArgumentsFromDocument(variablesToRemove, modifiedDoc); } @@ -201,7 +192,7 @@ export function removeDirectivesFromDocument( // document, as long as they aren't being used elsewhere. if (fragmentSpreadsToRemove) { fragmentSpreadsToRemove = fragmentSpreadsToRemove.filter( - fragSpread => fragmentSpreadsInUse.indexOf(fragSpread.name) === -1, + fragSpread => !fragmentSpreadsInUse[fragSpread.name], ); modifiedDoc = removeFragmentSpreadFromDocument( fragmentSpreadsToRemove, @@ -215,19 +206,14 @@ export function removeDirectivesFromDocument( export function addTypenameToDocument(doc: DocumentNode) { checkDocument(doc); - let isRoot = false; const modifiedDoc = visit(doc, { - OperationDefinition: { - enter(node) { - isRoot = true; - }, - }, - SelectionSet: { - enter(node) { + enter(node, _key, parent) { // Don't add __typename to OperationDefinitions. - if (isRoot) { - isRoot = false; + if ( + parent && + (parent as OperationDefinitionNode).kind === 'OperationDefinition' + ) { return undefined; } @@ -332,7 +318,7 @@ export function getDirectivesFromDocument( let parentPath: string; const modifiedDoc = visit(doc, { SelectionSet: { - enter(node, _, __, path) { + enter(node, _key, _parent, path) { const currentPath = path.join('-'); if ( @@ -418,6 +404,8 @@ export function removeArgumentsFromDocument( config: RemoveArgumentsConfig[], doc: DocumentNode, ): DocumentNode | null { + const argMatcher = getArgumentMatcher(config); + const modifiedDoc = visit(doc, { OperationDefinition: { enter(node) { @@ -440,7 +428,6 @@ export function removeArgumentsFromDocument( const shouldRemoveField = config.some(argConfig => argConfig.remove); if (shouldRemoveField) { - const argMatcher = getArgumentMatcher(config); let argMatchCount = 0; node.arguments.forEach(arg => { if (argMatcher(arg)) { @@ -457,7 +444,6 @@ export function removeArgumentsFromDocument( Argument: { enter(node) { // Remove all matching arguments. - const argMatcher = getArgumentMatcher(config); if (argMatcher(node)) { return null; } From aea4d3d94f1170cf08355e4b9263e056c5f07143 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 10:51:12 -0500 Subject: [PATCH 09/25] Replace isNotEmpty helper with simpler isEmpty helper. --- packages/apollo-utilities/src/transform.ts | 36 +++++++++------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 8fba4ad408d..9ca25b29afc 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -52,24 +52,14 @@ const TYPENAME_FIELD: FieldNode = { }, }; -function isNotEmpty( +function isEmpty( op: OperationDefinitionNode | FragmentDefinitionNode, fragments: FragmentMap, -): Boolean { - // keep selections that are still valid - return ( - op.selectionSet.selections.filter( - selectionSet => - // anything that doesn't match the compound filter is okay - !// not an empty array - ( - selectionSet && - // look into fragments to verify they should stay - selectionSet.kind === 'FragmentSpread' && - // see if the fragment in the map is valid (recursively) - !isNotEmpty(fragments[selectionSet.name.value], fragments) - ), - ).length > 0 +): boolean { + return op.selectionSet.selections.every( + selection => + selection.kind === 'FragmentSpread' && + isEmpty(fragments[selection.name.value], fragments), ); } @@ -347,9 +337,10 @@ export function getDirectivesFromDocument( }, }); - const operation = getOperationDefinitionOrDie(modifiedDoc); - const fragments = createFragmentMap(getFragmentDefinitions(modifiedDoc)); - return isNotEmpty(operation, fragments) ? modifiedDoc : null; + return !isEmpty( + getOperationDefinitionOrDie(modifiedDoc), + createFragmentMap(getFragmentDefinitions(modifiedDoc)), + ) ? modifiedDoc : null; } function getArgumentMatcher(config: RemoveArgumentsConfig[]) { @@ -451,9 +442,10 @@ export function removeArgumentsFromDocument( }, }); - const operation = getOperationDefinitionOrDie(modifiedDoc); - const fragments = createFragmentMap(getFragmentDefinitions(modifiedDoc)); - return isNotEmpty(operation, fragments) ? modifiedDoc : null; + return !isEmpty( + getOperationDefinitionOrDie(modifiedDoc), + createFragmentMap(getFragmentDefinitions(modifiedDoc)), + ) ? modifiedDoc : null; } export function removeFragmentSpreadFromDocument( From 8ac5d2d140cd83e44b9d9a738ee8e03d8c401343 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 10:57:07 -0500 Subject: [PATCH 10/25] Use boolean expression in directiveMatcher. --- packages/apollo-utilities/src/transform.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 9ca25b29afc..8d4ba339ba6 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -66,13 +66,11 @@ function isEmpty( function getDirectiveMatcher( directives: (RemoveDirectiveConfig | GetDirectiveConfig)[], ) { - return function directiveMatcher(directive: DirectiveNode): boolean { + return function directiveMatcher(directive: DirectiveNode) { return directives.some( - (dir: RemoveDirectiveConfig | GetDirectiveConfig) => { - if (dir.name && dir.name === directive.name.value) return true; - if (dir.test && dir.test(directive)) return true; - return false; - }, + dir => + (dir.name && dir.name === directive.name.value) || + (dir.test && dir.test(directive)), ); }; } From 7eae13d200583bfe138bf45d4694a0ad9270b520 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 13:01:13 -0500 Subject: [PATCH 11/25] Use boolean expression in getArgumentMatcher. --- packages/apollo-utilities/src/transform.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 8d4ba339ba6..0e8b72dc6ae 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -342,18 +342,15 @@ export function getDirectivesFromDocument( } function getArgumentMatcher(config: RemoveArgumentsConfig[]) { - return (argument: ArgumentNode): Boolean => { - return config.some((aConfig: RemoveArgumentsConfig) => { - if ( - argument.value.kind !== 'Variable' || - !(argument.value as VariableNode) - ) - return false; - if (!argument.value.name) return false; - if (aConfig.name === argument.value.name.value) return true; - if (aConfig.test && aConfig.test(argument)) return true; - return false; - }); + return function argumentMatcher(argument: ArgumentNode) { + return config.some( + (aConfig: RemoveArgumentsConfig) => + argument.value && + argument.value.kind === 'Variable' && + argument.value.name && + (aConfig.name === argument.value.name.value || + (aConfig.test && aConfig.test(argument))), + ); }; } From 760297d2c5c7aa4ec7480e7047c4d5eeb248a0b9 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:09:59 -0500 Subject: [PATCH 12/25] Reuse same enter function in removeFragmentSpreadFromDocument. --- packages/apollo-utilities/src/transform.ts | 35 +++++++--------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 0e8b72dc6ae..dcd616474ac 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -446,32 +446,19 @@ export function removeArgumentsFromDocument( export function removeFragmentSpreadFromDocument( config: RemoveFragmentSpreadConfig[], doc: DocumentNode, -): DocumentNode | null { - const modifiedDoc = visit(doc, { - FragmentSpread: { - enter(node) { - const fragSpreadFound = config.some( - fragSpread => fragSpread.name === node.name.value, - ); - if (fragSpreadFound) { - return null; - } - }, - }, +): DocumentNode { + function enter( + node: FragmentSpreadNode | FragmentDefinitionNode, + ): null | void { + if (config.some(def => def.name === node.name.value)) { + return null; + } + } - FragmentDefinition: { - enter(node) { - const fragDefFound = config.some( - fragDef => fragDef.name && fragDef.name === node.name.value, - ); - if (fragDefFound) { - return null; - } - }, - }, + return visit(doc, { + FragmentSpread: { enter }, + FragmentDefinition: { enter }, }); - - return modifiedDoc; } function getAllFragmentSpreadsFromSelectionSet( From 7d17240bb4f9dc8874dbcb75ce5f59442ccacfe7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:21:02 -0500 Subject: [PATCH 13/25] Reduce array creation in getAllFragmentSpreadsFromSelectionSet. --- packages/apollo-utilities/src/transform.ts | 40 ++++++++-------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index dcd616474ac..c884081aae7 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -464,33 +464,21 @@ export function removeFragmentSpreadFromDocument( function getAllFragmentSpreadsFromSelectionSet( selectionSet: SelectionSetNode, ): FragmentSpreadNode[] { - return selectionSet.selections - .map(getAllFragmentSpreadsFromSelection) - .reduce( - (allFragments, selectionFragments) => [ - ...allFragments, - ...selectionFragments, - ], - [], - ); -} - -function getAllFragmentSpreadsFromSelection( - selection: SelectionNode, -): FragmentSpreadNode[] { - if ( - (selection.kind === 'Field' || selection.kind === 'InlineFragment') && - selection.selectionSet - ) { - return getAllFragmentSpreadsFromSelectionSet(selection.selectionSet); - } else if ( - selection.kind === 'FragmentSpread' && - (selection as FragmentSpreadNode) - ) { - return [selection]; - } + const allFragments: FragmentSpreadNode[] = []; + + selectionSet.selections.forEach(selection => { + if ((selection.kind === 'Field' || + selection.kind === 'InlineFragment') && + selection.selectionSet) { + getAllFragmentSpreadsFromSelectionSet( + selection.selectionSet + ).forEach(frag => allFragments.push(frag)); + } else if (selection.kind === 'FragmentSpread') { + allFragments.push(selection); + } + }); - return []; + return allFragments; } function filterSelectionSet( From 2a6e7b7b63106174d3e5308c8a28b195e8e0879f Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:29:56 -0500 Subject: [PATCH 14/25] Simplify hasArgumentsInSelection. --- packages/apollo-utilities/src/transform.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index c884081aae7..43bd6314c0b 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -377,10 +377,9 @@ function hasArgumentsInSelection( if (!selection.arguments) { return false; } - const matcher = getArgumentMatcher(config); - const matchedArguments = selection.arguments.filter(matcher); + return ( - matchedArguments.length > 0 || + selection.arguments.some(getArgumentMatcher(config)) || (nestedCheck && hasArgumentsInSelectionSet(config, selection.selectionSet, nestedCheck)) ); From aad9e9a7b2810ba72789e98a8c4e16985dfde7fc Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:33:24 -0500 Subject: [PATCH 15/25] Eliminate unused hasArgumentsInSelection[Set] helper functions. --- packages/apollo-utilities/src/transform.ts | 31 ---------------------- 1 file changed, 31 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 43bd6314c0b..8e74f45d12f 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -354,37 +354,6 @@ function getArgumentMatcher(config: RemoveArgumentsConfig[]) { }; } -function hasArgumentsInSelectionSet( - config: RemoveArgumentsConfig[], - selectionSet: SelectionSetNode, - nestedCheck: boolean = false, -): boolean { - return filterSelectionSet(selectionSet, selection => - hasArgumentsInSelection(config, selection, nestedCheck), - ); -} - -function hasArgumentsInSelection( - config: RemoveArgumentsConfig[], - selection: SelectionNode, - nestedCheck: boolean = false, -): boolean { - // Selection is a FragmentSpread or InlineFragment, ignore (include it)... - if (selection.kind !== 'Field' || !(selection as FieldNode)) { - return true; - } - - if (!selection.arguments) { - return false; - } - - return ( - selection.arguments.some(getArgumentMatcher(config)) || - (nestedCheck && - hasArgumentsInSelectionSet(config, selection.selectionSet, nestedCheck)) - ); -} - export function removeArgumentsFromDocument( config: RemoveArgumentsConfig[], doc: DocumentNode, From af76400c34b8ddf6f6fb0c7bd609cfee6fc77fe8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:37:30 -0500 Subject: [PATCH 16/25] Simplify hasDirectivesInSelection helper function. --- packages/apollo-utilities/src/transform.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 8e74f45d12f..c42d9c9e7db 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -282,12 +282,9 @@ function hasDirectivesInSelection( if (!selection.directives) { return false; } - const directiveMatcher = getDirectiveMatcher(directives); - const matchedDirectives = selection.directives.filter(directiveMatcher); - const hasMatches = matchedDirectives.length > 0; return ( - hasMatches || + selection.directives.some(getDirectiveMatcher(directives)) || (nestedCheck && hasDirectivesInSelectionSet( directives, From 3b51ad5f378338c5833a4f58ed211327fcd46529 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:40:48 -0500 Subject: [PATCH 17/25] Inline filterSelectionSet into sole remaining call site. --- packages/apollo-utilities/src/transform.ts | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index c42d9c9e7db..634c1fe13c0 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -265,8 +265,12 @@ function hasDirectivesInSelectionSet( selectionSet: SelectionSetNode, nestedCheck = true, ): boolean { - return filterSelectionSet(selectionSet, selection => - hasDirectivesInSelection(directives, selection, nestedCheck), + return ( + selectionSet && + selectionSet.selections && + selectionSet.selections.some(selection => + hasDirectivesInSelection(directives, selection, nestedCheck), + ) ); } @@ -445,14 +449,3 @@ function getAllFragmentSpreadsFromSelectionSet( return allFragments; } - -function filterSelectionSet( - selectionSet: SelectionSetNode, - filter: (node: SelectionNode) => boolean, -) { - if (!(selectionSet && selectionSet.selections)) { - return false; - } - - return selectionSet.selections.filter(filter).length > 0; -} From 94eab297c213ce73f7fcd50ae7987f505dea6e34 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:51:12 -0500 Subject: [PATCH 18/25] Reuse getDirectiveMatcher in removeDirectivesFromDocument. --- packages/apollo-utilities/src/transform.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 634c1fe13c0..79e993ab909 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -152,13 +152,7 @@ export function removeDirectivesFromDocument( Directive: { enter(node) { // If a matching directive is found, remove it. - const directiveFound = directives.some(directive => { - if (directive.name && directive.name === node.name.value) return true; - if (directive.test && directive.test(node)) return true; - return false; - }); - if (directiveFound) { - // Remove the directive. + if (getDirectiveMatcher(directives)(node)) { return null; } }, From 9a67fcc9fb53664112623ed557c4e6368b878f52 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 11:56:44 -0500 Subject: [PATCH 19/25] Avoid unnecessary node.arguments.filter in removeDirectivesFromDocument. --- packages/apollo-utilities/src/transform.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 79e993ab909..5d3b84edd41 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -114,13 +114,13 @@ export function removeDirectivesFromDocument( if (node.arguments) { // Store field argument variables so they can be removed // from the operation definition. - node.arguments - .filter(arg => arg.value.kind === 'Variable') - .forEach(arg => { + node.arguments.forEach(arg => { + if (arg.value.kind === 'Variable') { variablesToRemove.push({ name: (arg.value as VariableNode).name.value, }); - }); + } + }); } if (node.selectionSet) { From 0b6bc769da206427917143e62b8b4693db7c9bff Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 12:16:55 -0500 Subject: [PATCH 20/25] Extract nullIfDocIsEmpty helper function. --- packages/apollo-utilities/src/transform.ts | 52 ++++++++++++---------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 5d3b84edd41..d6c2cf05df1 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -63,6 +63,15 @@ function isEmpty( ); } +function nullIfDocIsEmpty(doc: DocumentNode) { + return isEmpty( + getOperationDefinitionOrDie(doc), + createFragmentMap( + getFragmentDefinitions(doc) + ), + ) ? null : doc; +} + function getDirectiveMatcher( directives: (RemoveDirectiveConfig | GetDirectiveConfig)[], ) { @@ -85,7 +94,7 @@ export function removeDirectivesFromDocument( const fragmentSpreadsInUse: Record = Object.create(null); let fragmentSpreadsToRemove: RemoveFragmentSpreadConfig[] = []; - let modifiedDoc = visit(doc, { + let modifiedDoc = nullIfDocIsEmpty(visit(doc, { Variable: { enter(node, _key, parent) { // Store each variable that's referenced as part of an argument @@ -157,25 +166,27 @@ export function removeDirectivesFromDocument( } }, }, - }); + })); // If we've removed fields with arguments, make sure the associated // variables are also removed from the rest of the document, as long as they // aren't being used elsewhere. - if (variablesToRemove) { + if (modifiedDoc && variablesToRemove) { variablesToRemove = variablesToRemove.filter( variable => !variablesInUse[variable.name], ); + modifiedDoc = removeArgumentsFromDocument(variablesToRemove, modifiedDoc); } // If we've removed selection sets with fragment spreads, make sure the // associated fragment definitions are also removed from the rest of the // document, as long as they aren't being used elsewhere. - if (fragmentSpreadsToRemove) { + if (modifiedDoc && fragmentSpreadsToRemove) { fragmentSpreadsToRemove = fragmentSpreadsToRemove.filter( fragSpread => !fragmentSpreadsInUse[fragSpread.name], ); + modifiedDoc = removeFragmentSpreadFromDocument( fragmentSpreadsToRemove, modifiedDoc, @@ -295,11 +306,12 @@ function hasDirectivesInSelection( export function getDirectivesFromDocument( directives: GetDirectiveConfig[], doc: DocumentNode, -): DocumentNode | null { +): DocumentNode { checkDocument(doc); let parentPath: string; - const modifiedDoc = visit(doc, { + + return nullIfDocIsEmpty(visit(doc, { SelectionSet: { enter(node, _key, _parent, path) { const currentPath = path.join('-'); @@ -328,12 +340,7 @@ export function getDirectivesFromDocument( } }, }, - }); - - return !isEmpty( - getOperationDefinitionOrDie(modifiedDoc), - createFragmentMap(getFragmentDefinitions(modifiedDoc)), - ) ? modifiedDoc : null; + })); } function getArgumentMatcher(config: RemoveArgumentsConfig[]) { @@ -352,10 +359,10 @@ function getArgumentMatcher(config: RemoveArgumentsConfig[]) { export function removeArgumentsFromDocument( config: RemoveArgumentsConfig[], doc: DocumentNode, -): DocumentNode | null { +): DocumentNode { const argMatcher = getArgumentMatcher(config); - const modifiedDoc = visit(doc, { + return nullIfDocIsEmpty(visit(doc, { OperationDefinition: { enter(node) { // Remove matching top level variables definitions. @@ -398,12 +405,7 @@ export function removeArgumentsFromDocument( } }, }, - }); - - return !isEmpty( - getOperationDefinitionOrDie(modifiedDoc), - createFragmentMap(getFragmentDefinitions(modifiedDoc)), - ) ? modifiedDoc : null; + })); } export function removeFragmentSpreadFromDocument( @@ -418,10 +420,12 @@ export function removeFragmentSpreadFromDocument( } } - return visit(doc, { - FragmentSpread: { enter }, - FragmentDefinition: { enter }, - }); + return nullIfDocIsEmpty( + visit(doc, { + FragmentSpread: { enter }, + FragmentDefinition: { enter }, + }), + ); } function getAllFragmentSpreadsFromSelectionSet( From f3b640ffd5b23cb4e2acf37ae58ecda698faf9b0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 12:33:06 -0500 Subject: [PATCH 21/25] Use filterInPlace helper in removeDirectivesFromDocument. --- packages/apollo-utilities/src/transform.ts | 18 ++++++++---------- .../apollo-utilities/src/util/filterInPlace.ts | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 packages/apollo-utilities/src/util/filterInPlace.ts diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index d6c2cf05df1..b9d6475f051 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -20,6 +20,7 @@ import { createFragmentMap, FragmentMap, } from './getFromAST'; +import { filterInPlace } from './util/filterInPlace'; export type RemoveNodeConfig = { name?: string; @@ -171,22 +172,19 @@ export function removeDirectivesFromDocument( // If we've removed fields with arguments, make sure the associated // variables are also removed from the rest of the document, as long as they // aren't being used elsewhere. - if (modifiedDoc && variablesToRemove) { - variablesToRemove = variablesToRemove.filter( - variable => !variablesInUse[variable.name], - ); - + if (modifiedDoc && + filterInPlace(variablesToRemove, v => !variablesInUse[v.name]).length) { modifiedDoc = removeArgumentsFromDocument(variablesToRemove, modifiedDoc); } // If we've removed selection sets with fragment spreads, make sure the // associated fragment definitions are also removed from the rest of the // document, as long as they aren't being used elsewhere. - if (modifiedDoc && fragmentSpreadsToRemove) { - fragmentSpreadsToRemove = fragmentSpreadsToRemove.filter( - fragSpread => !fragmentSpreadsInUse[fragSpread.name], - ); - + if (modifiedDoc && + filterInPlace( + fragmentSpreadsToRemove, + fs => !fragmentSpreadsInUse[fs.name], + ).length) { modifiedDoc = removeFragmentSpreadFromDocument( fragmentSpreadsToRemove, modifiedDoc, diff --git a/packages/apollo-utilities/src/util/filterInPlace.ts b/packages/apollo-utilities/src/util/filterInPlace.ts new file mode 100644 index 00000000000..d5787cabaaf --- /dev/null +++ b/packages/apollo-utilities/src/util/filterInPlace.ts @@ -0,0 +1,14 @@ +export function filterInPlace( + array: T[], + test: (elem: T) => boolean, + context?: any, +): T[] { + let target = 0; + array.forEach(function (elem, i) { + if (test.call(this, elem, i, array)) { + array[target++] = elem; + } + }, context); + array.length = target; + return array; +} From 131acdd88b3cbbabdd7d86cb1871878e8ac635ff Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 12:43:43 -0500 Subject: [PATCH 22/25] Make checkDocument(doc) return doc. --- packages/apollo-utilities/src/getFromAST.ts | 2 ++ packages/apollo-utilities/src/transform.ts | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/apollo-utilities/src/getFromAST.ts b/packages/apollo-utilities/src/getFromAST.ts index 3db665b514e..1567e1bd35a 100644 --- a/packages/apollo-utilities/src/getFromAST.ts +++ b/packages/apollo-utilities/src/getFromAST.ts @@ -51,6 +51,8 @@ string in a "gql" tag? http://docs.apollostack.com/apollo-client/core.html#gql`) `Ambiguous GraphQL document: contains ${operations.length} operations`, ); } + + return doc; } export function getOperationDefinition( diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index b9d6475f051..e390ba03389 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -259,8 +259,10 @@ const connectionRemoveConfig = { }; export function removeConnectionDirectiveFromDocument(doc: DocumentNode) { - checkDocument(doc); - return removeDirectivesFromDocument([connectionRemoveConfig], doc); + return removeDirectivesFromDocument( + [connectionRemoveConfig], + checkDocument(doc), + ); } function hasDirectivesInSelectionSet( From d8ed6c0d8232c222e47c793ed5bba4f9bbbda0a0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 12:44:06 -0500 Subject: [PATCH 23/25] Simplify addTypenameToDocument. --- packages/apollo-utilities/src/transform.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index e390ba03389..1e84a5ca490 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -194,10 +194,8 @@ export function removeDirectivesFromDocument( return modifiedDoc; } -export function addTypenameToDocument(doc: DocumentNode) { - checkDocument(doc); - - const modifiedDoc = visit(doc, { +export function addTypenameToDocument(doc: DocumentNode): DocumentNode { + return visit(checkDocument(doc), { SelectionSet: { enter(node, _key, parent) { // Don't add __typename to OperationDefinitions. @@ -205,13 +203,13 @@ export function addTypenameToDocument(doc: DocumentNode) { parent && (parent as OperationDefinitionNode).kind === 'OperationDefinition' ) { - return undefined; + return; } // No changes if no selections. const { selections } = node; if (!selections) { - return undefined; + return; } // If selections already have a __typename, or are part of an @@ -224,7 +222,7 @@ export function addTypenameToDocument(doc: DocumentNode) { ); }); if (skip) { - return undefined; + return; } // Create and return a new SelectionSet with a __typename Field. @@ -235,8 +233,6 @@ export function addTypenameToDocument(doc: DocumentNode) { }, }, }); - - return modifiedDoc; } const connectionRemoveConfig = { From 60a2ddf7866b737e8ab58e427c0adf211696f96e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 20 Dec 2018 12:46:43 -0500 Subject: [PATCH 24/25] Inline variableDefinitions filter expression. --- packages/apollo-utilities/src/transform.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/apollo-utilities/src/transform.ts b/packages/apollo-utilities/src/transform.ts index 1e84a5ca490..907a13b3b12 100644 --- a/packages/apollo-utilities/src/transform.ts +++ b/packages/apollo-utilities/src/transform.ts @@ -361,14 +361,12 @@ export function removeArgumentsFromDocument( return nullIfDocIsEmpty(visit(doc, { OperationDefinition: { enter(node) { - // Remove matching top level variables definitions. - const variableDefinitions = node.variableDefinitions.filter( - varDef => - !config.some(arg => arg.name === varDef.variable.name.value), - ); return { ...node, - variableDefinitions, + // Remove matching top level variables definitions. + variableDefinitions: node.variableDefinitions.filter( + varDef => !config.some(arg => arg.name === varDef.variable.name.value), + ), }; }, }, From 2d3df6e561f4d87d8e6a55d5c94d6ac1f222aa94 Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 20 Dec 2018 13:53:02 -0500 Subject: [PATCH 25/25] Changelog updates --- CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b4f7756c8f..df60371b70e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,22 @@ ## Apollo Client (vNext) +### Apollo Client (vNext) + +- Apollo Client has been updated to use `graphql` 14.x as a dev dependency.
+ [@hwillson](https://github.com/hwillson) in [#4233](https://github.com/apollographql/apollo-client/pull/4233) + +### Apollo Utilities (vNext) + +- Transformation utilities have been refactored to work with `graphql` 14.x. + GraphQL AST's are no longer being directly modified.
+ [@hwillson](https://github.com/hwillson) in [#4233](https://github.com/apollographql/apollo-client/pull/4233) + ## Apollo Client (2.4.8) ### Apollo Client (2.4.8) -- Documtation and config updates.
+- Documentation and config updates.
[@justinanastos](https://github.com/justinanastos) in [#4187](https://github.com/apollographql/apollo-client/pull/4187)
[@PowerKiKi](https://github.com/PowerKiKi) in [#3693](https://github.com/apollographql/apollo-client/pull/3693)
[@nandito](https://github.com/nandito) in [#3865](https://github.com/apollographql/apollo-client/pull/3865)