PSA: potential lib breaking change after iterator-helpers proposal

[Josh-Cena]

lib Update Request

Configuration Check

My compilation target is ESNext and my lib is the default.

Missing / Incorrect Definition

The TS lib defs for Iterator, Iterable, and IterableIterator treat them as if they are all fictitious interfaces (a.k.a. protocols). The lib def's inheritance structure looks like this:

interface Iterator<T, TReturn = any, TNext = undefined> {
    // NOTE: 'next' is defined using a tuple to ensure we report the correct assignability errors in all places.
    next(...args: [] | [TNext]): IteratorResult<T, TReturn>;
    return?(value?: TReturn): IteratorResult<T, TReturn>;
    throw?(e?: any): IteratorResult<T, TReturn>;
}

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

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

However, while Iterable and arguably IterableIterator are protocols, Iterator is an actual JS entity. See MDN docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator This has some practical issues, because what TS terms as IterableIterator is the actual JS Iterator class, but what TS terms as Iterator is what JS regards as "iterator-like" or "iterator protocol". For example, the def for new Set().values() says it returns an IterableIterator, while in fact it should return an instance of the JS Iterator instance.

This becomes more of an issue after the iterator-helpers proposal, which is going to expose Iterator as an actual global that people can use as extends. For example:

class MyClass {
  static #MyClassIterator = class extends Iterator {
    next() { ... }
  };

  [Symbol.iterator]() {
    return new MyClass.#MyClassIterator();
  }
}

Under the current lib def, #MyClassIterator won't be seen as iterable by TS, because it extends Iterator, not IterableIterator.

On the other hand, if we change the definition of Iterator to say it has [Symbol.iterator] (mirroring what happens in JS), then it will break code of the following kind:

function stepIterator(it: Iterator) {
  return it.next();
}

stepIterator({ next() {} });

Of course you could assume that almost all userland iterators are built-in and therefore inherit from Iterator and have @@iterator, but it's a breaking change nonetheless.

The question thus arises that when iterator-helpers lands, where should we put the new definitions on. Right now, I'm using core-js to polyfill these methods, and when writing declarations, I chose to put them on IterableIterator, so that new Set().values() works:

declare global {
  interface IterableIterator<T> {
    filter(
      predicate: (value: T, index: number) => unknown,
    ): IterableIterator<T>;
    map<U>(mapper: (value: T, index: number) => U): IterableIterator<U>;
    toArray(): T[];
  }
}

