Use correct terminology around iterators and iterable (#3041)

Author: IvanGoncharov

src/execution/execute.js

@@ -10,7 +10,7 @@ import { isObjectLike } from '../jsutils/isObjectLike';
 import { promiseReduce } from '../jsutils/promiseReduce';
 import { promiseForObject } from '../jsutils/promiseForObject';
 import { addPath, pathToArray } from '../jsutils/Path';
-import { isIteratableObject } from '../jsutils/isIteratableObject';
+import { isIterableObject } from '../jsutils/isIterableObject';
 
 import type { GraphQLFormattedError } from '../error/formatError';
 import { GraphQLError } from '../error/GraphQLError';
@@ -822,7 +822,7 @@ function completeListValue(
   path: Path,
   result: mixed,
 ): PromiseOrValue<$ReadOnlyArray<mixed>> {
-  if (!isIteratableObject(result)) {
+  if (!isIterableObject(result)) {
     throw new GraphQLError(
       `Expected Iterable, but did not find one for field "${info.parentType.name}.${info.fieldName}".`,
     );

src/jsutils/__tests__/isAsyncIterable-test.js

@@ -6,8 +6,8 @@ import { isAsyncIterable } from '../isAsyncIterable';
 
 describe('isAsyncIterable', () => {
   it('should return `true` for AsyncIterable', () => {
-    const asyncIteratable = { [Symbol.asyncIterator]: identityFunc };
-    expect(isAsyncIterable(asyncIteratable)).to.equal(true);
+    const asyncIterable = { [Symbol.asyncIterator]: identityFunc };
+    expect(isAsyncIterable(asyncIterable)).to.equal(true);
 
     // istanbul ignore next (Never called and use just as a placeholder)
     async function* asyncGeneratorFunc() {
@@ -16,7 +16,7 @@ describe('isAsyncIterable', () => {
 
     expect(isAsyncIterable(asyncGeneratorFunc())).to.equal(true);
 
-    // But async generator function itself is not iteratable
+    // But async generator function itself is not iterable
     expect(isAsyncIterable(asyncGeneratorFunc)).to.equal(false);
   });
 
@@ -34,18 +34,21 @@ describe('isAsyncIterable', () => {
     expect(isAsyncIterable({})).to.equal(false);
     expect(isAsyncIterable({ iterable: true })).to.equal(false);
 
-    const iterator = { [Symbol.iterator]: identityFunc };
-    expect(isAsyncIterable(iterator)).to.equal(false);
+    const asyncIteratorWithoutSymbol = { next: identityFunc };
+    expect(isAsyncIterable(asyncIteratorWithoutSymbol)).to.equal(false);
+
+    const nonAsyncIterable = { [Symbol.iterator]: identityFunc };
+    expect(isAsyncIterable(nonAsyncIterable)).to.equal(false);
 
     // istanbul ignore next (Never called and use just as a placeholder)
     function* generatorFunc() {
       /* do nothing */
     }
     expect(isAsyncIterable(generatorFunc())).to.equal(false);
 
-    const invalidAsyncIteratable = {
+    const invalidAsyncIterable = {
       [Symbol.asyncIterator]: { next: identityFunc },
     };
-    expect(isAsyncIterable(invalidAsyncIteratable)).to.equal(false);
+    expect(isAsyncIterable(invalidAsyncIterable)).to.equal(false);
   });
 });

src/jsutils/__tests__/isIterableObject-test.js

@@ -0,0 +1,71 @@
+import { expect } from 'chai';
+import { describe, it } from 'mocha';
+
+import { identityFunc } from '../identityFunc';
+import { isIterableObject } from '../isIterableObject';
+
+describe('isIterableObject', () => {
+  it('should return `true` for collections', () => {
+    expect(isIterableObject([])).to.equal(true);
+    expect(isIterableObject(new Int8Array(1))).to.equal(true);
+
+    // eslint-disable-next-line no-new-wrappers
+    expect(isIterableObject(new String('ABC'))).to.equal(true);
+
+    function getArguments() {
+      return arguments;
+    }
+    expect(isIterableObject(getArguments())).to.equal(true);
+
+    const iterable = { [Symbol.iterator]: identityFunc };
+    expect(isIterableObject(iterable)).to.equal(true);
+
+    // istanbul ignore next (Never called and use just as a placeholder)
+    function* generatorFunc() {
+      /* do nothing */
+    }
+    expect(isIterableObject(generatorFunc())).to.equal(true);
+
+    // But generator function itself is not iterable
+    expect(isIterableObject(generatorFunc)).to.equal(false);
+  });
+
+  it('should return `false` for non-collections', () => {
+    expect(isIterableObject(null)).to.equal(false);
+    expect(isIterableObject(undefined)).to.equal(false);
+
+    expect(isIterableObject('ABC')).to.equal(false);
+    expect(isIterableObject('0')).to.equal(false);
+    expect(isIterableObject('')).to.equal(false);
+
+    expect(isIterableObject(1)).to.equal(false);
+    expect(isIterableObject(0)).to.equal(false);
+    expect(isIterableObject(NaN)).to.equal(false);
+    // eslint-disable-next-line no-new-wrappers
+    expect(isIterableObject(new Number(123))).to.equal(false);
+
+    expect(isIterableObject(true)).to.equal(false);
+    expect(isIterableObject(false)).to.equal(false);
+    // eslint-disable-next-line no-new-wrappers
+    expect(isIterableObject(new Boolean(true))).to.equal(false);
+
+    expect(isIterableObject({})).to.equal(false);
+    expect(isIterableObject({ iterable: true })).to.equal(false);
+
+    const iteratorWithoutSymbol = { next: identityFunc };
+    expect(isIterableObject(iteratorWithoutSymbol)).to.equal(false);
+
+    const invalidIterable = {
+      [Symbol.iterator]: { next: identityFunc },
+    };
+    expect(isIterableObject(invalidIterable)).to.equal(false);
+
+    const arrayLike = {};
+    arrayLike[0] = 'Alpha';
+    arrayLike[1] = 'Bravo';
+    arrayLike[2] = 'Charlie';
+    arrayLike.length = 3;
+
+    expect(isIterableObject(arrayLike)).to.equal(false);
+  });
+});

src/jsutils/__tests__/isIteratableObject-test.js

@@ -1,6 +1,6 @@
 /**
  * Returns true if the provided object implements the AsyncIterator protocol via
- * either implementing a `Symbol.asyncIterator` or `"@@asyncIterator"` method.
+ * implementing a `Symbol.asyncIterator` method.
  */
 export function isAsyncIterable(
   maybeAsyncIterable: unknown,

src/jsutils/isAsyncIterable.d.ts

@@ -1,6 +1,6 @@
 /**
  * Returns true if the provided object implements the AsyncIterator protocol via
- * either implementing a `Symbol.asyncIterator` or `"@@asyncIterator"` method.
+ * implementing a `Symbol.asyncIterator` method.
  */
 declare function isAsyncIterable(
   value: mixed,

src/jsutils/isAsyncIterable.js

@@ -8,12 +8,12 @@
  *
  * @example
  *
- * isIteratableObject([ 1, 2, 3 ]) // true
- * isIteratableObject(new Map()) // true
- * isIteratableObject('ABC') // false
- * isIteratableObject({ key: 'value' }) // false
- * isIteratableObject({ length: 1, 0: 'Alpha' }) // false
+ * isIterableObject([ 1, 2, 3 ]) // true
+ * isIterableObject(new Map()) // true
+ * isIterableObject('ABC') // false
+ * isIterableObject({ key: 'value' }) // false
+ * isIterableObject({ length: 1, 0: 'Alpha' }) // false
  */
-export function isIteratableObject(
-  maybeIteratable: unknown,
-): maybeIteratable is Iterable<unknown>;
+export function isIterableObject(
+  maybeIterable: unknown,
+): maybeIterable is Iterable<unknown>;

src/jsutils/isIteratableObject.d.ts renamed to src/jsutils/isIterableObject.d.ts

@@ -8,21 +8,21 @@
  *
  * @example
  *
- * isIteratableObject([ 1, 2, 3 ]) // true
- * isIteratableObject(new Map()) // true
- * isIteratableObject('ABC') // false
- * isIteratableObject({ key: 'value' }) // false
- * isIteratableObject({ length: 1, 0: 'Alpha' }) // false
+ * isIterableObject([ 1, 2, 3 ]) // true
+ * isIterableObject(new Map()) // true
+ * isIterableObject('ABC') // false
+ * isIterableObject({ key: 'value' }) // false
+ * isIterableObject({ length: 1, 0: 'Alpha' }) // false
  */
-declare function isIteratableObject(
+declare function isIterableObject(
   value: mixed,
   // $FlowFixMe[invalid-in-rhs]
 ): boolean %checks(value instanceof Iterable);
 
 // eslint-disable-next-line no-redeclare
-export function isIteratableObject(maybeIteratable: mixed): boolean {
+export function isIterableObject(maybeIterable: mixed): boolean {
   return (
-    typeof maybeIteratable === 'object' &&
-    typeof maybeIteratable?.[Symbol.iterator] === 'function'
+    typeof maybeIterable === 'object' &&
+    typeof maybeIterable?.[Symbol.iterator] === 'function'
   );
 }

src/jsutils/isIteratableObject.js renamed to src/jsutils/isIterableObject.js

@@ -22,10 +22,10 @@ describe('mapAsyncIterator', () => {
     });
   });
 
-  it('maps over async iterator', async () => {
+  it('maps over async iterable', async () => {
     const items = [1, 2, 3];
 
-    const iterator: $FlowFixMe = {
+    const iterable: $FlowFixMe = {
       [Symbol.asyncIterator]() {
         return this;
       },
@@ -39,7 +39,7 @@ describe('mapAsyncIterator', () => {
       },
     };
 
-    const doubles = mapAsyncIterator(iterator, (x) => x + x);
+    const doubles = mapAsyncIterator(iterable, (x) => x + x);
 
     expect(await doubles.next()).to.deep.equal({ value: 2, done: false });
     expect(await doubles.next()).to.deep.equal({ value: 4, done: false });
@@ -119,10 +119,10 @@ describe('mapAsyncIterator', () => {
     });
   });
 
-  it('allows returning early from mapped async iterator', async () => {
+  it('allows returning early from mapped async iterable', async () => {
     const items = [1, 2, 3];
 
-    const iterator: any = {
+    const iterable: any = {
       [Symbol.asyncIterator]() {
         return this;
       },
@@ -134,7 +134,7 @@ describe('mapAsyncIterator', () => {
       },
     };
 
-    const doubles = mapAsyncIterator(iterator, (x) => x + x);
+    const doubles = mapAsyncIterator(iterable, (x) => x + x);
 
     expect(await doubles.next()).to.deep.equal({ value: 2, done: false });
     expect(await doubles.next()).to.deep.equal({ value: 4, done: false });
@@ -182,10 +182,10 @@ describe('mapAsyncIterator', () => {
     });
   });
 
-  it('allows throwing errors through async iterators', async () => {
+  it('allows throwing errors through async iterable', async () => {
     const items = [1, 2, 3];
 
-    const iterator: any = {
+    const iterable: any = {
       [Symbol.asyncIterator]() {
         return this;
       },
@@ -197,7 +197,7 @@ describe('mapAsyncIterator', () => {
       },
     };
 
-    const doubles = mapAsyncIterator(iterator, (x) => x + x);
+    const doubles = mapAsyncIterator(iterable, (x) => x + x);
 
     expect(await doubles.next()).to.deep.equal({ value: 2, done: false });
     expect(await doubles.next()).to.deep.equal({ value: 4, done: false });

src/subscription/__tests__/mapAsyncIterator-test.js

@@ -1,7 +1,7 @@
 import { inspect } from '../jsutils/inspect';
 import { invariant } from '../jsutils/invariant';
 import { isObjectLike } from '../jsutils/isObjectLike';
-import { isIteratableObject } from '../jsutils/isIteratableObject';
+import { isIterableObject } from '../jsutils/isIterableObject';
 
 import type { ValueNode } from '../language/ast';
 import { Kind } from '../language/kinds';
@@ -60,10 +60,10 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode {
   // the value is not an array, convert the value using the list's item type.
   if (isListType(type)) {
     const itemType = type.ofType;
-    if (isIteratableObject(value)) {
+    if (isIterableObject(value)) {
       const valuesNodes = [];
       // Since we transpile for-of in loose mode it doesn't support iterators
-      // and it's required to first convert iteratable into array
+      // and it's required to first convert iterable into array
       for (const item of Array.from(value)) {
         const itemNode = astFromValue(item, itemType);
         if (itemNode != null) {