From 499a75939f70c4863d44149371d6a99d57ff7c35 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Tue, 20 Feb 2018 23:28:27 -0500 Subject: [PATCH] Replace getPromise with isPromise (#1255) * Replace getPromise with isPromise Simplifies logic within the executor, uses the %checks predicate from Flow * Improve flow types --- src/execution/execute.js | 74 ++++++++++++++++-------------------- src/jsutils/getPromise.js | 24 ------------ src/jsutils/isPromise.js | 20 ++++++++++ src/jsutils/promiseReduce.js | 17 ++++----- 4 files changed, 60 insertions(+), 75 deletions(-) delete mode 100644 src/jsutils/getPromise.js create mode 100644 src/jsutils/isPromise.js diff --git a/src/execution/execute.js b/src/execution/execute.js index 602eed2c21..72dbcec6cc 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -9,10 +9,10 @@ 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 isPromise from '../jsutils/isPromise'; import memoize3 from '../jsutils/memoize3'; import promiseForObject from '../jsutils/promiseForObject'; import promiseReduce from '../jsutils/promiseReduce'; @@ -229,9 +229,8 @@ function buildResponse( context: ExecutionContext, data: MaybePromise | null>, ) { - const promise = getPromise(data); - if (promise) { - return promise.then(resolved => buildResponse(context, resolved)); + if (isPromise(data)) { + return data.then(resolved => buildResponse(context, resolved)); } return context.errors.length === 0 ? { data } @@ -403,9 +402,8 @@ function executeOperation( operation.operation === 'mutation' ? executeFieldsSerially(exeContext, type, rootValue, path, fields) : executeFields(exeContext, type, rootValue, path, fields); - const promise = getPromise(result); - if (promise) { - return promise.then(undefined, error => { + if (isPromise(result)) { + return result.then(undefined, error => { exeContext.errors.push(error); return Promise.resolve(null); }); @@ -484,9 +482,8 @@ function executeFieldsSerially( if (result === undefined) { return results; } - const promise = getPromise(result); - if (promise) { - return promise.then(resolvedResult => { + if (isPromise(result)) { + return result.then(resolvedResult => { results[responseName] = resolvedResult; return results; }); @@ -525,7 +522,7 @@ function executeFields( return results; } results[responseName] = result; - if (getPromise(result)) { + if (!containsPromise && isPromise(result)) { containsPromise = true; } return results; @@ -684,7 +681,7 @@ function resolveField( source: mixed, fieldNodes: $ReadOnlyArray, path: ResponsePath, -): mixed { +): MaybePromise { const fieldNode = fieldNodes[0]; const fieldName = fieldNode.name.value; @@ -773,8 +770,7 @@ export function resolveFieldValueOrError( const context = exeContext.contextValue; const result = resolveFn(source, args, context, info); - const promise = getPromise(result); - return promise ? promise.then(undefined, asErrorInstance) : result; + return isPromise(result) ? result.then(undefined, asErrorInstance) : result; } catch (error) { return asErrorInstance(error); } @@ -795,7 +791,7 @@ function completeValueCatchingError( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise { // 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)) { @@ -820,13 +816,12 @@ function completeValueCatchingError( path, result, ); - const promise = getPromise(completed); - if (promise) { + if (isPromise(completed)) { // If `completeValueWithLocatedError` returned a rejected promise, log // 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 => { + return completed.then(undefined, error => { exeContext.errors.push(error); return Promise.resolve(null); }); @@ -849,7 +844,7 @@ function completeValueWithLocatedError( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise { try { const completed = completeValue( exeContext, @@ -859,9 +854,8 @@ function completeValueWithLocatedError( path, result, ); - const promise = getPromise(completed); - if (promise) { - return promise.then(undefined, error => + if (isPromise(completed)) { + return completed.then(undefined, error => Promise.reject( locatedError( asErrorInstance(error), @@ -909,11 +903,10 @@ function completeValue( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise { // If result is a Promise, apply-lift over completeValue. - const promise = getPromise(result); - if (promise) { - return promise.then(resolved => + if (isPromise(result)) { + return result.then(resolved => completeValue(exeContext, returnType, fieldNodes, info, path, resolved), ); } @@ -1012,7 +1005,7 @@ function completeListValue( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise<$ReadOnlyArray> { invariant( isCollection(result), `Expected Iterable, but did not find one for field ${ @@ -1038,7 +1031,7 @@ function completeListValue( item, ); - if (!containsPromise && getPromise(completedItem)) { + if (!containsPromise && isPromise(completedItem)) { containsPromise = true; } completedResults.push(completedItem); @@ -1074,14 +1067,13 @@ function completeAbstractValue( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise> { const runtimeType = returnType.resolveType ? returnType.resolveType(result, exeContext.contextValue, info) : defaultResolveTypeFn(result, exeContext.contextValue, info, returnType); - const promise = getPromise(runtimeType); - if (promise) { - return promise.then(resolvedRuntimeType => + if (isPromise(runtimeType)) { + return runtimeType.then(resolvedRuntimeType => completeObjectValue( exeContext, ensureValidRuntimeType( @@ -1103,7 +1095,7 @@ function completeAbstractValue( return completeObjectValue( exeContext, ensureValidRuntimeType( - ((runtimeType: any): ?GraphQLObjectType | string), + runtimeType, exeContext, returnType, fieldNodes, @@ -1163,17 +1155,16 @@ function completeObjectValue( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise> { // 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. if (returnType.isTypeOf) { const isTypeOf = returnType.isTypeOf(result, exeContext.contextValue, info); - const promise = getPromise(isTypeOf); - if (promise) { - return promise.then(isTypeOfResult => { - if (!isTypeOfResult) { + if (isPromise(isTypeOf)) { + return isTypeOf.then(resolvedIsTypeOf => { + if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } return collectAndExecuteSubfields( @@ -1220,7 +1211,7 @@ function collectAndExecuteSubfields( info: GraphQLResolveInfo, path: ResponsePath, result: mixed, -): mixed { +): MaybePromise> { // Collect sub-fields to execute to complete this value. const subFieldNodes = collectSubfields(exeContext, returnType, fieldNodes); return executeFields(exeContext, returnType, result, path, subFieldNodes); @@ -1289,9 +1280,8 @@ function defaultResolveTypeFn( if (type.isTypeOf) { const isTypeOfResult = type.isTypeOf(value, context, info); - const promise = getPromise(isTypeOfResult); - if (promise) { - promisedIsTypeOfResults[i] = promise; + if (isPromise(isTypeOfResult)) { + promisedIsTypeOfResults[i] = isTypeOfResult; } else if (isTypeOfResult) { return type; } diff --git a/src/jsutils/getPromise.js b/src/jsutils/getPromise.js deleted file mode 100644 index e56a3af76e..0000000000 --- a/src/jsutils/getPromise.js +++ /dev/null @@ -1,24 +0,0 @@ -/** - * 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 strict - */ - -/** - * Only returns the value if it acts like a Promise, i.e. has a "then" function, - * otherwise returns void. - */ -export default function getPromise( - value: Promise | mixed, -): Promise | void { - if ( - typeof value === 'object' && - value !== null && - typeof value.then === 'function' - ) { - return (value: any); - } -} diff --git a/src/jsutils/isPromise.js b/src/jsutils/isPromise.js new file mode 100644 index 0000000000..d53c9eea6b --- /dev/null +++ b/src/jsutils/isPromise.js @@ -0,0 +1,20 @@ +/** + * 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 strict + */ + +/** + * Returns true if the value acts like a Promise, i.e. has a "then" function, + * otherwise returns false. + */ +declare function isPromise(value: mixed): boolean %checks(value instanceof + Promise); + +// eslint-disable-next-line no-redeclare +export default function isPromise(value) { + return Boolean(value && typeof value.then === 'function'); +} diff --git a/src/jsutils/promiseReduce.js b/src/jsutils/promiseReduce.js index c4f4cdfb74..a09837b8ac 100644 --- a/src/jsutils/promiseReduce.js +++ b/src/jsutils/promiseReduce.js @@ -7,7 +7,7 @@ * @flow strict */ -import getPromise from './getPromise'; +import isPromise from './isPromise'; import type { MaybePromise } from './MaybePromise'; /** @@ -22,12 +22,11 @@ export default function promiseReduce( callback: (U, T) => MaybePromise, initialValue: MaybePromise, ): MaybePromise { - return values.reduce((previous, value) => { - const promise = getPromise(previous); - if (promise) { - return promise.then(resolved => callback(resolved, value)); - } - // Previous is not Promise, so it is U. - return callback((previous: any), value); - }, initialValue); + return values.reduce( + (previous, value) => + isPromise(previous) + ? previous.then(resolved => callback(resolved, value)) + : callback(previous, value), + initialValue, + ); }