...but this is suboptimal, for obvious reasons (it means the Iterator class doesn't actually have these methods).

Sample Code

Documentation Link

[rbuckton]

IterableIterator is possibly the best place for them, but in the event that is too much of a break we may introduce a NativeIterator<T> or BuiltinIterator<T> interface to avoid breaks.

The biggest issue I see is that we can't properly model the type of the native Iterator constructor without using class, because the native Iterator class from the iterator helpers proposal is essentially abstract because you must still define next:

declare abstract class Iterator<T> {
  constructor();
  abstract next(value?: undefined): IteratorResult<T, void>;
  // ... iterator helper instance methods ...
  static from<T>(obj: Iterable<T> | TheCurrentIteratorType<T>): Iterator<T>;
  // ... iterator helper static methods ...
}

While we can model the abstract constructor using an abstract new constructor type, the abstract new syntax isn't supported on construct signatures in an interface. We also have no way to indicate an abstract method in an interface at the moment. If we did, a non-class version might look like this:

interface IterableIterator<T> extends Iterator<T> {
    abstract next(value?: undefined): IteratorResult<T, void>;
 // ^^^^^^^^ this doesn't matter for implementing interfaces. it is
 //          only applied when tied to an `abstract new` signature.

    [Symbol.iterator](): IterableIterator<T>;

    // ... iterator helper instance methods ...
}

interface IteratorConstructor {
    abstract new<T>(): IterableIterator<T>;
 // ^^^^^^^^ opt-in to caring about the `abstract`-ness 
 //          of the interface.

    readonly prototype: IterableIterator<any>;

    from(obj: Iterable<T> | Iterator<T>): IterableIterator<T>;
    // ... iterator helper static methods ...
}

declare var Iterator: IteratorConstructor;

[ljharb]

I’m confused; Iterator.prototype.next exists, why would you need to define it yourself?

[Josh-Cena]

Iterator.prototype.next exists, why would you need to define it yourself?

...Does it? Conceptually, all iterator objects function differently exactly because they have different next(). I don't think the proposal adds it either:

[ljharb]

ahhh, you're right, in this case it's IteratorHelperPrototype.next that the proposal adds.

[TheCymaera]

It could be renamed to IteratorLike, similar to the existing PromiseLike for promises.

[felixfbecker]

Now that iterator helpers are shipped in Chrome and Edge, meaning it will also ship in the next NodeJS release this month (and there are polyfills available too for other engines that currently have it behind experimental flags) it would be really nice if TypeScript could tackle typing the iterator helpers. Given the current naming conflict, it is difficult to write custom typings to fill in the gap in userland (and I haven't found any successful attempts in DT or polyfill packages).

[bakkot]

A concrete proposal to maybe get the ball rolling:

• As soon as possible, rename the existing Iterator interface to IteratorLike (or some other better name). Change all internal uses of Iterator to refer to IteratorLike instead, but no need to rename IterableIterator or anything else.
• Add abstract class Iterator { ... } modeling the actual Iterator class in the language.

This is a breaking change for anyone using the Iterator type explicitly, but hopefully it's not that many - there's not much reason to explicitly use the Iterator type right now. There are only 176 files in DefinitelyTyped which use Iterator< right now, and manual inspection reveals that more than half of those aren't referring to the TS Iterator type (e.g. x, x, x).

The main way that this would break is that places which declared to need an Iterator<T> would start giving an error if provided with { next() {...} }, which previously worked. There do not appear to be that many cases of this. Also, types in dependencies which declare themselves to provide Iterator<T> and give { next() {...} } would appear to have .map etc, when in fact that is missing - this gives you the wrong typing, but not in a way which causes errors for existing code.

[rbuckton]

The main way that this would break is that places which declared to need an Iterator<T> would start giving an error if provided with { next() {...} }, which previously worked. There do not appear to be that many cases of this. Also, types in dependencies which declare themselves to provide Iterator<T> and give { next() {...} } would appear to have .map etc, when in fact that is missing - this gives you the wrong typing, but not in a way which causes errors for existing code.

The main way this would break is that any existing package that serves an Iterator<T> for any reason would now appear to have the methods of the native Iterator class when they may not, in fact, actually implement those. I still think the IterableIterator<T> type is the correct place to vend these methods from. Despite the difference in naming between the Iterator class and the IterableIterator interface, we can still describe a constructor type that works:

interface IteratorConstructor {
 // ...static methods
}
declare var Iterator: (abstract new <T>() => IterableIterator<T>) & IteratorConstructor;

[bakkot]

The main way this would break is that any existing package that serves an Iterator<T> for any reason would now appear to have the methods of the native Iterator class when they may not, in fact, actually implement those.

That's what I was saying in the second half of the bit you quoted, yeah. But that wouldn't actually cause any existing code to fail to compile (except in quite obscure cases which depend on non-existence of map etc at the type level), so it's only debatably "breaking". And from a quick survey of DT that happens quite rarely anyway; not many people are manually implementing iterators.

I still think the IterableIterator<T> type is the correct place to vend these methods from.

IterableIterator is an interface which can be implemented by existing things which don't provide those methods. Merely having next and Symbol.iterator methods suffices to implement that interface, but doesn't give you map etc. Are you proposing to change the definition of IterableIterator so that implementing it requires implementing those methods? If you're going to make a breaking change anyway, I'd think it would be better to stick closer to the actual types.

---

Another option would be to stick with class for the declaration, but call it something other than Iterator so that name doesn't end up in the type namespace (and ideally mark that name as non-exported, but I don't know if that's possible), then add a new NativeIterator interface for the type-level name. Something like

declare abstract class __TheActualIteratorBuiltin<T> implements IterableIterator<T> {
  abstract next(value?: undefined): IteratorResult<T, void>;
  [Symbol.iterator]: () => NativeIterator<T>;
  
  map<R>(fn: (v: T) => R): NativeIterator<T>;
  // etc
}
declare var Iterator: typeof __TheActualIteratorBuiltin;
type NativeIterator<T> = __TheActualIteratorBuiltin<T>;

---

Incidentally, with any of these approaches, how do you update the type for Generator so that it properly inherits these new methods, without just copying them all over? Right now generators implicitly implement IterableIterator but they don't actually get any methods from anywhere.

[rbuckton]

Another option would be to stick with class for the declaration, [...]

class always introduces a value-space symbol in the binder, so it would always appear as if __TheActual... (or whatever we called it) is an actual value. We generally avoid that by using var declarations. The abstract new <T>(): IterableIterator<T> signature intersected with the IteratorConstructor interface achieves the same thing without introducing a non-existing value. Albeit without the ability to define an abstract method. We unfortunately don't have a way to define those outside of class currently.

I'm also fine with calling this NativeIterator<T> and updating the built-ins to use this interface instead.

Incidentally, with any of these approaches, how do you update the type for Generator so that it properly inherits these new methods, without just copying them all over?

We would just do interface Generator extends NativeIterator.

[rbuckton]

Are you proposing to change the definition of IterableIterator so that implementing it requires implementing those methods?

We've generally discouraged implementing them directly in the past to try to set the expectation that IterableIterator is the thing built-ins return. I'm not sure how many explicit implementations of IterableIterator exist today, though.

[bakkot]

Albeit without the ability to define an abstract method. We unfortunately don't have a way to define those outside of class currently.

Yeah, that's what I was going for with the class thing. But if you think the cost of next not being abstract is worth the benefit of avoiding a new name appearing to be an actual global, that's reasonable. Shame there isn't a way to introduce the type-level class only for use in this file without polluting the global namespace.

I'm also fine with calling this NativeIterator<T> and updating the built-ins to use this interface instead. [...] I'm not sure how many explicit implementations of IterableIterator exist today, though.

There's 77 files on DT which match \bIterableIterator< right now, so, not many. But I'd personally still be happier with IterableIterator meaning just Iterable + Iterator. I'll get a PR starting with the NativeIterator approach for now; if you/the TS team decide that methods on IterableIterator would be better we can always switch over to that.