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
IteratorResult<T> definition conflicts with the JS spec #38479
Expected behavior:
The code should compile without errors. Actual behavior:
The code compiles with the error: "Type '{ done: true; }' is not assignable to type 'IteratorResult<string, any>'.
Property 'value' is missing in type '{ done: true; }' but required in type 'IteratorReturnResult'.(2322)"
The error can be bypassed by changing { done: true } to { value: undefined, done: true }. However, the specification allows omitting the value property. See Iteration Protocols
A suggested fix would be to change the definition of the type IteratorReturnResult to:
I confess I don't fully understand what the feared failure is, based on reading issue 30790. Is there a specific example of a type deduction failure that can illustrate that?
In any case, an alternative fix could be as follows, although without fully understanding the objection, I don't know if this is any better. This would see IteratorReturnResult tun into a type alias, and default TResult to match T to improve type inference:
I think there 2 kind of propblems will arise with value?: TReturn:
won't allow to use return value without checking for undefined first
won't catch cases when I forget to return a value.
constlist=['v1','v2','v3']constf: ()=>Iterator<string,number>=()=>{letindex=0return{next: (): IteratorResult<string,number>=>{console.log(list)if(index<list.length){return{value: list[index++]}}else{return{done: true,value: index}// Ooops, I can forget to return value here}}}}constiter=f();constres=iter.next();if(res.done){console.log(`I wish I had ${res.value*10} values instead`);// ~~~~~~~~~ Object is possibly 'undefined'. ts(2532)}
Both points are inconveniences, but this is what the spec requires - value is optional when done is true, and therefore it can be undefined. Forcing a different protocol here could create an issue when interfacing with an iterator obtained from a JS library that follows the spec - TS asserts that value is always defined, yet the JS iterator could return { done: true }.
Although it’s not clear what to do in the case where done is dynamically determined and therefore not statically known to be true or false (I’ve actually written code where this is the case). Should value be optional or mandatory in that case?
Although it’s not clear what to do in the case where done is dynamically determined and therefore not statically known to be true or false (I’ve actually written code where this is the case). Should value be optional or mandatory in that case?
IteratorResult is currently defined as a discriminated union. The problem with letting value be optional is as Andrii Dieiev (@IllusionMH) describes:
won't allow to use return value without checking for undefined first
The only time it would be type-safe to allow value to be optional would be when TReturn is either undefined or void. We can't change the definition of IteratorReturnResult to make value optional only to serve that specific case. It might be worth considering whether we should consider a void property to be optional, just as we currently consider a void parameter "optional": Playground example.
Daniel Rosenwasser (@DanielRosenwasser) - Any thoughts on extending the same treatment of a void parameter to a void property and treating it as an optional property?
Ron Buckton (@rbuckton) Maybe I'm missing something but the discriminated union I described would have value be mandatory if done is false - which means you only need to check for nullish values if done is true, and it's been my experience that in most cases code doesn't tend to return a meaningful value for done: true anyway (I'm open to being proven wrong on this, though). A typical loop would likely be:
constiter=obj[Symbol.iterator]();while(!iter.done){iter.value;// never undefined, no need to check}
No need to check for undefined inside the loop since the discriminated union in combination with CFA takes care of that. The only time you'd need to check is if done is true, which you probably should do anyway, or else use a non-nullish assertion ! if you're absolutely sure.
edit: I screwed up the loop, but it gets the point across and I'm too lazy to fix it. 😛
To reiterate, the problem is that, as written, IteratorReturnResult conflicts with the Iterator spec. Bruce Pascoe (@fatcerberus) and I have suggested fixes. The argument against fixing it, so far, seems to be that it would make it less convenient to use, by requiring an undefined check. Two points:
The definition of the Iterator spec is what's driving the need for an 'undefined' check. I understand that one may wish it didn't, but that's a complaint about the spec itself. It doesn't work to have a type that reflects how we wish the spec to be and not how it actually is.
The inconvenience of checking for undefined seems extremely minor, especially considering the optional chaining and non-null assertion operators. Moreover, and this bears repeating, the Iterator spec is defined in a way that makes the 'undefined' check unavoidable.
fwiw I ran into this in practice when attempting to use a ReadableStream as an async interator. E.g. this example from Jake Archibald's article doesn't type correctly:
function streamAsyncIterator(stream) {
// Get a lock on the stream:
const reader = stream.getReader();
return {
next() {
// Stream reads already resolve with {done, value}, so
// we can just call read:
return reader.read();
},
return() {
// Release the lock if the iterator terminates.
reader.releaseLock();
return {};
},
// for-await calls this on whatever it's passed, so
// iterators tend to return themselves.
[Symbol.asyncIterator]() {
return this;
}
};
}
Since, I, too, am having compilation problems regarding the IteratorReturnResult I add to this issue by adding a point in favor of Lubo Antonov (@ldantonov).
The following is the current definition of the Iterator iterface and the IteratorReturnResult type provided by typescript and it contains a kind of paradox
interfaceIteratorYieldResult<TYield>{done?: false;value: TYield;}interfaceIteratorReturnResult<TReturn>{done: true;value: TReturn;// here the value can't be undefined}typeIteratorResult<T,TReturn=any>=IteratorYieldResult<T>|IteratorReturnResult<TReturn>;interfaceIterator<T,TReturn=any,TNext=undefined>{next(...args: []|[TNext]): IteratorResult<T,TReturn>;return?(value?: TReturn): IteratorResult<T,TReturn>;// here the value provided by the user can be undefinedthrow?(e?: any): IteratorResult<T,TReturn>;}
If i try to implement the class MyIterator as follow:
classMyIterator<T,TReturn,TN>implementsIterator<T,TReturn,TN>{next(...args: []|[TN]): IteratorResult<T,TReturn>{thrownewError("Method not implemented.");}return?(value?: TReturn): IteratorResult<T,TReturn>{return{done: true, value };// what should I use as value if the user set the value parameter to undefined by calling myIterator.return() with empty paranthesis?// from MDN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#the_iterator_protocol/* return(value) Optional A function that accepts zero or one argument and returns an object conforming to the IteratorResult interface, typically with value equal to the value passed in and done equal to true. Calling this method tells the iterator that the caller does not intend to make any more next() calls and can perform any cleanup actions. */}throw?(e?: any): IteratorResult<T,TReturn>{thrownewError("Method not implemented.");}}
I get the following error:
(property) IteratorReturnResult<TReturn>.value: TReturn
Type 'TReturn | undefined' is not assignable to type 'TReturn | T'.
Type 'undefined' is not assignable to type 'TReturn | T'.ts(2322)
So why the Iterator iterface allows to pass undefined to the Iterator.return function if the IteratorReturnResult don't allows to have undefined as a valid value for the value property?
There is no way to provide a default value for the TResult generic since it can be of any type and the done property typically is true (as stated in the MDN).
To have it working the current return signature should be return(value: TReturn) but then it will rises issues when the done property is set to false devs will be forced to pass a value to the return function.
As they are right now the Iterator interfaces are not consistent with the Iterator protocol and neither with themselves.
I don't know what is the best solution but i upvote the one proposed by Lubo Antonov (@ldantonov) since is the most closed to the iterator protocol: the iterator protocol itself tells you to check for undefined values
The shape for IteratorReturnResult is as close to the specification as would be allowed by type safety given the current capabilities of TypeScript without introducing significant complexity in the library typings. The only case in which a value could be safely elided for IteratorReturnResult is when TResult includes either void or undefined as a constituent, thus simply making value optional is not type safe.
We previously investigated whether we could make properties optional if they are instantiated with the type void, but ultimately decided against that as it would add more complexity to void.
For now, I am closing this as a design limitation.
Activity
IllusionMH commentedon May 12, 2020
Suggested fix won't work as described here https://github.com/microsoft/TypeScript/pull/30790/files#r282595851
ldantonov commentedon May 12, 2020
I confess I don't fully understand what the feared failure is, based on reading issue 30790. Is there a specific example of a type deduction failure that can illustrate that?
In any case, an alternative fix could be as follows, although without fully understanding the objection, I don't know if this is any better. This would see IteratorReturnResult tun into a type alias, and default TResult to match T to improve type inference:
IllusionMH commentedon May 12, 2020
I think there 2 kind of propblems will arise with
value?: TReturn:undefinedfirstldantonov commentedon May 17, 2020
Both points are inconveniences, but this is what the spec requires -
valueis optional whendoneis true, and therefore it can beundefined. Forcing a different protocol here could create an issue when interfacing with an iterator obtained from a JS library that follows the spec - TS asserts thatvalueis always defined, yet the JS iterator could return{ done: true }.fatcerberus commentedon Jun 11, 2020
One potential solution would be to define
IteratorResultas a discriminated union (withdoneas the discriminant):Although it’s not clear what to do in the case where
doneis dynamically determined and therefore not statically known to be true or false (I’ve actually written code where this is the case). Shouldvaluebe optional or mandatory in that case?rbuckton commentedon Jun 27, 2020
IteratorResultis currently defined as a discriminated union. The problem with lettingvaluebe optional is as Andrii Dieiev (@IllusionMH) describes:The only time it would be type-safe to allow
valueto be optional would be whenTReturnis eitherundefinedorvoid. We can't change the definition ofIteratorReturnResultto makevalueoptional only to serve that specific case. It might be worth considering whether we should consider avoidproperty to be optional, just as we currently consider avoidparameter "optional": Playground example.Daniel Rosenwasser (@DanielRosenwasser) - Any thoughts on extending the same treatment of a
voidparameter to avoidproperty and treating it as an optional property?fatcerberus commentedon Jun 27, 2020
Ron Buckton (@rbuckton) Maybe I'm missing something but the discriminated union I described would have
valuebe mandatory ifdoneis false - which means you only need to check for nullish values ifdoneis true, and it's been my experience that in most cases code doesn't tend to return a meaningful value fordone: trueanyway (I'm open to being proven wrong on this, though). A typical loop would likely be:No need to check for undefined inside the loop since the discriminated union in combination with CFA takes care of that. The only time you'd need to check is if
doneis true, which you probably should do anyway, or else use a non-nullish assertion!if you're absolutely sure.edit: I screwed up the loop, but it gets the point across and I'm too lazy to fix it. 😛
ldantonov commentedon Jun 29, 2020
To reiterate, the problem is that, as written,
IteratorReturnResultconflicts with the Iterator spec. Bruce Pascoe (@fatcerberus) and I have suggested fixes. The argument against fixing it, so far, seems to be that it would make it less convenient to use, by requiring anundefinedcheck. Two points:19 remaining items
zaach commentedon Sep 21, 2022
fwiw I ran into this in practice when attempting to use a
ReadableStreamas an async interator. E.g. this example from Jake Archibald's article doesn't type correctly:Amebus commentedon Jan 21, 2023
Since, I, too, am having compilation problems regarding the IteratorReturnResult I add to this issue by adding a point in favor of Lubo Antonov (@ldantonov).
The following is the current definition of the
Iteratoriterface and theIteratorReturnResulttype provided by typescript and it contains a kind of paradoxIf i try to implement the class
MyIteratoras follow:I get the following error:
So why the
Iteratoriterface allows to passundefinedto theIterator.returnfunction if theIteratorReturnResultdon't allows to haveundefinedas a valid value for thevalueproperty?There is no way to provide a default value for the
TResultgeneric since it can be of any type and thedoneproperty typically istrue(as stated in the MDN).To have it working the current
returnsignature should bereturn(value: TReturn)but then it will rises issues when thedoneproperty is set tofalsedevs will be forced to pass a value to thereturnfunction.As they are right now the
Iteratorinterfaces are not consistent with the Iterator protocol and neither with themselves.I don't know what is the best solution but i upvote the one proposed by Lubo Antonov (@ldantonov) since is the most closed to the iterator protocol: the iterator protocol itself tells you to check for undefined values
rbuckton commentedon Aug 15, 2024
The shape for
IteratorReturnResultis as close to the specification as would be allowed by type safety given the current capabilities of TypeScript without introducing significant complexity in the library typings. The only case in which avaluecould be safely elided forIteratorReturnResultis whenTResultincludes eithervoidorundefinedas a constituent, thus simply makingvalueoptional is not type safe.We previously investigated whether we could make properties optional if they are instantiated with the type
void, but ultimately decided against that as it would add more complexity tovoid.For now, I am closing this as a design limitation.