repeaterjs / repeater

The missing constructor for creating safe async iterators
https://repeater.js.org
MIT License
461 stars 12 forks source link

Doesn't quite match AsyncIterable interface? #56

Open jedwards1211 opened 4 years ago

jedwards1211 commented 4 years ago

I don't know if this causes problems in TS, but I noticed it because I'm trying to convert the defs to Flow, so I wanted to point it out.

The AsyncIterator interface is

// es2018.asyncgenerator.d.ts
interface AsyncGenerator<T = unknown, TReturn = any, TNext = unknown> 
    extends AsyncIterator<T, TReturn, TNext> {
    // NOTE: 'next' is defined using a tuple to ensure we report the correct assignability errors
    // in all places.
    next(...args: [] | [TNext]): Promise<IteratorResult<T, TReturn>>;
    return(value: TReturn): Promise<IteratorResult<T, TReturn>>;
    throw(e: any): Promise<IteratorResult<T, TReturn>>;
    [Symbol.asyncIterator](): AsyncGenerator<T, TReturn, TNext>;
}

next and return don't accept PromiseLike the way they do in Repeater.

brainkim commented 4 years ago

Is that the typescript definition? I’m pretty sure return accepts PromiseLike parameters: https://github.com/microsoft/TypeScript/blob/master/lib/lib.es2018.asynciterable.d.ts#L32-L37 If you call return with a promise-like it will await the argument. It’s a weird thing that never happens but whatever.

As for next, Repeater.prototype.next is assignable to AsyncGenerator.prototype.next because the latter’s parameters are assignable to the former’s parameters (contravariance). If you think about it (value: number | Promise<number>) => any is assignable to (value: number) => any, while (value: number) => any is not assignable to (value: number | Promise<number>) => any. Same with Repeater.prototype.next. Again, I’m not really sure there’s a use-case here, it was more a consequence of the fact that the value returned from the push function resolves to the parameter passed to next, so we might as well allow it to be a PromiseLike.

If you need help with flow stuff or any other questions let me know! Brian

jedwards1211 commented 4 years ago

ah okay, they've updated the interface from whatever I found, which I think was one of the first commits of it

jedwards1211 commented 4 years ago

FWIW the spec says this behavior isn't enforced...

If the argument value is used in the typical manner, then if it is a rejected promise, a promise rejected with the same reason should be returned; if it is a fulfilled promise, then its fulfillment value should be used as the value property of the returned promise's IteratorResult object fulfillment value. However, these requirements are also not enforced.

But I guess the TS declarations enforce this on anything implementing the TS AsyncIterator interface.

The flow defs are so messed up sigh