-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Improve Iterator Helper Type Signatures #59926
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
Comments
Not all built-in iterators. Your two specific examples in fact do not: e.g. But many built-in iterators do have a
I haven't actually tested it, but I was originally thinking of just having the return types of such methods be an intersection type, along the lines of the following (probably with some tweaks for the various type parameters): type ClosableIteratorObject<T> = IteratorObject<T> & { return(): { done: true, value: undefined } };
interface IteratorObject<T, TReturn, TNext> {
map<U>(callbackfn: (value: T, index: number) => U): ClosableIteratorObject<U>;
} (instead of having |
Thanks for the clarification. and thanks for the prompt response. I also realized that the following statement is wrong:
I have updated the issue description to reflect that. I do believe there is value in adding the I was able to confirm that all major implementation - Node.js (and by extension Chrome, Edge etc browsers sharing the same engine, which I have not tested), Firefox, Opera and core-js all exhibit the behavior of passing through the wrapped iterator return value: core-js
node.js 22
Firefox
Opera
|
The PR that implements the change proposed here is ready for review - #59927 |
Just confirmed that the behavior of adding a return method to the Iterator object is consistent across Firefox, Opera, Node, and core-js:
Do you want me to open a PR adding the |
Iterator.from({ next: () => ({ value: 42 }) }).return is present, but Iterator.from({ __proto__: Iterator.prototype, next: () => ({ value: 42 }) }).return is not. I wouldn't worry about trying to reflect this behavior in the TypeScript types. It's OK for them to be a little imprecise.
I'm not on the TS team. I personally am unlikely to open such a PR in the near future, but if you want to see if the TS team would accept such a PR you are welcome to do so. |
Amazing. Thanks for the clarification.
👍
Given the above behavior you brought up, which I was unaware of, I am not convinced it is any good to try and reflect the addition of the return method when wrapping an iterator in the type signature. |
Since IteratorObject instances (objects whose prototype is Iterator.prototype) are not wrapped by Iterator.from, we could add an Iterator.from overload to represent those cases and pass through all generic type parameters: from<T, TReturn = any, TNext = any>(value: IteratorObject<T, TReturn, TNext>): IteratorObject<T, TReturn, TNext>; This would make the following two examples compile: declare const g1: Generator<string, number, boolean>;
const iter1 = Iterator.from(g1); and class A extends Iterator<string, void, string> { next = (value: string): IteratorResult<string, void> => ({ value }); [Symbol.iterator] = () => this }
const g3 = new A();
const iter5 = Iterator.from(g3); while still failing when a custom iterator would get wrapped by const g2 = { next: (value: string) => ({ value }) };
const iter4 = Iterator.from(g2); |
@bakkot do you have plans or are you aware of anyone else's plans to add async iterator helper and iterator helper polyfill type definitions to If not, I would like to make an effort to write these type definitions. |
I don't know anything about how types for core-js work; if you do and you want to contribute, go for it. |
Thanks. There are types in DefinitelyTyped where one could add type definitions for AsyncIterator helpers, which core-js implements today. |
⚙ Compilation target
ESNext
⚙ Library
esnext.iterator.d.ts
Missing / Incorrect Definition
Iterator.from(value: Iterator)
- PR Improve generic type signature of Iterator.from() #59927this is a bad idea; my mistakevalue
iterator argument withTNext
other thanundefined
should be accepted;IteratorObject
instances (objects whose prototype isIterator.prototype
) are not wrapped byIterator.from
, we could add anIterator.from
overload to represent those cases and pass through all generic type parameters:from<T, TReturn = any, TNext = any>(value: IteratorObject<T, TReturn, TNext>): IteratorObject<T, TReturn, TNext>;
TReturn
type should be passed through from the argument to the return typefilter
,map
etc) do not propagate the return value of their source iterator and should haveTReturn
set toundefined
Iterator objects returned fromthanks to @bakkot for the clarificationIterator.from
and the other built-in iterators returned fromArray.values
etc always have areturn
method (built-in iterators returned fromArray.values
etc also always have athrow
method)IteratorObject
without also forcing user-defined classes extending from the javascriptIterator
class to also implement thereturn
method, which would be wrong. The impact on client code is minimal as the Iterator object'sreturn
method can always be called safely using thereturn!()
notation.Sample Code
https://github.com/nikolaybotev/iteratorhelpersdemo
Documentation Link
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator
and
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols
The text was updated successfully, but these errors were encountered: