Refactored query transformer (Issue #373)

CHANGELOG.md

@@ -4,6 +4,8 @@ Expect active development and potentially significant breaking changes in the `0
 
 ### vNEXT
 
+- Make sure query transformers are also applied to named fragments, and new methods that allow transforming query document with multiple query transformers. [Issue #373](https://github.com/apollostack/apollo-client/issues/373) [PR #412](https://github.com/apollostack/apollo-client/pull/412)
+
 ### v0.4.3
 
 - Introduce a new (preferable) way to express how the mutation result should be incorporated into the store and update watched queries results: `updateQueries`. [PR #404](https://github.com/apollostack/apollo-client/pull/404).

src/QueryManager.ts

@@ -21,7 +21,6 @@ import {
 import {
   getMutationDefinition,
   getQueryDefinition,
-  replaceOperationDefinition,
   getFragmentDefinitions,
   createFragmentMap,
   getOperationName,
@@ -30,7 +29,7 @@ import {
 
 import {
   QueryTransformer,
-  applyTransformerToOperation,
+  applyTransformers,
 } from './queries/queryTransform';
 
 import {
@@ -298,17 +297,15 @@ export class QueryManager {
   }): Promise<ApolloQueryResult> {
     const mutationId = this.generateQueryId();
 
-    let mutationDef = getMutationDefinition(mutation);
-    if (this.queryTransformer) {
-      mutationDef = applyTransformerToOperation(mutationDef, this.queryTransformer);
-      mutation = replaceOperationDefinition(mutation, mutationDef);
-    }
-    mutation = replaceOperationDefinition(mutation, mutationDef);
-
     // Add the fragments that were passed in to the mutation document and then
     // construct the fragment map.
     mutation = addFragmentsToDocument(mutation, fragments);
 
+    if (this.queryTransformer) {
+      mutation = applyTransformers(mutation, [this.queryTransformer]);
+    }
+
+    let mutationDef = getMutationDefinition(mutation);
     const mutationString = print(mutation);
     const queryFragmentMap = createFragmentMap(getFragmentDefinitions(mutation));
     const request = {
@@ -670,18 +667,16 @@ export class QueryManager {
       fragments = [],
     } = options;
 
-    let queryDef = getQueryDefinition(query);
+    let queryDoc = addFragmentsToDocument(query, fragments);
     // Apply the query transformer if one has been provided.
     if (this.queryTransformer) {
-      queryDef = applyTransformerToOperation(queryDef, this.queryTransformer);
+      queryDoc = applyTransformers(queryDoc, [this.queryTransformer]);
     }
-    let transformedQuery = replaceOperationDefinition(query, queryDef);
 
     // Add the fragments passed in into the query and then create the fragment map
-    transformedQuery = addFragmentsToDocument(transformedQuery, fragments);
-    const queryFragmentMap = createFragmentMap(getFragmentDefinitions(transformedQuery));
-
-    const queryString = print(transformedQuery);
+    const queryFragmentMap = createFragmentMap(getFragmentDefinitions(queryDoc));
+    const queryDef = getQueryDefinition(queryDoc);
+    const queryString = print(queryDoc);
 
     // Parse the query passed in -- this could also be done by a build plugin or tagged
     // template string
@@ -695,7 +690,7 @@ export class QueryManager {
     // the queryTransformer that could have been applied.
     let minimizedQueryString = queryString;
     let minimizedQuery = querySS;
-    let minimizedQueryDoc = transformedQuery;
+    let minimizedQueryDoc = queryDoc;
     let initialResult;
 
     if (!forceFetch) {

src/index.ts

@@ -118,7 +118,6 @@ this in the docs: http://docs.apollostack.com/`);
       fragmentDefinitionsMap[fragmentName] = [fragmentDefinition];
     }
   });
-
   return fragments.concat(fragmentDefinitions);
 }
 

src/queries/getFromAST.ts

