A union including non-iterable types is not iterable #40350
Conversation
…ype, use its type arguments
typescript-bot
added
Author: Team
For Milestone Bug
|
|
||
| const a = yield 0; | ||
| >a : 1 | ||
| >yield 0 : 1 | ||
| >a : 1 | undefined |
There was a problem hiding this comment.
In a sense, this is not wrong—gen could have its next() called with no arguments, making a undefined at runtime. But previously, this fell into a special case of ignoring that optionality for convenience.
Now, when we try to get the iteration types of Generator<R, T, S> | AsyncGenerator<R, T, S> as an async iterable, we get noIterationTypes, because the former doesn’t have a [Symbol.asyncIterator]() member. Instead, we fall back to getting the iteration types of this union as an async iterator, using its next/return/throw method signatures to determine T, TReturn, and TNext. Because we have a union signature, it doesn’t count as a reference to one of the global generator/iterator types, and so it doesn’t take the fast path that ignores the optionality. Instead, it correctly sees that the next() method can take no arguments or [1], and so it infers the type 1 | undefined.
More work could be done to make union signatures take the fast path that drops the optionality if every signature in the union is a reference to one of the global generator/iterator types, but I felt like this was getting fairly complex for limited utility—the results it produces with less complexity are strictly more correct given the limitations we have for modeling this.
| @@ -33444,8 +33444,9 @@ namespace ts { | |||
| if (iterationTypes === noIterationTypes) { | |||
| if (errorNode) { | |||
| reportTypeNotIterableError(errorNode, type, !!(use & IterationUse.AllowsAsyncIterablesFlag)); | |||
| errorNode = undefined; | |||
There was a problem hiding this comment.
The reason I kept looping was to provide some kind of useful type for iteration once they fixed the error. The problem the issue mentioned was that no error was reported.
There was a problem hiding this comment.
(btw: I'm not saying this change is wrong, just expressing the reasoning behind the existing logic).
There was a problem hiding this comment.
Oh, that makes sense in theory, but the problem here was that errorNode was intentionally not supplied, so the non-iterableness of the type was completely lost.
There was a problem hiding this comment.
What do you think about the baseline change?
There was a problem hiding this comment.
It seems fine. I do wonder about the | null case. If it were data[Symbol.iterator]() we would have errored on data with Object is possibly 'null'..
|
This also seems to have fixed #34956. |
See discussion inline.
Fixes #35497.