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

Add BuiltInIterableIterator for built-in iterables with undefined return values #56517

Conversation

lionel-rowe
Copy link
Contributor

Fixes #52998

Adds BuiltInIterableIterator, a stricter version of IterableIterator which is adhered to by all built-in iterables. This ensures correct types for next().value and for the return value of yield*:

declare const str: string
const iter = str[Symbol.iterator]()

// old type: any
// new type: string | undefined
const maybeChar = iter.next().value

function* generator() {
    // old type: any
    // new type: undefined
    const val = yield* [1, 2, 3]
}

// old type: Generator<number, any, undefined>
// new type: Generator<number, undefined, undefined>
function* generator2() {
    return yield* [1, 2, 3]
}

// old version: TS allows this
// new version: Property 'return' does not exist on type 'BuiltInIterableIterator<number>'.
iter.return?.()
// old version: TS allows this
// new version: Property 'throw' does not exist on type 'BuiltInIterableIterator<number>'.
iter.throw?.()

IterableIterator still remains, and should probably be preferred for usage as a parameter type to maintain backward compatibility and to allow lenience where the return value is discarded.

Currently out of scope for this PR: corresponding changes to DOM libs; any equivalent changes to async iterables.

@Jack-Works
Copy link
Contributor

Maybe the name IterableIteratorWithNoReturn is better? built-in does not definitely means it will return undefined, and user land iterators can also return undefined.

@lionel-rowe
Copy link
Contributor Author

Maybe the name IterableIteratorWithNoReturn is better? built-in does not definitely means it will return undefined, and user land iterators can also return undefined.

Yeah, it'd be worth considering alternatives for the name. Some other possibles, per #52998 (comment):

GenericIterableIterator (not to be confused with TS generics) ... StrictIterableIterator, ImplicitReturnIterableIterator, StandardIterableIterator

Personally I prefer the nomenclature "implicit return" over "no return", because the latter could also refer to a non-terminating function/generator (function* noReturn() { while (true) yield 1; })

However, bikeshedding about names may be premature — per #52998 (comment), this will likely remain blocked pending #36239, unless someone smarter than me can see a non-breaking way forward.

@rbuckton
Copy link
Member

To preserve backwards compatibility, in #58243 we've opted to introduce a strict-mode flag to control the expected return type for a built-in iterator. We've also adopted BuiltinIterator as part of introducing types to support the iterator helpers proposal, per #58222.

There is a slight conflation of terms here, however. #58222 introduces BuiltinIterator as a place to hang the iterator helpers methods so as not to collide with TypeScript's Iterator, which merely describes the minimal interface for iteration. However, all generators are also derived from BuiltinIterator, so we cannot specify a default return type lest we conflict with existing generator functions. In #58243 we introduced the BuiltinIteratorReturn type alias, which is an intrinsic type whose actual type is either undefined or any depending on whether the --strictBuiltinIteratorReturn compiler option is set. Since this is a strict-mode flag, anyone with strict: true in their configurations will get this new behavior automatically, but can manually opt out by setting strictBuiltinIteratorReturn: false. Anyone not compiling under strict mode will retain the existing behavior where the return type is any.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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