@@ -6,7 +6,6 @@ import {
 
 import countBy = require('lodash.countby');
 import identity = require('lodash.identity');
-import cloneDeep = require('lodash.clonedeep');
 
 export function getMutationDefinition(doc: Document): OperationDefinition {
   checkDocument(doc);
@@ -105,25 +104,6 @@ string in a "gql" tag? http://docs.apollostack.com/apollo-client/core.html#gql`)
   return fragmentDef as FragmentDefinition;
 }
 
-// Modifies a document in order to replace the operation definition with another
-// operation definition. Returns a new copy of the document.
-export function replaceOperationDefinition(doc: Document,
-  newOpDef: OperationDefinition): Document {
-  checkDocument(doc);
-
-  const docCopy = cloneDeep(doc);
-
-  docCopy.definitions = doc.definitions.map((definition) => {
-    if (definition.kind === 'OperationDefinition') {
-      return newOpDef;
-    } else {
-      return definition;
-    }
-  });
-
-  return docCopy;
-}
-
 export interface FragmentMap {
   [fragmentName: string]: FragmentDefinition;
 }
@@ -143,6 +123,7 @@ export function createFragmentMap(fragments: FragmentDefinition[]): FragmentMap
 // document.
 export function addFragmentsToDocument(queryDoc: Document,
   fragments: FragmentDefinition[]): Document {
+  checkDocument(queryDoc);
   queryDoc.definitions = queryDoc.definitions.concat(fragments);
   return queryDoc;
 }

src/queries/queryTransform.ts

@@ -1,10 +1,16 @@
 import {
+  Document,
   SelectionSet,
   OperationDefinition,
+  FragmentDefinition,
   Field,
   InlineFragment,
 } from 'graphql';
 
+import {
+  checkDocument,
+} from './getFromAST';
+
 import cloneDeep = require('lodash.clonedeep');
 
 // A QueryTransformer takes a SelectionSet and transforms it in someway (in place).
@@ -13,10 +19,6 @@ export type QueryTransformer = (selectionSet: SelectionSet) => void
 // Adds a field with a given name to every node in the AST recursively.
 // Note: this mutates the AST passed in.
 export function addFieldToSelectionSet(fieldName: string, selectionSet: SelectionSet) {
-  if (selectionSet == null || selectionSet.selections == null) {
-    return selectionSet;
-  }
-
   const fieldAst: Field = {
     kind: 'Field',
     alias: null,
@@ -26,25 +28,17 @@ export function addFieldToSelectionSet(fieldName: string, selectionSet: Selectio
     },
   };
 
-  let alreadyHasThisField = false;
-  selectionSet.selections.map((selection) => {
-    // We use type assertions to make sure the selection isn't a FragmentSpread because
-    // that can't have a selectionSet.
-    if (selection.kind === 'Field' || selection.kind === 'InlineFragment') {
-      addTypenameToSelectionSet((selection as (Field | InlineFragment)).selectionSet);
-    }
-
-    if (selection.kind === 'Field' && (selection as Field).name.value === fieldName) {
-      alreadyHasThisField = true;
+  if (selectionSet && selectionSet.selections) {
+    let alreadyHasThisField = false;
+    selectionSet.selections.forEach((selection) => {
+      if (selection.kind === 'Field' && (selection as Field).name.value === fieldName) {
+        alreadyHasThisField = true;
+      }
+    });
+    if (! alreadyHasThisField) {
+      selectionSet.selections.push(fieldAst);
     }
-  });
-
-  if (! alreadyHasThisField) {
-    // Add the typename to this particular node's children
-    selectionSet.selections.push(fieldAst);
   }
-
-  return selectionSet;
 }
 
 // Adds typename fields to every node in the AST recursively.
@@ -53,20 +47,29 @@ export function addTypenameToSelectionSet(selectionSet: SelectionSet) {
   return addFieldToSelectionSet('__typename', selectionSet);
 }
 
-// Add typename field to the root query node (i.e. OperationDefinition). Returns a new
-// query tree.
-export function addTypenameToQuery(queryDef: OperationDefinition): OperationDefinition {
-  const queryClone = cloneDeep(queryDef);
-  this.addTypenameToSelectionSet(queryClone.selectionSet);
-  return queryClone;
+function traverseSelectionSet(selectionSet: SelectionSet, queryTransformers: QueryTransformer[]) {
+  if (selectionSet && selectionSet.selections) {
+    queryTransformers.forEach((transformer) => {
+      transformer(selectionSet); // transforms in place
+      selectionSet.selections.forEach((selection) => {
+        if (selection.kind === 'Field' || selection.kind === 'InlineFragment') {
+          traverseSelectionSet((selection as Field | InlineFragment).selectionSet, queryTransformers);
+        }
+      });
+    });
+  }
 }
-
-// Apply a QueryTranformer to an OperationDefinition (extracted from a query
-// or a mutation.)
-// Returns a new query tree.
-export function applyTransformerToOperation(queryDef: OperationDefinition,
-  queryTransformer: QueryTransformer): OperationDefinition {
-    const queryClone = cloneDeep(queryDef);
-    queryTransformer(queryClone.selectionSet);
-    return queryClone;
+/**
+ * Applies transformers to document and returns a new transformed document.
+ * @param {Document} doc - A GraphQL document that will be transformed
+ * @param {QueryTranformer[]} queryTransformers - transformers to be applied to the document
+ * @ return {Document} - a new transformed document
+ */
+export function applyTransformers(doc: Document, queryTransformers: QueryTransformer[]): Document {
+  checkDocument(doc);
+  const docClone = cloneDeep(doc);
+  docClone.definitions.forEach((definition) => {
+    traverseSelectionSet((definition as (OperationDefinition | FragmentDefinition)).selectionSet, queryTransformers);
+  });
+  return docClone;
 }

test/getFromAST.ts

@@ -3,7 +3,6 @@ import {
   getFragmentDefinitions,
   getQueryDefinition,
   getMutationDefinition,
-  replaceOperationDefinition,
   createFragmentMap,
   FragmentMap,
   getOperationName,
@@ -170,42 +169,6 @@ describe('AST utility functions', () => {
     assert.equal(print(actualResult), print(expectedResult));
   });
 
-  it('should replace the operation definition correctly', () => {
-    const queryWithFragments = gql`
-      fragment authorDetails on Author {
-        firstName
-        lastName
-      }
-      query {
-        author {
-          ...authorDetails
-        }
-      }`;
-    const newQueryDef = getQueryDefinition(gql`
-      query {
-        author {
-          ...authorDetails
-          __typename
-        }
-        __typename
-      }`);
-    const expectedNewQuery = gql`
-      fragment authorDetails on Author {
-        firstName
-        lastName
-      }
-
-      query {
-        author {
-          ...authorDetails
-          __typename
-        }
-        __typename
-      }`;
-    const newDoc = replaceOperationDefinition(queryWithFragments, newQueryDef);
-    assert.equal(print(newDoc), print(expectedNewQuery));
-  });
-
   it('should create the fragment map correctly', () => {
     const fragments = getFragmentDefinitions(gql`
       fragment authorDetails on Author {