[RFC] Support iterable values as inputs and outputs (#449)

This adds support for returning any Iterable from a resolver function expecting a List type, rather than only Arrays, by using the `iterall` library.

This also adds support for accepting any Iterable as input for arguments or variable values which expect a List type.

.flowconfig

@@ -3,7 +3,6 @@
 .*/dist/.*
 .*/coverage/.*
 .*/resources/.*
-.*/node_modules/.*
 
 [include]
 

package.json

@@ -34,6 +34,9 @@
     "preversion": ". ./resources/checkgit.sh && npm test",
     "prepublish": ". ./resources/prepublish.sh"
   },
+  "dependencies": {
+    "iterall": "1.0.2"
+  },
   "devDependencies": {
     "babel-cli": "6.10.1",
     "babel-eslint": "6.1.0",

src/execution/__tests__/lists-test.js

@@ -18,6 +18,7 @@ import { parse } from '../../language';
 import {
   GraphQLSchema,
   GraphQLObjectType,
+  GraphQLString,
   GraphQLInt,
   GraphQLList,
   GraphQLNonNull
@@ -67,6 +68,48 @@ function check(testType, testData, expected) {
   };
 }
 
+describe('Execute: Accepts any iterable as list value', () => {
+
+  it('Accepts a Set as a List value', check(
+    new GraphQLList(GraphQLString),
+    new Set([ 'apple', 'banana', 'apple', 'coconut' ]),
+    { data: { nest: { test: [ 'apple', 'banana', 'coconut' ] } } }
+  ));
+
+  function *yieldItems() {
+    yield 'one';
+    yield 2;
+    yield true;
+  }
+
+  it('Accepts an Generator function as a List value', check(
+    new GraphQLList(GraphQLString),
+    yieldItems(),
+    { data: { nest: { test: [ 'one', '2', 'true' ] } } }
+  ));
+
+  function getArgs() {
+    return arguments;
+  }
+
+  it('Accepts function arguments as a List value', check(
+    new GraphQLList(GraphQLString),
+    getArgs('one', 'two'),
+    { data: { nest: { test: [ 'one', 'two' ] } } }
+  ));
+
+  it('Does not accept (Iterable) String-literal as a List value', check(
+    new GraphQLList(GraphQLString),
+    'Singluar',
+    { data: { nest: { test: null } },
+      errors: [ {
+        message: 'Expected Iterable, but did not find one for field DataType.test.',
+        locations: [ { line: 1, column: 10 } ]
+      } ] }
+  ));
+
+});
+
 describe('Execute: Handles list nullability', () => {
 
   describe('[T]', () => {

src/execution/execute.js

@@ -8,6 +8,8 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import { forEach, isCollection } from 'iterall';
+
 import { GraphQLError, locatedError } from '../error';
 import find from '../jsutils/find';
 import invariant from '../jsutils/invariant';
@@ -829,16 +831,17 @@ function completeListValue(
   result: mixed
 ): mixed {
   invariant(
-    Array.isArray(result),
-    `User Error: expected iterable, but did not find one for field ${
+    isCollection(result),
+    `Expected Iterable, but did not find one for field ${
       info.parentType.name}.${info.fieldName}.`
   );
 
   // This is specified as a simple map, however we're optimizing the path
   // where the list contains no Promises by avoiding creating another Promise.
   const itemType = returnType.ofType;
   let containsPromise = false;
-  const completedResults = result.map((item, index) => {
+  const completedResults = [];
+  forEach((result: any), (item, index) => {
     // No need to modify the info object containing the path,
     // since from here on it is not ever accessed by resolver functions.
     const fieldPath = path.concat([ index ]);
@@ -854,7 +857,7 @@ function completeListValue(
     if (!containsPromise && isThenable(completedItem)) {
       containsPromise = true;
     }
-    return completedItem;
+    completedResults.push(completedItem);
   });
 
   return containsPromise ? Promise.all(completedResults) : completedResults;

src/execution/values.js

@@ -8,6 +8,8 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import { forEach, isCollection } from 'iterall';
+
 import { GraphQLError } from '../error';
 import invariant from '../jsutils/invariant';
 import isNullish from '../jsutils/isNullish';
@@ -138,9 +140,12 @@ function coerceValue(type: GraphQLInputType, value: mixed): mixed {
 
   if (type instanceof GraphQLList) {
     const itemType = type.ofType;
-    // TODO: support iterable input
-    if (Array.isArray(_value)) {
-      return _value.map(item => coerceValue(itemType, item));
+    if (isCollection(_value)) {
+      const coercedValues = [];
+      forEach((_value: any), item => {
+        coercedValues.push(coerceValue(itemType, item));
+      });
+      return coercedValues;
     }
     return [ coerceValue(itemType, _value) ];
   }

src/utilities/astFromValue.js

@@ -8,6 +8,8 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import { forEach, isCollection } from 'iterall';
+
 import invariant from '../jsutils/invariant';
 import isNullish from '../jsutils/isNullish';
 import type {
@@ -79,9 +81,9 @@ export function astFromValue(
   // the value is not an array, convert the value using the list's item type.
   if (type instanceof GraphQLList) {
     const itemType = type.ofType;
-    if (Array.isArray(_value)) {
+    if (isCollection(_value)) {
       const valuesASTs = [];
-      _value.forEach(item => {
+      forEach((_value: any), item => {
         const itemAST = astFromValue(item, itemType);
         if (itemAST) {
           valuesASTs.push(itemAST);

src/utilities/isValidJSValue.js

@@ -8,6 +8,8 @@
  *  of patent rights can be found in the PATENTS file in the same directory.
  */
 
+import { forEach, isCollection } from 'iterall';
+
 import invariant from '../jsutils/invariant';
 import isNullish from '../jsutils/isNullish';
 import {
@@ -44,13 +46,14 @@ export function isValidJSValue(value: mixed, type: GraphQLInputType): [string] {
   // Lists accept a non-list value as a list of one.
   if (type instanceof GraphQLList) {
     const itemType = type.ofType;
-    if (Array.isArray(value)) {
-      return value.reduce((acc, item, index) => {
-        const errors = isValidJSValue(item, itemType);
-        return acc.concat(errors.map(error =>
+    if (isCollection(value)) {
+      const errors = [];
+      forEach((value: any), (item, index) => {
+        errors.push.apply(errors, isValidJSValue(item, itemType).map(error =>
           `In element #${index}: ${error}`
         ));
-      }, []);
+      });
+      return errors;
     }
     return isValidJSValue(value, itemType);
   }