You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
IterableIterator<T>.next().value is now any instead of T #33353
The root cause is that IterableIterator<T> uses Iterator<T>, only specifying the type of yielded values. The type of returned values (TReturn) defaults to any. As a result, iter.next() is of type IteratorResult<number, any> which equals IteratorYieldResult<number> | IteratorReturnResult<any>. The former has { value: number } and the latter has { value: any }, so the union becomes { value: any }.
That said: IteratorResult is a discriminated union type, so you can use its done property to find out whether you're dealing with an IteratorYieldResult or an IteratorReturnResult. This would also make your code sample more correct: if iter is empty, value will probably not be a number. Unfortunately, you can't use a destructuring assignment then, since that loses the "connection" between done and value.
constiter=getIterator();constresult=iter.next();if(!result.done){constvalue=result.value;// value is a number here}
Reacted by bysdxt, Pierre Leroux, Max Khokhryakov, chpio, Sukka, Krisztián Balla, benstechlab, Federico Tibaldo, Peter Lamby, steve and 6 more
Okay, fair enough. I had been using done but I didn't realize the type was discriminated.
I specifically ran into this with calling numberArray[Symbol.iterator]() – could that type be narrowed since we know the behavior of array iterators? If done is true, then value will be undefined.
Yeah, it's unfortunate that TReturn defaults to any, since most iterators return { done: true, value: undefined } once they're done. I guess it may be needed for backwards compatibility with older code? 🤷♂
Reacted by Jòan, lionel-rowe, Alex C, Jun Wooram and htbkoo
The problem was that we previously conflated the yielded type and the return type in generators. Unfortunately, defaulting to any was necessary for backwards compatibility. Ideally I would default TReturn to void, but that caused significant breaks in real-world code.
@SergeyDvornikovSetronica that's working as intended, as far as I can tell?
If next.done is true, that means there is no value, iteration has finished.
Here's an example to explain more clearly:
function*loadNumbers(): Iterator<number,void>{yield*[1,2,3];}constiterator=loadNumbers();constnext=iterator.next();if(next.done){conststr=next.value.toFixed()// 👍 Correctly errors (you're done - there is no value)}else{conststr=next.value.toFixed()// 👍 Works}
It was more verbose, and I don't need the result type, which I would never use. (I don't know why it exist in JS in the first place - a function should either return or yield, doing both has really confusing ergonomics.)
Simply type-hinting like I did in the example here has been working fine for me.
The remaining question is why does this default to any, which doesn't actually work - instead of never, which works fine?
function*loadNumbers(): Iterator<number,void>{yield*[1,2,3];}constiterator=loadNumbers();constnext=iterator.next();if(next.done){conststr=next.value.toFixed()// 👍 Correctly errors (you're done - there is no value)}else{conststr=next.value.toFixed()// 👍 Works <-- Error}
not working (for me, on ts 3.8.3) with error
error TS2339: Property'toFixed'doesnotexistontype'number | void'.Property'toFixed'doesnotexistontype'void'.491conststr=next.value.toFixed()// 👍 Works
The remaining question is why does this default to any, which doesn't actually work - instead of never, which works fine?
As I said before, never also has a problem:
function*loadNumbers(): Iterator<number,never>{yield*[1,2,3];returnundefinedasnever;}constiterator=loadNumbers();constnext=iterator.next();conststr=next.value.toFixed()// 👍 Works
compiles without problems even though it is an error
@SergeyDvornikovSetronica I think the assumption with these type definitions is you're going to check done - apparently, if you don't check done, the assumption is that's because you know you're not done, but there's no scenario (that I can think of) where you would manually read one result from an iterator, and also no scenario where you would iterate without checking done.
So this doesn't force you to correctly write manual iteration code - that may not have been the primary goal with these types, or it could be that this was just written before TS 3.5 added smarter union type checking.
Arguably, maybe the underlying type union should have been the "reverse" somehow, forcing you to "prove" that there is a value by checking done first, and it would resolve to never if you didn't check.
Here's an example of forcing a caller to check a type union:
typeTResultDone<T>={done: true};// { done: true, value: void } would also work (but value: never would not)typeTResultNotDone<T>={done: false,value: T};typeTResult<T>=TResultDone<T>|TResultNotDone<T>;functionmaybeNumber(): TResult<number>{return{done: false,value: 1};}constresult=maybeNumber();consta=result.value.toFixed();// 👍 correctly fails: you must check done firstif(result.done){constb=result.value.toFixed();// 👍 correctly fails: we're done, there is no value}else{constc=result.value.toFixed();// 👍 succeeds: we're not done, so there is a value}
As noted in the comment, a union with { done: true, value: never } would not work in terms of narrowing the type of value - apparently, a union between void and T is void, whereas a union between never and T is T, so maybe that's the issue with the underlying Iterator types...
I haven't looked closely, but this should be applicable to solving this Iterator problem.
I'm sure more improvements could be made here, which is why I'm calling for this issue to be reopened.
You did mention you're using TS 3.8.3, which is very old - I don't know which other problems it might have, but I did check my last example here with that version, and it worked fine. Maybe consider upgrading to TS 4. You also mentioned IDEA doesn't highlight errors - that's probably because you're using the built-in TS 4 version - to get correct error messages in IDEA (or VS Code or most other IDEs) you need to configure the IDE to use your local TS version instead.
but there's no scenario (that I can think of) where you would manually read one result from an iterator, and also no scenario where you would iterate without checking done.
This is pretty common when you want to get the first element from an iterable that doesn't otherwise provide direct access (e.g. a Set). If the set happens to be empty, then it's going to be undefined. Using never glosses over that easy-to-forget-about case.
As noted in the comment, a union with { done: true, value: never } would not work in terms of narrowing the type of value - apparently, a union between void and T is void, whereas a union between never and T is T, so maybe that's the issue with the underlying Iterator types...
I don't think this is true for any version of TypeScript. The union between void and T is T|void. The problem (to my mind) is that void is strictly less useful than undefined (and will complain if you try to pass it somewhere that accepts T|undefined), and it's perfectly reasonably to use map.values().next().value to get a V|undefined, just like you might write map.get(someKey) to get the same type if you don't know that the key is in the map. I think the precedent of potentially-missing collection elements being T|undefined is pretty strong here, so Iterator<T, undefined> seems like the clearly correct thing to do. I'd be interested to see how much actually breaks if this is changed for specific individual collections.
Shouldn't this be reopened? Now that Iterator Helpers is going to be a thing in browsers, generators are going to be used a lot more outside of for...of loops and therefore this annoying typing problem (which already expresses when trying to get a first value from Set) is only going to get worse.
The type gymnastics required here are fairly extreme, and needlessly so. Also, the solution to achieve the expected behavior is too obstruse -- users should not be expected to know in advance any special knowledge required to achieve the expected result here.
The way this works should be redesigned with these points in mind.
No matter what changes we might make here, the only safe way to access .value is by checking .done. Any operation that merely performs .next().value is going to get the union of the yielded value (T) and the return value (TReturn).
In #58243 we are looking into adding the TReturn and TNext type parameters to Iterable/IterableIterator/AsyncIterable/AsyncIterableIterator just as we do for Iterator/Generator/AsyncIterator/AsyncGenerator, though the default for TReturn will still be any as anything else is a significant breaking change.
We're also looking to explicitly type built-ins to use void for TReturn instead of any to more accurately reflect the underlying behavior. You will still need to discriminate on done to get the correct type for value, but at least the union of both sides will be T | void instead of any.
The .value in the done: true branch isn't relevant. You can use const value = !step.done ? step.value : undefined if you'd like. Alternatively, this helper function can be useful:
functiongetValue<T,TReturn>(res: IteratorResult<T,TReturn>): T{if(res.done){thrownewError("Iterator was closed");}returnres.value;}
and then you can use getValue(setExample.keys().next()).
It's important to remember that the compiler can't make as many assumptions about iterators as it can about a tuple, so inputs[0] in your example is guaranteed to have the type "typescript" because the index is known. The compiler doesn't know about the size of setExample, since it's not fixed, and also doesn't know whether a given call to .next() is the first or last call to that function, or anywhere in between.
Activity
MattiasBuelens commentedon Sep 10, 2019
The root cause is that
IterableIterator<T>usesIterator<T>, only specifying the type of yielded values. The type of returned values (TReturn) defaults toany. As a result,iter.next()is of typeIteratorResult<number, any>which equalsIteratorYieldResult<number> | IteratorReturnResult<any>. The former has{ value: number }and the latter has{ value: any }, so the union becomes{ value: any }.That said:
IteratorResultis a discriminated union type, so you can use itsdoneproperty to find out whether you're dealing with anIteratorYieldResultor anIteratorReturnResult. This would also make your code sample more correct: ifiteris empty,valuewill probably not be anumber. Unfortunately, you can't use a destructuring assignment then, since that loses the "connection" betweendoneandvalue.blixt commentedon Sep 10, 2019
Okay, fair enough. I had been using
donebut I didn't realize the type was discriminated.I specifically ran into this with calling
numberArray[Symbol.iterator]()– could that type be narrowed since we know the behavior of array iterators? Ifdoneis true, thenvaluewill beundefined.MattiasBuelens commentedon Sep 10, 2019
Yeah, it's unfortunate that
TReturndefaults toany, since most iterators return{ done: true, value: undefined }once they're done. I guess it may be needed for backwards compatibility with older code? 🤷♂brainkim commentedon Sep 10, 2019
Related issue:
#33241
Turns out you can’t rely on the checking the
doneproperty to narrow the type ofvalueso in your example abovevalueis still of typeany.MattiasBuelens commentedon Sep 11, 2019
Apparently this only happens when
strictNullChecksis off (see comment). If possible, you can try turning onstrictNullChecks(or even beter:strict).rbuckton commentedon Sep 18, 2019
The problem was that we previously conflated the yielded type and the return type in generators. Unfortunately, defaulting to
anywas necessary for backwards compatibility. Ideally I would defaultTReturntovoid, but that caused significant breaks in real-world code.blixt commentedon Sep 18, 2019
What about
interface Array<T> { [Symbol.iterator]: … }? Can that iterator be typed better thanany?34 remaining items
SergeyDvornikov commentedon Sep 20, 2022
Unfortunately,
Iterator<T, never>also not a fully functional option. Something like thiswill not catch compilation error. Even in strict mode.
Although this code:
will result in a compilation error
papb commentedon Sep 20, 2022
Rasmus Schultz (@mindplay-dk) @SergeyDvornikovSetronica why not just use my solution above?
SergeyDvornikov commentedon Sep 21, 2022
Pedro Augusto de Paula Barbosa (@papb), unfortunately, this does not work, for exactly the same reason that is discussed everywhere.
Simple code:
leads to compilation error in ts 3.8.3 with error:
even with strict mode disabled, although IDEA says everything is fine. Those. exactly what they say a lot where
mindplay-dk commentedon Sep 21, 2022
@SergeyDvornikovSetronica that's working as intended, as far as I can tell?
If
next.doneistrue, that means there is novalue, iteration has finished.Here's an example to explain more clearly:
It was more verbose, and I don't need the result type, which I would never use. (I don't know why it exist in JS in the first place - a function should either return or yield, doing both has really confusing ergonomics.)
Simply type-hinting like I did in the example here has been working fine for me.
The remaining question is why does this default to
any, which doesn't actually work - instead ofnever, which works fine?I think this issue should be reopened.
SergeyDvornikov commentedon Sep 21, 2022
Rasmus Schultz (@mindplay-dk) ,
Yes,
works as intended, but your example:
not working (for me, on ts 3.8.3) with error
although IDEA doesn't highlight any errors
SergeyDvornikov commentedon Sep 21, 2022
Rasmus Schultz (@mindplay-dk) ,
As I said before,
neveralso has a problem:compiles without problems even though it is an error
mindplay-dk commentedon Sep 21, 2022
@SergeyDvornikovSetronica I think the assumption with these type definitions is you're going to check
done- apparently, if you don't checkdone, the assumption is that's because you know you're not done, but there's no scenario (that I can think of) where you would manually read one result from an iterator, and also no scenario where you would iterate without checkingdone.So this doesn't force you to correctly write manual iteration code - that may not have been the primary goal with these types, or it could be that this was just written before TS 3.5 added smarter union type checking.
Arguably, maybe the underlying type union should have been the "reverse" somehow, forcing you to "prove" that there is a
valueby checkingdonefirst, and it would resolve toneverif you didn't check.Here's an example of forcing a caller to check a type union:
As noted in the comment, a union with
{ done: true, value: never }would not work in terms of narrowing the type ofvalue- apparently, a union betweenvoidandTisvoid, whereas a union betweenneverandTisT, so maybe that's the issue with the underlying Iterator types...I haven't looked closely, but this should be applicable to solving this Iterator problem.
I'm sure more improvements could be made here, which is why I'm calling for this issue to be reopened.
You did mention you're using TS 3.8.3, which is very old - I don't know which other problems it might have, but I did check my last example here with that version, and it worked fine. Maybe consider upgrading to TS 4. You also mentioned IDEA doesn't highlight errors - that's probably because you're using the built-in TS 4 version - to get correct error messages in IDEA (or VS Code or most other IDEs) you need to configure the IDE to use your local TS version instead.
shicks commentedon Oct 12, 2022
This is pretty common when you want to get the first element from an iterable that doesn't otherwise provide direct access (e.g. a
Set). If the set happens to be empty, then it's going to be undefined. Usingneverglosses over that easy-to-forget-about case.I don't think this is true for any version of TypeScript. The union between
voidandTisT|void. The problem (to my mind) is thatvoidis strictly less useful thanundefined(and will complain if you try to pass it somewhere that acceptsT|undefined), and it's perfectly reasonably to usemap.values().next().valueto get aV|undefined, just like you might writemap.get(someKey)to get the same type if you don't know that the key is in the map. I think the precedent of potentially-missing collection elements beingT|undefinedis pretty strong here, soIterator<T, undefined>seems like the clearly correct thing to do. I'd be interested to see how much actually breaks if this is changed for specific individual collections.GabenGar commentedon Apr 12, 2024
Shouldn't this be reopened? Now that Iterator Helpers is going to be a thing in browsers, generators are going to be used a lot more outside of
for...ofloops and therefore this annoying typing problem (which already expresses when trying to get a first value fromSet) is only going to get worse.Johnrobmiller commentedon Apr 18, 2024
This issue should be reopened.
The type gymnastics required here are fairly extreme, and needlessly so. Also, the solution to achieve the expected behavior is too obstruse -- users should not be expected to know in advance any special knowledge required to achieve the expected result here.
The way this works should be redesigned with these points in mind.
rbuckton commentedon Apr 22, 2024
No matter what changes we might make here, the only safe way to access
.valueis by checking.done. Any operation that merely performs.next().valueis going to get the union of the yielded value (T) and the return value (TReturn).In #58243 we are looking into adding the
TReturnandTNexttype parameters toIterable/IterableIterator/AsyncIterable/AsyncIterableIteratorjust as we do forIterator/Generator/AsyncIterator/AsyncGenerator, though the default forTReturnwill still beanyas anything else is a significant breaking change.We're also looking to explicitly type built-ins to use
voidforTReturninstead ofanyto more accurately reflect the underlying behavior. You will still need to discriminate ondoneto get the correct type forvalue, but at least the union of both sides will beT | voidinstead ofany.GabenGar commentedon Apr 22, 2024
So is this typescript-approved way to fetch the first value of a set?
Can't even use
const value = !step.done ? step.value : step.valuebecause it widens up toany.rbuckton commentedon Apr 22, 2024
The
.valuein thedone: truebranch isn't relevant. You can useconst value = !step.done ? step.value : undefinedif you'd like. Alternatively, this helper function can be useful:and then you can use
getValue(setExample.keys().next()).It's important to remember that the compiler can't make as many assumptions about iterators as it can about a tuple, so
inputs[0]in your example is guaranteed to have the type"typescript"because the index is known. The compiler doesn't know about the size ofsetExample, since it's not fixed, and also doesn't know whether a given call to.next()is the first or last call to that function, or anywhere in between.rbuckton commentedon Apr 22, 2024
You could also use this code to retrieve the first value: