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

Incorrect typing for iterables in lib #29982

Open
dobesv opened this issue Feb 20, 2019 · 1 comment
Open

Incorrect typing for iterables in lib #29982

dobesv opened this issue Feb 20, 2019 · 1 comment
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@dobesv
Copy link

dobesv commented Feb 20, 2019

Search Terms:

iterable return type
asynciterable return type

Related Issues:

#2983 - describes how the iterables types are not inferred correctly for generators. Fixing this current issue would be a step towards making that issue resolvable as well.

The Problem

The type definitions for iterable and asynciterable do not match the specification.

According to the specification, when IteratorResult has done: true, the value is the return value of the iterator. This is not the same type as an element of the iterator. When implementing an iterator or async iterator "by hand", you get spurious type errors. For example:

const counter = (offset = 0, limit = 10) => {
  let x = 0;
  const a: AsyncIterator<number> = {
    next: async () => {
      if (x < 5) {
        x++;
        return { x, done: false };
      }
      return { done: true };
    },
  };
};

export default counter;

Gives errors:

src/testing/temp.ts:4:5 - error TS2322: Type '() => Promise<{ x: number; done: false; } | { done: true; x?: undefined; }>' is not assignable to type '(value?: any) => Promise<IteratorResult<number>>'.
  Type 'Promise<{ x: number; done: false; } | { done: true; x?: undefined; }>' is not assignable to type 'Promise<IteratorResult<number>>'.
    Type '{ x: number; done: false; } | { done: true; x?: undefined; }' is not assignable to type 'IteratorResult<number>'.
      Property 'value' is missing in type '{ x: number; done: false; }' but required in type 'IteratorResult<number>'.

4     next: async () => {
      ~~~~

Thus, the types for iterables and async iterables should look more like this:

interface IteratorValueResult<T> {
    done?: false;
    value: T;
}
interface IteratorReturnResult<T = undefined> {
    done: true;
    value: T;
}
type IteratorResult<T, U = undefined> = IteratorValueResult<T> | IteratorReturnResult<U>;

interface Iterator<T, U = undefined> {
    next(value?: any): IteratorResult<T, U>;
    return?(value?: any): IteratorResult<T, U>;
    throw?(e?: any): IteratorResult<T, U>;
}

interface Iterable<T, U = undefined> {
    [Symbol.iterator](): Iterator<T, U>;
}

interface IterableIterator<T, U = undefined> extends Iterator<T, U> {
    [Symbol.iterator](): IterableIterator<T, U>;
}

and

interface AsyncIterator<T, U = undefined, NextArg = undefined> {
    // the type of the argument to next (if any) depends on the specific async iterator
    next(value?: NextArg): Promise<IteratorResult<T, U>>;
    // return should give an iterator result with done: true` and the passed value (if any) as the value
    return?<R = undefined>(value: R): Promise<IteratorReturnResult<R>>;
    // throw should return a rejected promise with the given argument as the error, or return an iterator result with done: true
    throw?(e: any): Promise<IteratorReturnResult<undefined>>;
}

interface AsyncIterable<T, U = undefined, NextArg = undefined> {
    [Symbol.asyncIterator](): AsyncIterator<T, U, NextArg>;
}

interface AsyncIterableIterator<T, U = undefined, NextArg = undefined> extends AsyncIterator<T, U> {
    [Symbol.asyncIterator](): AsyncIterableIterator<T, U, NextArg>;
}

This should allow typescript to correctly infer that value is not required when done: true. For generators that do have a distinct return type from their yield type, they will be able to typecheck without spurious errors.

If the above definitions are acceptable to the maintainers I can prepare a PR to update the type definitions.

References

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Feb 27, 2019
@justjake
Copy link

justjake commented Apr 12, 2019

I hit some runtime errors after following Typescript's iterable types.

At runtime, a call to await iterable.next() will return { done: true, value: undefined }
for a complete async * function generator. However, the return type for async * function generator,
AsyncIterableIterator<T>, does not allow for value: undefined.

The return type of await iterable.next() should include | { done: true, value: undefined }.

Here's a demonstration of the issue:

https://github.com/justjake/ts-3.4.-async-iterable-issue

async function* generate() {
  yield 1
  yield 2
  yield 3
}

// AsyncIterableIterator<1 | 2 | 3>
type Out = ReturnType<typeof generate>

async function consume() {
  const iterator = generate()
  const yield1 = await iterator.next()
  const yield2 = await iterator.next()
  const yield3 = await iterator.next()
  const yield4 = await iterator.next()

  // IteratorResult<1 | 2 | 3>
  type ExpectedYield4Type = typeof yield4
  //  1 | 2 | 3
  type ExpectedYield4ValueType = typeof yield4.value

  console.log('yield4', yield4) // { value: undefined, done: true }
  console.log('yiled4.value', yield4.value) // undefined
  if (yield4.value === undefined) {
    throw new Error('yield4.value type is 1 | 2 | 3, so value cannot be undefined')
  }
}

consume()

Here's a playground link, although this is less helpful because playground does not have the AsyncIterableIterator type (see also https://github.com/Microsoft/TypeScript/issues/23030#issuecomment-482692170)

https://www.typescriptlang.org/play/#src=async%20function*%20generate()%20%7B%0D%0A%20%20yield%201%0D%0A%20%20yield%202%0D%0A%20%20yield%203%0D%0A%7D%0D%0A%0D%0A%2F%2F%20AsyncIterableIterator%3C1%20%7C%202%20%7C%203%3E%0D%0Atype%20Out%20%3D%20ReturnType%3Ctypeof%20generate%3E%0D%0A%0D%0Aasync%20function%20consume()%20%7B%0D%0A%20%20const%20iterator%20%3D%20generate()%0D%0A%20%20const%20yield1%20%3D%20await%20iterator.next()%0D%0A%20%20const%20yield2%20%3D%20await%20iterator.next()%0D%0A%20%20const%20yield3%20%3D%20await%20iterator.next()%0D%0A%20%20const%20yield4%20%3D%20await%20iterator.next()%0D%0A%0D%0A%20%20%2F%2F%20IteratorResult%3C1%20%7C%202%20%7C%203%3E%0D%0A%20%20type%20ExpectedYield4Type%20%3D%20typeof%20yield4%0D%0A%20%20%2F%2F%20%201%20%7C%202%20%7C%203%0D%0A%20%20type%20ExpectedYield4ValueType%20%3D%20typeof%20yield4.value%0D%0A%0D%0A%20%20console.log('yield4'%2C%20yield4)%20%2F%2F%20%7B%20value%3A%20undefined%2C%20done%3A%20true%20%7D%0D%0A%20%20console.log('yiled4.value'%2C%20yield4.value)%20%2F%2F%20undefined%0D%0A%20%20if%20(yield4.value%20%3D%3D%3D%20undefined)%20%7B%0D%0A%20%20%20%20throw%20new%20Error('yield4.value%20type%20is%201%20%7C%202%20%7C%203%2C%20so%20value%20cannot%20be%20undefined')%0D%0A%20%20%7D%0D%0A%7D%0D%0A%0D%0Aconsume()

@RyanCavanaugh @rbuckton

@RyanCavanaugh RyanCavanaugh added Needs Investigation This issue needs a team member to investigate its status. and removed Needs More Info The issue still hasn't been fully clarified labels Feb 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants