use { data, errors } so no mutating exeContext

Author: mohawk2

src/execution/execute.js

@@ -99,7 +99,6 @@ export type ExecutionContext = {
   operation: OperationDefinitionNode,
   variableValues: { [variable: string]: mixed },
   fieldResolver: GraphQLFieldResolver<any, any>,
-  errors: Array<GraphQLError>,
 };
 
 /**
@@ -113,6 +112,23 @@ export type ExecutionResult = {
   data?: ObjMap<mixed>,
 };
 
+/**
+ * A partial result of GraphQL execution. `data` and `errors` as above.
+ * "NP" = "Not Promise".
+ */
+export type ExecutionPartialResultNP<T> = {
+  errors?: $ReadOnlyArray<GraphQLError>,
+  data?: T | null,
+};
+
+/**
+ * A partial result of GraphQL execution. `data` and `errors` as above.
+ * This version might be a promise.
+ */
+export type ExecutionPartialResult<T> = MaybePromise<
+  ExecutionPartialResultNP<T>,
+>;
+
 export type ExecutionArgs = {|
   schema: GraphQLSchema,
   document: DocumentNode,
@@ -218,24 +234,21 @@ function executeImpl(
   // be executed. An execution which encounters errors will still result in a
   // resolved Promise.
   const data = executeOperation(context, context.operation, rootValue);
-  return buildResponse(context, data);
+  return buildResponse(data);
 }
 
 /**
- * Given a completed execution context and data, build the { errors, data }
- * response defined by the "Response" section of the GraphQL specification.
+ * Strip out `errors` if empty.
  */
-function buildResponse(
-  context: ExecutionContext,
-  data: MaybePromise<ObjMap<mixed> | null>,
-) {
-  const promise = getPromise(data);
+function buildResponse(result: ExecutionPartialResult<mixed>) {
+  const promise = getPromise(result);
   if (promise) {
-    return promise.then(resolved => buildResponse(context, resolved));
+    return promise.then(buildResponse);
   }
-  return context.errors.length === 0
-    ? { data }
-    : { errors: context.errors, data };
+  if (result.data && (!result.errors || !result.errors.length)) {
+    return { data: result.data };
+  }
+  return result;
 }
 
 /**
@@ -370,7 +383,6 @@ export function buildExecutionContext(
     operation,
     variableValues,
     fieldResolver: fieldResolver || defaultFieldResolver,
-    errors,
   };
 }
 
@@ -381,7 +393,7 @@ function executeOperation(
   exeContext: ExecutionContext,
   operation: OperationDefinitionNode,
   rootValue: mixed,
-): MaybePromise<ObjMap<mixed> | null> {
+): ExecutionPartialResult<mixed> {
   const type = getOperationRootType(exeContext.schema, operation);
   const fields = collectFields(
     exeContext,
@@ -405,15 +417,14 @@ function executeOperation(
         : executeFields(exeContext, type, rootValue, path, fields);
     const promise = getPromise(result);
     if (promise) {
-      return promise.then(undefined, error => {
-        exeContext.errors.push(error);
-        return Promise.resolve(null);
-      });
+      return promise.then(undefined, error => ({
+        data: null,
+        errors: [error],
+      }));
     }
     return result;
   } catch (error) {
-    exeContext.errors.push(error);
-    return null;
+    return { data: null, errors: [error] };
   }
 }
 
@@ -468,8 +479,8 @@ function executeFieldsSerially(
   sourceValue: mixed,
   path: ResponsePath | void,
   fields: ObjMap<Array<FieldNode>>,
-): MaybePromise<ObjMap<mixed>> {
-  return promiseReduce(
+): ExecutionPartialResult<mixed> {
+  const finalResult = promiseReduce(
     Object.keys(fields),
     (results, responseName) => {
       const fieldNodes = fields[responseName];
@@ -481,7 +492,7 @@ function executeFieldsSerially(
         fieldNodes,
         fieldPath,
       );
-      if (result === undefined) {
+      if (result === null) {
         return results;
       }
       const promise = getPromise(result);
@@ -496,6 +507,8 @@ function executeFieldsSerially(
     },
     Object.create(null),
   );
+  const promise = getPromise(finalResult);
+  return promise ? promise.then(mergeEPRs) : mergeEPRs((finalResult: any));
 }
 
 /**
@@ -508,7 +521,7 @@ function executeFields(
   sourceValue: mixed,
   path: ResponsePath | void,
   fields: ObjMap<Array<FieldNode>>,
-): MaybePromise<ObjMap<mixed>> {
+): ExecutionPartialResult<mixed> {
   let containsPromise = false;
 
   const finalResults = Object.keys(fields).reduce((results, responseName) => {
@@ -521,7 +534,7 @@ function executeFields(
       fieldNodes,
       fieldPath,
     );
-    if (result === undefined) {
+    if (result === null) {
       return results;
     }
     results[responseName] = result;
@@ -533,14 +546,31 @@ function executeFields(
 
   // If there are no promises, we can just return the object
   if (!containsPromise) {
-    return finalResults;
+    return mergeEPRs(finalResults);
   }
 
   // Otherwise, results is a map from field name to the result
   // of resolving that field, which is possibly a promise. Return
   // a promise that will return this same map, but with any
   // promises replaced with the values they resolved to.
-  return promiseForObject(finalResults);
+  return promiseForObject(finalResults).then(mergeEPRs);
+}
+
+function mergeEPRs(
+  eprMap: ObjMap<ExecutionPartialResultNP<mixed>>,
+): ExecutionPartialResultNP<mixed> {
+  const keys = Object.keys(eprMap);
+  const values = keys.map(name => eprMap[name]);
+  return values.reduce(
+    (merged, value, i) => {
+      merged.data[keys[i]] = value.data;
+      if (value.errors && value.errors.length) {
+        merged.errors.push(...value.errors);
+      }
+      return merged;
+    },
+    { data: Object.create(null), errors: [] },
+  );
 }
 
 /**
@@ -684,13 +714,13 @@ function resolveField(
   source: mixed,
   fieldNodes: $ReadOnlyArray<FieldNode>,
   path: ResponsePath,
-): mixed {
+): ExecutionPartialResult<mixed> | null {
   const fieldNode = fieldNodes[0];
   const fieldName = fieldNode.name.value;
 
   const fieldDef = getFieldDef(exeContext.schema, parentType, fieldName);
   if (!fieldDef) {
-    return;
+    return null;
   }
 
   const resolveFn = fieldDef.resolve || exeContext.fieldResolver;
@@ -795,7 +825,7 @@ function completeValueCatchingError(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   // If the field type is non-nullable, then it is resolved without any
   // protection from errors, however it still properly locates the error.
   if (isNonNullType(returnType)) {
@@ -826,17 +856,16 @@ function completeValueCatchingError(
       // the rejection error and resolve to null.
       // Note: we don't rely on a `catch` method, but we do expect "thenable"
       // to take a second callback for the error case.
-      return promise.then(undefined, error => {
-        exeContext.errors.push(error);
-        return Promise.resolve(null);
-      });
+      return promise.then(undefined, error => ({
+        data: null,
+        errors: [error],
+      }));
     }
     return completed;
   } catch (error) {
     // If `completeValueWithLocatedError` returned abruptly (threw an error),
     // log the error and return null.
-    exeContext.errors.push(error);
-    return null;
+    return { data: null, errors: [error] };
   }
 }
 
@@ -849,7 +878,7 @@ function completeValueWithLocatedError(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   try {
     const completed = completeValue(
       exeContext,
@@ -909,7 +938,7 @@ function completeValue(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   // If result is a Promise, apply-lift over completeValue.
   const promise = getPromise(result);
   if (promise) {
@@ -934,7 +963,7 @@ function completeValue(
       path,
       result,
     );
-    if (completed === null) {
+    if (completed.data === null) {
       throw new Error(
         `Cannot return null for non-nullable field ${info.parentType.name}.${
           info.fieldName
@@ -946,7 +975,7 @@ function completeValue(
 
   // If result value is null-ish (null, undefined, or NaN) then return null.
   if (isNullish(result)) {
-    return null;
+    return { data: null };
   }
 
   // If field type is List, complete each item in the list with the inner type
@@ -964,7 +993,7 @@ function completeValue(
   // If field type is a leaf type, Scalar or Enum, serialize to a valid value,
   // returning null if serialization is not possible.
   if (isLeafType(returnType)) {
-    return completeLeafValue(returnType, result);
+    return { data: completeLeafValue(returnType, result) };
   }
 
   // If field type is an abstract type, Interface or Union, determine the
@@ -1012,7 +1041,7 @@ function completeListValue(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   invariant(
     isCollection(result),
     `Expected Iterable, but did not find one for field ${
@@ -1044,7 +1073,23 @@ function completeListValue(
     completedResults.push(completedItem);
   });
 
-  return containsPromise ? Promise.all(completedResults) : completedResults;
+  return containsPromise
+    ? Promise.all(completedResults).then(flattenEPRs)
+    : flattenEPRs(completedResults);
+}
+
+function flattenEPRs(
+  eprs: $ReadOnlyArray<ExecutionPartialResultNP<mixed>>,
+): ExecutionPartialResultNP<mixed> {
+  const errors = [];
+  const data = [];
+  forEach((eprs: any), item => {
+    data.push(item.data);
+    if (item.errors && item.errors.length) {
+      errors.push(...item.errors);
+    }
+  });
+  return { data, errors };
 }
 
 /**
@@ -1074,7 +1119,7 @@ function completeAbstractValue(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   const runtimeType = returnType.resolveType
     ? returnType.resolveType(result, exeContext.contextValue, info)
     : defaultResolveTypeFn(result, exeContext.contextValue, info, returnType);
@@ -1163,7 +1208,7 @@ function completeObjectValue(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   // If there is an isTypeOf predicate function, call it with the
   // current result. If isTypeOf returns false, then raise an error rather
   // than continuing execution.
@@ -1220,7 +1265,7 @@ function collectAndExecuteSubfields(
   info: GraphQLResolveInfo,
   path: ResponsePath,
   result: mixed,
-): mixed {
+): ExecutionPartialResult<mixed> {
   // Collect sub-fields to execute to complete this value.
   const subFieldNodes = collectSubfields(exeContext, returnType, fieldNodes);
   return executeFields(exeContext, returnType, result, path, subFieldNodes);