Skip to content

Iterator Helpers Typing #59908

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

Closed
nikolaybotev opened this issue Sep 9, 2024 · 7 comments
Closed

Iterator Helpers Typing #59908

nikolaybotev opened this issue Sep 9, 2024 · 7 comments
Assignees

Comments

@nikolaybotev
Copy link

nikolaybotev commented Sep 9, 2024

⚙ Compilation target

ESNext

⚙ Library

lib.esnext.d.ts

Missing / Incorrect Definition

global Iterator class

Sample Code

Iterator.from([1, 2, 3, 4, 5])
  .map(n => n + 1)
  .filter(n => n > 2)
  .drop(1)
  .take(2)
  .toArray() // [4, 5]

See also all the validation tests (working since Node.js 22.0.0) at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Iterator#iterator_helpers

However, the following code should (ideally) continue working:

const customIterator: Iterator<number> = { next: () => ({ value: 42 }) };

Documentation Link

@nikolaybotev
Copy link
Author

nikolaybotev commented Sep 9, 2024

How are you going to handle the new Iterator class introduced in the Iterator helpers proposal?

The methods of this class -- if introduced as a TypeScript class declaration, will merge with the pre-existing Iterator interface (that describes the Iterator protocol) and break all code that uses the Iterator type to describe custom iterators [by introducing the requirement for the presence of Iterator helper methods on all objects of type Iterator].

This could be an uncommon occurrence, and it might be acceptable to rename the Iterator interface to IteratorProtocol [or something along these lines].

Have you given this any consideration?

@nikolaybotev
Copy link
Author

See a clear demonstration of the problem here: https://github.com/nikolaybotev/DefinitelyTyped/pull/1/files

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 9, 2024
@Josh-Cena
Copy link
Contributor

Isn't this just a rehash of #54481, in which it was decided that Iterator would refer to different things in type space and value space (that is, Iterator() does not return you an object of type Iterator, but of type BuiltinIterator?)

@nikolaybotev
Copy link
Author

nikolaybotev commented Sep 10, 2024

The downside with the approach taken in #58222 is that by maintaining backwards compatibility, new typing information is omitted / incorrect and can result in both working code failing type validation and failing code passing type validation.

  1. instanceof Iterator will not expose the Iterator Helpers to users:

Have you considered this?

let it: object;

if (it instanceof Iterator) {
  it.toArray(); // this works, but is not accepted by TypeScript!
}
  1. The absence of the next etc methods of iterator protocol from the Iterator class (which should ideally be defined as abstract in typescript which is not possible due to the Iterator interface from es2015 holding up that name):
class IncompleteIterator extends Iterator { }

const it = new IncompleteIterator();

it.next(); // Uncaught TypeError: it.next is not a function, but accepted by TypeScript

Was it a conscious decision to pay this price to maintain backwards compatibility?

Is this going to be mentioned in the release notes for TS6, which is where I assume this lands?

@nikolaybotev
Copy link
Author

Nevermind, I see that both of the above examples are handled correctly with the new type definitions...

@nikolaybotev
Copy link
Author

Looks like there is some serious TypeScript magic involved to make this work :-) glad it does.

@nikolaybotev
Copy link
Author

Thank you @Josh-Cena for the prompt response here.

I was able to catch up on the work done to type Iterator Helpers and understand the solution to the problem of the divergence between the Iterator type and the Iterator value and how TypeScript's modules /namespaces, augmentation and class+interface merging allow the abstract nature of the javascript Iterator class to be properly expressed in a backwards-compatible manner.

I also happily observed that the generic type signatures of Iterator and Iterable have been improved by adding the TReturn and TNext type arguments to the Iterable interface for example. This is a very welcome change!

Now, TNext, and TReturn relate to some very obscure and not commonly used capabilities, but as supported features of the JavaScript runtime, I believe it is important that they are used in a manner that exposes as much as that niche JavaScript iterator/gnerator functionality to the typescript world.

To that end, I have opened #59926 and done some background work on empirically determining Iterator Helpers behavior, and opened the first PR #59927 that updates the Iterator.from generic type signature, to support those use cases with TNext and TReturn.

@rbuckton rbuckton removed the Needs Investigation This issue needs a team member to investigate its status. label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants