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

Should IteratorResult have optional value? #8938

Closed
christyharagan opened this issue Jun 2, 2016 · 6 comments
Closed

Should IteratorResult have optional value? #8938

christyharagan opened this issue Jun 2, 2016 · 6 comments
Assignees
Labels
Bug A bug in TypeScript Duplicate An existing issue was already created

Comments

@christyharagan
Copy link

Looking at the ECMA spec for IteratorResult, it seems that value can be optional (when done is true)

In the TypeScript definition, this would translate to:

interface IteratorResult<T> {
    done: boolean;
    value?: T;
}

This does create a potentially annoying situation in which, when `strictNullChecksis true, thevalue``has to be cast to``T`` (unless the new control-flow based type guards are smart enough to recognise this).

As it is, however, it isn't possible to actually implement an Iterator with strictNullChecks because we can't return an undefined value for value.

What is the correct approach here?

@ivogabe
Copy link
Contributor

ivogabe commented Jun 2, 2016

That value was declared optional previously, but that indeed caused to much casts. See #8357. Ideally IteratorResult would be defined as:

type IteratorResult<TYield, TReturn>
  = { done: true, value: TYield }
  | { done: false, value: TReturn }

But that's not possible yet. Then you could define an iterator that does not return as IteratorResult<T, undefined>.

@mhegazy mhegazy added Bug A bug in TypeScript Revisit An issue worth coming back to labels Jun 2, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jun 2, 2016

As noted by @ivogabe we need to have boolean literal types, to be able to model this one accurately.

@mhegazy mhegazy added Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Jun 7, 2016
@benmosher
Copy link
Contributor

I'm fairly new to TypeScript, so maybe this is obvious to others, but I was able to work around with:

    return function next(): IteratorResult<T> {
      if (yielded < arr.length) {
        return { value: arr[yielded++], done: false }
      } else {
        return ({ done: true } as IteratorResult<T>)  // <= explicit cast here!
      }
    }

@mhegazy mhegazy removed the Revisit An issue worth coming back to label Aug 25, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2016

the definition is currently not correct, the correct one would be:

type IteratorResult<TYield, TReturn>
  = { done: false, value: TYield }
  | { done: done, value: TReturn | undefined }

@mhegazy mhegazy added this to the TypeScript 2.1 milestone Aug 25, 2016
@mhegazy mhegazy added Bug A bug in TypeScript and removed Suggestion An idea for TypeScript labels Aug 25, 2016
@DanielRosenwasser
Copy link
Member

Should this be closed in favor of #2983?

@mhegazy
Copy link
Contributor

mhegazy commented Aug 26, 2016

Yes.

@mhegazy mhegazy closed this as completed Aug 26, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Aug 26, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

6 participants