Skip to content

IteratorResult should be a discriminated union #28670

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
5 tasks done
KSXGitHub opened this issue Nov 26, 2018 · 10 comments
Closed
5 tasks done

IteratorResult should be a discriminated union #28670

KSXGitHub opened this issue Nov 26, 2018 · 10 comments
Labels
Duplicate An existing issue was already created

Comments

@KSXGitHub
Copy link
Contributor

KSXGitHub commented Nov 26, 2018

Suggestion

Current definition of IteratorResult demands value to be provided regardless of whether done is true or false.

Suggestion: Change IteratorResult to something like this

type IteratorResult<T> =
  { readonly done: true } |
  { readonly done: false, readonly value: T }

Use Cases

I am currently trying to build a custom async iterator that cannot be done with async generator, it is absurd that I had to provide some value when iteration is done.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 26, 2018

I think I just force cast to as any or sth.

Edit:
Found in my code, lol

                        //We passed the end of the last page
                        return {
                            done : true,
                            value : undefined as any,
                        };

@KSXGitHub
Copy link
Contributor Author

@AnyhowStep Yeah, that was my workaround too. But I think it shouldn't be, hence the request.

@cpplearner
Copy link

duplicate of #11375?

@hwanders
Copy link

According to the spec and MDN the value property may (but is not required to) be present when done is true.
I think the proper typing should be

type IteratorResult<T> =
  { readonly done: true, readonly value?: T } |
  { readonly done: false, readonly value: T }

or even:

type IteratorResult<T> =
  { readonly done: true, readonly value?: any } |
  { readonly done: false, readonly value: T }

@weswigham
Copy link
Member

@rbuckton think we've got enough infrastructure in place to be able to try this lib change yet?

@weswigham weswigham added Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Experience Enhancement Noncontroversial enhancements labels Nov 26, 2018
@DanielRosenwasser DanielRosenwasser added Duplicate An existing issue was already created and removed Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Experience Enhancement Noncontroversial enhancements labels Nov 26, 2018
@KSXGitHub
Copy link
Contributor Author

@hwanders If we were to follow the spec, done should be optional:

type IteratorResult<T> =
  { readonly done: true, readonly value?: T } |
  { readonly done?: false, readonly value: T }

@rbuckton
Copy link
Contributor

rbuckton commented Nov 27, 2018

For reference, @KSXGitHub is talking about this: https://tc39.github.io/ecma262/#sec-iteratorresult-interface (emphasis added):

If the end of the iterator was reached done is true. If the end was not reached done is false and a value is available. If a done property (either own or inherited) does not exist, it is consider to have the value false.

And this (emphasis added):

If done is false, this is the current iteration element value. If done is true, this is the return value of the iterator, if it supplied one. If the iterator does not have a return value, value is undefined. In that case, the value property may be absent from the conforming object if it does not inherit an explicit value property.

@rbuckton
Copy link
Contributor

There is, however, nothing that indicates done or value are read-only.

@KSXGitHub
Copy link
Contributor Author

KSXGitHub commented Nov 27, 2018

@rbuckton Although the spec does not demand the properties to be either writable or read-only, not making them read-only makes it impossible to use a read-only object (e.g. frozen) as an IteratorResult. The way I see it, you are not supposed to modify these properties unless stated otherwise.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a duplicate and has seen no activity in the last day. It has been closed automatic house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

8 participants