Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use correct terminology around iterators and iterable #3041

Merged
merged 1 commit into from Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/execution/execute.js
Expand Up @@ -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';
Expand Down Expand Up @@ -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}".`,
);
Expand Down
17 changes: 10 additions & 7 deletions src/jsutils/__tests__/isAsyncIterable-test.js
Expand Up @@ -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() {
Expand All @@ -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);
});

Expand All @@ -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);
});
});
71 changes: 71 additions & 0 deletions 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);
});
});
71 changes: 0 additions & 71 deletions src/jsutils/__tests__/isIteratableObject-test.js

This file was deleted.

2 changes: 1 addition & 1 deletion 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.
*/
export function isAsyncIterable(
maybeAsyncIterable: unknown,
Expand Down
2 changes: 1 addition & 1 deletion src/jsutils/isAsyncIterable.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.
*/
declare function isAsyncIterable(
value: mixed,
Expand Down
Expand Up @@ -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>;
Expand Up @@ -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'
);
}
18 changes: 9 additions & 9 deletions src/subscription/__tests__/mapAsyncIterator-test.js
Expand Up @@ -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;
},
Expand All @@ -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 });
Expand Down Expand Up @@ -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;
},
Expand All @@ -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 });
Expand Down Expand Up @@ -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;
},
Expand All @@ -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 });
Expand Down
6 changes: 3 additions & 3 deletions src/utilities/astFromValue.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';
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/utilities/coerceInputValue.js
Expand Up @@ -6,7 +6,7 @@ import { isObjectLike } from '../jsutils/isObjectLike';
import { suggestionList } from '../jsutils/suggestionList';
import { printPathArray } from '../jsutils/printPathArray';
import { addPath, pathToArray } from '../jsutils/Path';
import { isIteratableObject } from '../jsutils/isIteratableObject';
import { isIterableObject } from '../jsutils/isIterableObject';

import { GraphQLError } from '../error/GraphQLError';

Expand Down Expand Up @@ -75,7 +75,7 @@ function coerceInputValueImpl(

if (isListType(type)) {
const itemType = type.ofType;
if (isIteratableObject(inputValue)) {
if (isIterableObject(inputValue)) {
return Array.from(inputValue, (itemValue, index) => {
const itemPath = addPath(path, index, undefined);
return coerceInputValueImpl(itemValue, itemType, onError, itemPath);
Expand Down