Built-in Symbol.iterator methods return IterableIterator<T> extending Iterator<T>, not Iterator<T, undefined>

[lionel-rowe]

lib Update Request

The built-in Symbol.iterator methods return IterableIterator<T> extending Iterator<T>, not the expected Iterator<T, undefined>. For example:

const char = string[Symbol.iterator]().next().value

The expected type of char is string | undefined, but instead it's any. This isn't correct — it will never be anything other than a string (strictly a single Unicode char) or undefined at runtime.

This looks to be related to #33353, which was closed due to backwards-compatibility concerns; however, it could presumably fixed without changing the default types of IterableIterator, instead changing its definition to take an extra type parameter:

- interface IterableIterator<T> extends Iterator<T> {
-     [Symbol.iterator](): IterableIterator<T>;
+ interface IterableIterator<T, TReturn = any> extends Iterator<T, TReturn> {
+     [Symbol.iterator](): IterableIterator<T, TReturn>;
  }

And then changing all the built-ins (String, Array, etc) like so:

  interface String {
      /** Iterator */
-     [Symbol.iterator](): IterableIterator<string>;
+     [Symbol.iterator](): IterableIterator<string, undefined>;
  }

Could that work as a fix and be sufficiently backward-compatible?

Configuration Check

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

Missing / Incorrect Definition

Various — any built-in JS type with a Symbol.iterator method, such as Array, String, Map, Set, Uint8Array, etc.

Sample Code

const char = string[Symbol.iterator]().next().value
// expected type: string | undefined
// actual type: any

const definitelyChar = nonEmptyString[Symbol.iterator]().next().value!
// expected type: string
// actual type: any

Documentation Link

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/@@iterator

[fatcerberus]

Somewhat of a tangent but void is not a synonym for undefined and shouldn’t be treated as such. See #42709.

[lionel-rowe]

Somewhat of a tangent but void is not a synonym for undefined and shouldn’t be treated as such. See #42709.

Thanks for the correction! I've fixed my original comment to be more accurate about this.

[fatcerberus]

The expected type of char is string | void

Again, I don’t think you really want this. You can’t narrow void out of a union. If your argument is that the value can be undefined then you should use undefined explicitly.

[lionel-rowe]

If your argument is that the value can be undefined then you should use undefined explicitly.

Semantically, string | void makes more sense to me than string | undefined, by analogy with generator functions that don't explicitly return something:

// type of iterateStr is Generator<string, void, undefined>, i.e. TReturn is void
function* iterateStr(str: string) {
    yield* str
}

You can’t narrow void out of a union.

Seems to work fine to me? Playground

declare let x: string | void 

if (typeof x === 'string') {
    x // string
}

let y = x! // string
let z: Exclude<typeof x, void> // string

I'm not married to using void over undefined if the latter will prevent some class of bugs, though.

[fatcerberus]

The problem with string | void, is that void is an implicit promise that the code receiving a value of that type won't access it at all. More critically, () => T is assignable to () => void for all T, which means the value is not even guaranteed to be undefined. string | void is thus conceptually unsound because you have to access the value to narrow it.

More concretely, here's the issue with void:

const foo: () => void = () => 777;
const bar: () => number = () => 1206;
const x: number | void = Math.random() > 0.5 ? foo() : bar();
// note: by calling foo which has a void return type, we've promised to discard its return value.
// but now there's a problem: we have no way to know whether x is garbage or not!

In other words, in the context of larger union type it's basically just an any with extra steps.

[lionel-rowe]

More concretely, here's the issue with void:

Wow, void is a lot weirder than I thought... I've reworded the OP to use undefined exclusively.

Is the fact that union types like string | void can be checked for truthiness considered a bug, then? It certainly seems like one:

const fn: () => void = () => 1
let x: string | void = 2 > 1 ? fn() : 'str'

if (x) {
    x.trim() // passes type checking but throws runtime error
}

If this is considered a bug, is there a live issue for it where we can split of this discussion?

[fatcerberus]

Yeah, void is an interesting animal. Truthiness checks can sometimes narrow it away but they really shouldn’t. All the pitfalls are discussed in the issue I linked above: #42709

[MuTsunTsai]

The same issue happens on Set.prototype.values() as well:

const set = new Set<string>();
const v = set.values().next().value;
// should be string | undefined, but get any

Playground

[lionel-rowe]

Draft PR up here: #56502

Having thought some more about it though, maybe it'd be better to have an entirely new interface that doesn't extend Iterator. In addition to defaulting TReturn to any, Iterator also has two methods that are never present on the built-in iterable iterators:

    return?(value?: TReturn): IteratorResult<T, TReturn>;
    throw?(e?: any): IteratorResult<T, TReturn>;

That widens the discrepancy between compile-time type checking and runtime type guarantees even further:

const arr = [1]
const iter = arr.values()

const next = iter.next()
if (!next.done) next.value  // ✅ type = number
next.value // ❌ type = any (should be number | undefined)

iter.nonExistentMethod?.() // ✅ Property 'nonExistentMethod' does not exist on type 'IterableIterator<number>'.
iter.return?.() // ❌ TS allows this, even though the `return` method will never exist (absent monkey-patching etc)
iter.throw?.() // ❌ TS allows this, even though the `throw` method will never exist (absent monkey-patching etc)

Instead, we could have something like this:

interface Iterator<T, TReturn = any, TNext = undefined> {
    next(...args: [] | [TNext]): IteratorResult<T, TReturn>;
    return?(value?: TReturn): IteratorResult<T, TReturn>;
    throw?(e?: any): IteratorResult<T, TReturn>;
}

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

+ interface BuiltInIterableIterator<T> {
+     next(): IteratorResult<T, undefined>;
+     [Symbol.iterator](): BuiltInIterableIterator<T>;
+ }

interface Array<T> {
-     [Symbol.iterator](): IterableIterator<T>;
+     [Symbol.iterator](): BuiltInIterableIterator<T>;
-     entries(): IterableIterator<[number, T]>;
+     entries(): BuiltInIterableIterator<[number, T]>;
-     keys(): IterableIterator<number>;
+     keys(): BuiltInIterableIterator<number>;
-     values(): IterableIterator<T>;
+     values(): BuiltInIterableIterator<T>;
}

Not sure if BuiltInIterableIterator is the best name though — maybe GenericIterableIterator (not to be confused with TS generics) or something else? Some other options: StrictIterableIterator, ImplicitReturnIterableIterator, StandardIterableIterator

[fatcerberus]

Not sure I agree on the second point re: removing the nonexistent methods simply because they never exist. Code may want to deal with iterators generically and not have to care whether it’s a built-in iterator or not. iter.return?.() should still be allowed, IMO.

Future versions of ECMAScript may also decide to add those methods, too, so people might be writing iter.return?.() today, just for the sake of future-proofing.

[lionel-rowe]

Not sure I agree on the second point re: removing the nonexistent methods simply because they never exist. Code may want to deal with iterators generically and not have to care whether it’s a built-in iterator or not. iter.return?.() should still be allowed, IMO.

I'm not suggesting function parameter types in library code should be changed, only the types of the built-in iterables themselves. Userland code, on the other hand, can be as strict or lenient as it likes.

interface Iterator<T, TReturn = any, TNext = undefined> {
    next(...args: [] | [TNext]): IteratorResult<T, TReturn>;
    return?(value?: TReturn): IteratorResult<T, TReturn>;
    throw?(e?: any): IteratorResult<T, TReturn>;
}

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

interface BuiltInIterableIterator<T> {
    next(): IteratorResult<T, undefined>;
    [Symbol.iterator](): BuiltInIterableIterator<T>;
}

// an existing library function that's always accepted `IterableIterator<number>`; signature remains unchanged
function someLibraryFn(iter: IterableIterator<number>) {
    iter.throw?.()
    iter.return?.()

    return iter.next().value
}

declare const iter: BuiltInIterableIterator<number>

// works with no issues
// `BuiltInIterableIterator<number>` is still assignable to IterableIterator<number>
// type of `val` remains `any`
const val = someLibraryFn(iter)

Anyhow, my PR #56517 is failing CI due to Type 'void' is not assignable to type 'undefined'. Per the discussion earlier in this issue thread, I don't think typing next's return type as IteratorResult<T, void>; is an appropriate solution; in which case, it would seem like this fix is blocked pending #36239.

[lionel-rowe]

As this may not get fixed any time soon, I'll leave this here too: my main use case for this is wanting an easy way to grab the first element from an iterable (often the first UTF-8 char of a string). The most ergonomic way is [...str][0], but iterating the whole string just to get the first item seems pretty wasteful. str[Symbol.iterator]().next().value is only a shade more verbose, but loses all type information in the process due to this issue.

What I've settled on since I opened this issue is the following:

let char = ''
for (const ch of str) {
    char = ch
    break
}

It looks a little funky, but it's reasonably concise, performant (doesn't iterate the whole string), correctly typed, and also has the advantage of forcing you to choose a default (in this case '') in case the string is empty.

An alternative using an IIFE, which is even funkier, but has the advantage of allowing you to declare the variable with const and/or allowing a different type for the default value without explicitly declaring it:

const char = (() => {
    for (const ch of str) return ch
    return ''
})()