Execute serially supporting sync execution. (#1198)

Refactors executeFieldsSerially to return MaybePromise

Fixes #1195

Author: leebyron

src/execution/__tests__/sync-test.js

@@ -31,6 +31,17 @@ describe('Execute: synchronously when possible', () => {
         },
       },
     }),
+    mutation: new GraphQLObjectType({
+      name: 'Mutation',
+      fields: {
+        syncMutationField: {
+          type: GraphQLString,
+          resolve(rootValue) {
+            return rootValue;
+          },
+        },
+      },
+    }),
   });
 
   it('does not return a Promise for initial errors', () => {
@@ -61,6 +72,16 @@ describe('Execute: synchronously when possible', () => {
     expect(result).to.deep.equal({ data: { syncField: 'rootValue' } });
   });
 
+  it('does not return a Promise if mutation fields are all synchronous', () => {
+    const doc = 'mutation Example { syncMutationField }';
+    const result = execute({
+      schema,
+      document: parse(doc),
+      rootValue: 'rootValue',
+    });
+    expect(result).to.deep.equal({ data: { syncMutationField: 'rootValue' } });
+  });
+
   it('returns a Promise if any field is asynchronous', async () => {
     const doc = 'query Example { syncField, asyncField }';
     const result = execute({

src/execution/execute.js

@@ -9,10 +9,13 @@
 
 import { forEach, isCollection } from 'iterall';
 import { GraphQLError, locatedError } from '../error';
+import getPromise from '../jsutils/getPromise';
 import invariant from '../jsutils/invariant';
 import isInvalid from '../jsutils/isInvalid';
 import isNullish from '../jsutils/isNullish';
 import memoize3 from '../jsutils/memoize3';
+import promiseForObject from '../jsutils/promiseForObject';
+import promiseReduce from '../jsutils/promiseReduce';
 import type { ObjMap } from '../jsutils/ObjMap';
 import type { MaybePromise } from '../jsutils/MaybePromise';
 
@@ -465,33 +468,33 @@ function executeFieldsSerially(
   sourceValue: mixed,
   path: ResponsePath | void,
   fields: ObjMap<Array<FieldNode>>,
-): Promise<ObjMap<mixed>> {
-  return Object.keys(fields).reduce(
-    (prevPromise, responseName) =>
-      prevPromise.then(results => {
-        const fieldNodes = fields[responseName];
-        const fieldPath = addPath(path, responseName);
-        const result = resolveField(
-          exeContext,
-          parentType,
-          sourceValue,
-          fieldNodes,
-          fieldPath,
-        );
-        if (result === undefined) {
-          return results;
-        }
-        const promise = getPromise(result);
-        if (promise) {
-          return promise.then(resolvedResult => {
-            results[responseName] = resolvedResult;
-            return results;
-          });
-        }
-        results[responseName] = result;
+): MaybePromise<ObjMap<mixed>> {
+  return promiseReduce(
+    Object.keys(fields),
+    (results, responseName) => {
+      const fieldNodes = fields[responseName];
+      const fieldPath = addPath(path, responseName);
+      const result = resolveField(
+        exeContext,
+        parentType,
+        sourceValue,
+        fieldNodes,
+        fieldPath,
+      );
+      if (result === undefined) {
         return results;
-      }),
-    Promise.resolve({}),
+      }
+      const promise = getPromise(result);
+      if (promise) {
+        return promise.then(resolvedResult => {
+          results[responseName] = resolvedResult;
+          return results;
+        });
+      }
+      results[responseName] = result;
+      return results;
+    },
+    Object.create(null),
   );
 }
 
@@ -662,24 +665,6 @@ function doesFragmentConditionMatch(
   return false;
 }
 
-/**
- * This function transforms a JS object `ObjMap<Promise<T>>` into
- * a `Promise<ObjMap<T>>`
- *
- * This is akin to bluebird's `Promise.props`, but implemented only using
- * `Promise.all` so it will work with any implementation of ES6 promises.
- */
-function promiseForObject<T>(object: ObjMap<Promise<T>>): Promise<ObjMap<T>> {
-  const keys = Object.keys(object);
-  const valuesAndPromises = keys.map(name => object[name]);
-  return Promise.all(valuesAndPromises).then(values =>
-    values.reduce((resolvedObject, value, i) => {
-      resolvedObject[keys[i]] = value;
-      return resolvedObject;
-    }, Object.create(null)),
-  );
-}
-
 /**
  * Implements the logic to compute the key of a given field's entry
  */
@@ -1346,20 +1331,6 @@ export const defaultFieldResolver: GraphQLFieldResolver<any, *> = function(
   }
 };
 
-/**
- * Only returns the value if it acts like a Promise, i.e. has a "then" function,
- * otherwise returns void.
- */
-function getPromise<T>(value: Promise<T> | mixed): Promise<T> | void {
-  if (
-    typeof value === 'object' &&
-    value !== null &&
-    typeof value.then === 'function'
-  ) {
-    return (value: any);
-  }
-}
-
 /**
  * This method looks up the field on the given type defintion.
  * It has special casing for the two introspection fields, __schema

src/jsutils/getPromise.js

@@ -0,0 +1,24 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+/**
+ * Only returns the value if it acts like a Promise, i.e. has a "then" function,
+ * otherwise returns void.
+ */
+export default function getPromise<T>(
+  value: Promise<T> | mixed,
+): Promise<T> | void {
+  if (
+    typeof value === 'object' &&
+    value !== null &&
+    typeof value.then === 'function'
+  ) {
+    return (value: any);
+  }
+}

src/jsutils/promiseForObject.js

@@ -0,0 +1,30 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import type { ObjMap } from './ObjMap';
+
+/**
+ * This function transforms a JS object `ObjMap<Promise<T>>` into
+ * a `Promise<ObjMap<T>>`
+ *
+ * This is akin to bluebird's `Promise.props`, but implemented only using
+ * `Promise.all` so it will work with any implementation of ES6 promises.
+ */
+export default function promiseForObject<T>(
+  object: ObjMap<Promise<T>>,
+): Promise<ObjMap<T>> {
+  const keys = Object.keys(object);
+  const valuesAndPromises = keys.map(name => object[name]);
+  return Promise.all(valuesAndPromises).then(values =>
+    values.reduce((resolvedObject, value, i) => {
+      resolvedObject[keys[i]] = value;
+      return resolvedObject;
+    }, Object.create(null)),
+  );
+}

src/jsutils/promiseReduce.js

@@ -0,0 +1,33 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import getPromise from './getPromise';
+import type { MaybePromise } from './MaybePromise';
+
+/**
+ * Similar to Array.prototype.reduce(), however the reducing callback may return
+ * a Promise, in which case reduction will continue after each promise resolves.
+ *
+ * If the callback does not return a Promise, then this function will also not
+ * return a Promise.
+ */
+export default function promiseReduce<T, U>(
+  values: $ReadOnlyArray<T>,
+  callback: (U, T) => MaybePromise<U>,
+  initialValue: MaybePromise<U>,
+): MaybePromise<U> {
+  return values.reduce((previous, value) => {
+    const promise = getPromise(previous);
+    if (promise) {
+      return promise.then(resolved => callback(resolved, value));
+    }
+    // Previous is not Promise<U>, so it is U.
+    return callback((previous: any), value);
+  }, initialValue);
+}