Iterable and AsyncIterable interfaces should have full type arguments

[falsandtru]

Adding optional type arguments wouldn't affect compatibility. @rbuckton Right?

TypeScript Version: 3.4.0-dev.201xxxxx

Search Terms:

Code

// A *self-contained* demonstration of the problem follows...
// Test this by running `tsc` on the command-line, rather than through another build tool such as Gulp, Webpack, etc.

Expected behavior:

interface Iterable<T, TReturn = any, TNext = undefined> {
    [Symbol.iterator](): Iterator<T, TReturn, TNext>;
}
interface AsyncIterable<T, TReturn = any, TNext = undefined> {
    [Symbol.asyncIterator](): AsyncIterator<T, TReturn, TNext>;
}

Actual behavior:

interface Iterable<T> {
    [Symbol.iterator](): Iterator<T>;
}
interface AsyncIterable<T> {
    [Symbol.asyncIterator](): AsyncIterator<T>;
}

Playground Link:

Related Issues:

[IllusionMH]

(Removed totally wrong assumption based on version from template. Sorry)

[rbuckton]

I investigated this in #30790 (comment) and determined that supplying defaults resulted in additional complexity and some compatibility issues. We intend to leave Iterable<T> and AsyncIterable<T> as is and direct users to Generator<T, TReturn, TNext> if they intend to leverage the TReturn and TNext types.

[falsandtru]

I'd read it but couldn't read that Iterable has the same issues as IterableIterator. Does Iterable really have the same issues? And replacing Iterable with Generator may be good with return types but would be bad with argument types. Generator makes unnecessary constraints for them. These constraints make type errors with function calls. Also with return types Iterable makes good conservative interfaces. It is a good thing that tolerant for input and conservative for output.

[brainkim]

According to the note from the ecmascript spec 25.1.1.2, all well-formed iterators should be prepared for next (and possibly return) to be called without arguments.

NOTE 1
Arguments may be passed to the next function but their interpretation and validity is dependent upon the target Iterator. The for-of statement and other common users of Iterators do not pass any arguments, so Iterator objects that expect to be used in such a manner must be prepared to deal with being called with no arguments.

[falsandtru]

That rule applies to Iterator interface (firstly). So it is not an issue of Iterable interface.

[brainkim]

Yeah I was trying to collect my thoughts earlier when I posted that but I got distracted by baseball ⚾️ ⚾️ ⚾️ . I think the note is relevant because @rbuckton states in the linked comment:

These changes are intended to further improve the strictness of Generators (and Async Generators) without sacrificing backwards compatibility for existing libraries.

It seems like what he’s trying to make strict is the possibility that TReturn/TNext do not extend undefined, but there are simply too many ways to invoke/run an iterator which will cause TReturn and TNext to be undefined in almost every single position where the types are used. Like, is the logical conclusion of a generator with a non-undefined TReturn or TNext type that the generator itself is not iterable? This feels like a violation of tc39’s definition of Iterables, and it seems like a reductio ad absurdum argument against having strongly typed generators altogether. I feel like we should be starting from the premise that generators are iterable, no matter what the type arguments are.

I dunno if any of this makes sense. I’ve been trying to upgrade some async iterator stuff to 3.6.2 and I’m having issues with assignability everywhere.

I think a good middle ground is that TReturn and TNext should be forced to extend undefined, at least for IterableIterator. So the definition might look something like:

type IterableIterator<T, TReturn extends undefined = undefined, TNext extends undefined = undefined> = Iterable<T> & Iterator<T, TReturn, TNext>;

I want to get a better sense of the compatibility issues caused by IterableIterator being passed type arguments but it doesn’t seem like there are any examples of actual code which caused issues.

[rbuckton]

@brainkim if TReturn or TNext are forced to extend undefined, then they can only ever have the types undefined, any, or never, as those are the only types that could be considered subtypes of undefined (since any is both a supertype and subtype of everything).

[rbuckton]

I've generally been against adding type arguments to Iterable (and AsyncIterable) due to the increased output size for declaration emit, since we would expand every Iterable<number> to Iterable<number, void, undefined>. I plan to investigate whether we can simplify the emit for inferred types whose type arguments are the same type as the default type, but that likely won't happen until after 4.1.

[brainkim]

if TReturn or TNext are forced to extend undefined, then they can only ever have the types undefined, any, or never, as those are the only types that could be considered subtypes of undefined (since any is both a supertype and subtype of everything)

I guess what I wanted is for TNext and TReturn to be a union (TNext | undefined) but the correct way to implement this is by adding a union with undefined wherever the type parameters appear in the interface/class.

These days, I think forcing people to reason about the potential for TReturn/TNext types being undefined is fine. You can just use the non-null assertion operator.

[RyanCavanaugh]

I haven't seen any important downstream impacts of not having these parameters so let's call this good enough for now until there's more concrete feedback to drive potential changes here.