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

Boolean literal types and return type propagation for generators #2983

Closed
JsonFreeman opened this issue May 1, 2015 · 58 comments · Fixed by #30790 or #9407
Closed

Boolean literal types and return type propagation for generators #2983

JsonFreeman opened this issue May 1, 2015 · 58 comments · Fixed by #30790 or #9407
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@JsonFreeman
Copy link
Contributor

This suggestion has a few pieces:

  1. Implement boolean literal types for true and false, in a fashion similar to Singleton types under the form of string literal types #1003
  2. Implement type guards for booleans. Essentially, control flow constructs like if-else, while, for, etc would be subject to a type guard if their guard expression is a boolean.
  3. Now we can update generators to have a next method that returns { done: false; value: TYield; } | { done: true; value: TReturn; }, where TYield is inferred from the yield expressions of a generator, and TReturn is inferred from the return expressions of a generator.
  4. Iterators would return { done: false; value: TYield; } | { done: true; value: any; } to be compatible with generators.
  5. for-of, spread, array destructuring, and the type yielded by yield* would only pick the value associated with done being false.
  6. The value of a yield* expression would pick the value associated with done being true.
  7. Introduce a Generator type that can be used to track the desired type of a yield expression. This would be TNext, and would be the type of the parameter for the next method on a generator.

The generator type would look something like this:

interface Generator<TYield, TReturn, TNext> extends IterableIterator<TYield> {
    next(value?: TNext): IteratorYieldResult<TYield> | IteratorReturnResult<TReturn>;
    throw(exception: any): IteratorYieldResult<TYield> | IteratorReturnResult<TReturn>;
    return(value: any): IteratorYieldResult<TYield> | IteratorReturnResult<TReturn>;
    [Symbol.iterator](): Generator<TYield, TReturn, TNext>;
    [Symbol.toStringTag]: string;
}
@ahejlsberg
Copy link
Member

Boolean literal types are now available in #9407. Once that is merged we can update the return type of generators and iterators.

@s-panferov
Copy link

s-panferov commented Jul 5, 2016

Does it allow to type async-like yield expressions in libraries like co or react-saga?

// getUser(): Promise<User>
let user = yield getUser()
// user: ?

@yortus
Copy link
Contributor

yortus commented Jul 5, 2016

@s-panferov unfortunately not. You are talking about the async-runner style of generator used by things like co right?

The generator proposal (#2873) doesn't offer much typing support for async-runner generators. In particular:

  • All yield expressions are typed as any, as you can see in the example comments below. This is actually a very complex problem and I tried tackling it with some ideas which are all there in Proposal for generators design #2873. This won't change with Number, enum, and boolean literal types #9407. And the TNext type in the OP above won't solve this either, since in an async runner there is generally not a single TNext type, but rather the type of each yield expression is a function of the type of that yield's operand (eg Promise<T> maps to T, Array<Promise<T>> maps to T[], etc). In particular the type of each yield expression is generally unrelated to the types of the other yield expressions in the body in an async-runner generator function.
  • The compiler infers a single TYield type that must be the best common type among all the yield operands. For async runners there often is no such best common type, so such generators often won't compile until at least one yield operand is cast to any. E.g. the function *bar example below doesn't compile for this reason.
  • Return types of generators are currently not tracked because without boolean literals, they can't be separated from the TYield type. This is what will be solvable once Number, enum, and boolean literal types #9407 lands.

Example code:

interface User {id; name; address}
interface Order {id; date; items; supplierId}
interface Supplier {id; name; phone}
declare function getUser(id: number): Promise<User>;
declare function getOrders(user: User): Promise<Order[]>;
declare function getSupplier(id: number): Promise<Supplier>;

function* foo() {
    let user = yield getUser(42); // user is of type 'any'
    let user2 = <User> user;
    return user2; // This return type is not preserved
}

function* bar() { // ERROR: No best common type exists among yield expressions
    let user = yield getUser(42);       // user has type 'any'
    let orders = yield getOrders(user); // orders has type 'any'

    let orders2 = <Order[]> orders;
    let suppliers = yield orders2.map(o => getSupplier(o.supplierId)); // suppliers has type 'any'

    let suppliers2 = <Supplier[]> suppliers;
    return suppliers2; // This return type is not preserved
}

@s-panferov
Copy link

s-panferov commented Jul 5, 2016

@yortus big thanks for the clarification!

All yield expressions are typed as any, as you can see in the example comments below. This is actually a very complex problem and I tried tackling it with some ideas which are all there in #2873. This won't change with #9407. And the TNext type in the OP above won't solve this either, since in an async runner there is generally not a single TNext type, but rather the type of each yield expression is a function of the type of that yield's operand (eg Promise maps to T, Promise[] maps to Promise<T[]>, etc). In particular the type of each yield expression is generally unrelated to the types of the other yield expressions in the body in an async-runner generator function.

Do you know if there is a tracking issue for this use-case? I think we definitely need to continue discussion, because this use-case is quite common and becomes more and more popular.

@yortus
Copy link
Contributor

yortus commented Jul 5, 2016

@s-panferov no problem. I think there's just #2873. There's quite a lot of discussion about the async-runner use-case in there, but I think that the team wanted to focus on getting simpler use cases working initially. Since that issue is now closed, I guess you could open a new issue focused specifically on better typing for co-style generators.

@DanielRosenwasser
Copy link
Member

This hasn't actually been fixed yet.

@DanielRosenwasser
Copy link
Member

The issue as I see it is that without #2175, this would be a breaking change. For example, you start out fixing IteratorResult:

interface IteratorYieldResult<Y> {
    done: false;
    value: Y;
}

interface IteratorReturnResult<R> {
    done: true;
    value: R;
}

type IteratorResult<Y, R> = IteratorYieldResult<Y> | IteratorReturnResult<R>

Now all of a sudden you need to introduce another type parameter to Iterator:

interface Iterator<Y, R> {
    next(value?: any): IteratorResult<Y, R>;
    return?(value?: any): IteratorResult<Y, R>;
    throw?(e?: any): IteratorResult<Y, R>;
}

which infects Iterable & IterableIterator:

interface Iterable<Y, R> {
    [Symbol.iterator](): Iterator<Y, R>;
}

interface IterableIterator<Y, R> extends Iterator<Y, R> {
    [Symbol.iterator](): IterableIterator<Y, R>;
}

These now break any users of Iterators. For instance, Array's members needed to be fixed up to:

interface Array<T> {
    /** Iterator */
    [Symbol.iterator](): IterableIterator<T, undefined>;

    /** 
      * Returns an array of key, value pairs for every entry in the array
      */
    entries(): IterableIterator<[number, T], undefined>;

    /** 
      * Returns an list of keys in the array
      */
    keys(): IterableIterator<number, undefined>;

    /** 
      * Returns an list of values in the array
      */
    values(): IterableIterator<T, undefined>;
}

@JsonFreeman
Copy link
Contributor Author

Yes I remember our long discussion about this. The tricky bit is that many users will just want to use for-of, spread and rest, which never use the R type. Those users will not care about R, only Y. Then there are some users who will call the iterator methods explicitly, and they will care about the R type. The art is in serving both use cases simultaneously. I think there needs to be a type with two type parameters, and another type with only one, where the second type argument is any.

@falsandtru
Copy link
Contributor

I feel definitions using literal types is too complex for common interfaces because we need to explicitly assert a boolean literal type for now. We need more easy ways to use literal types.

function iter(): IteratorResult<void, void> {
  return {
    done: <true>true
  };
}

@Igorbek
Copy link
Contributor

Igorbek commented Oct 5, 2016

With respect to what @JsonFreeman said according to the concern raised by @DanielRosenwasser, I experimented with a hypothetical typing of iterators that may return values.

Currently we have this:

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

interface Iterator<T> {
    next(value?: any): IteratorResult<T>;
    return?(value?: any): IteratorResult<T>;
    throw?(e?: any): IteratorResult<T>;
}

This can be changed to:
Sorry for names end with 2, that's just for illustration

interface IteratorYieldResult<Y> {
    done: false;
    value: Y;
}
interface IteratorReturnResult<R> {
    done: true;
    value: R;
}

type IteratorResult2<T, R> = IteratorYieldResult<T> | IteratorReturnResult<R>;
// redefine IteratorResult through extended interface to preserve generic arity
type IteratorResult<T> = IteratorResult2<T, any>;

interface Iterator2<T, R, I> {
    next(value?: I): IteratorResult2<T, R>;
    return?(value: R): IteratorResult2<T, R>;
    throw?(e?: any): IteratorResult2<T, R>;
}
// redefine Iterator through extended interface to preserve generic arity
type Iterator<T> = Iterator2<T, any, any>;

Open questions:

  • next(value: I) or next(value?: I)
    • if value is optional then we will not be able to enforce strict type I in yield expression in generators 👎
    • if value were not be optional then to allow next() we'd need to either type I | undefined or require consumer to work around like next(undefined as I) 👎
  • return(value: R) strictly requires value to be passed, right?

@JsonFreeman
Copy link
Contributor Author

Nice typing @Igorbek!

I don't think the return parameter should be required. I get the impression that the return method is largely for cleanup, just executing any finally clauses that correspond to try/catch blocks surrounding the execution point (for a generator in particular). Unless you have a yield expression in one of the finally blocks, the consumer will already have the value they want returned.

For the next parameter, I think it has to be optional. Consider the language constructs for iterating (for-of, spread, etc). None of those constructs pass a parameter. If the generator really needs the parameter to be passed, then I think it is misleading to call it an iterator. It might be better to have a separate generator type for that, since the consumer will have to interact with the object in a different way.

@yortus
Copy link
Contributor

yortus commented Oct 5, 2016

Further to @JsonFreeman's comment, there are two very different uses generators in real-world code:

(1) For creating a series of values to iterate over:

  • don't care about the return value
  • don't pass anything to next
  • do care about yielded values, which are usually all the same type

(2) For async runners (e.g. using the co library):

  • do care about the return value, which is treated differently to the yielded values
  • do pass values to next
  • do care about yielded values, which are generally of unrelated types.

The latter case (async runners) should diminish with the growing awareness of async/await, but there's still a lot of existing code out there using generators this way.

@jesseschalken
Copy link
Contributor

jesseschalken commented Oct 5, 2016

For reference, this is what Flow currently has for typing ES6 iterators and generators (from here, blog post here.). Flow is stricter than TypeScript but I suspect they had to make much of the same decisions.

type IteratorResult<Yield,Return> = {
  done: true,
  value?: Return,
} | {
  done: false,
  value: Yield,
};

interface $Iterator<+Yield,+Return,-Next> {
    @@iterator(): $Iterator<Yield,Return,Next>;
    next(value?: Next): IteratorResult<Yield,Return>;
}
type Iterator<+T> = $Iterator<T,void,void>;

interface $Iterable<+Yield,+Return,-Next> {
    @@iterator(): $Iterator<Yield,Return,Next>;
}
type Iterable<+T> = $Iterable<T,void,void>;

declare function $iterate<T>(p: Iterable<T>): T;

/* Generators */
interface Generator<+Yield,+Return,-Next> {
    @@iterator(): $Iterator<Yield,Return,Next>;
    next(value?: Next): IteratorResult<Yield,Return>;
    return<R>(value: R): { done: true, value: R };
    throw(error?: any): IteratorResult<Yield,Return>;
}

I don't think the return parameter should be required. I get the impression that the return method is largely for cleanup, just executing any finally clauses that correspond to try/catch blocks surrounding the execution point (for a generator in particular). Unless you have a yield expression in one of the finally blocks, the consumer will already have the value they want returned.

Unless the generator yields or returns inside a finally block as you said, .return(x) on a generator always returns {done: true, value: x}. So Flow appears to have the correct type with return<R>(value: R): {done: true, value: R}, although if you wanted to handle the case of yield or return in finally correctly it would be return<R super Return>(value: R): IteratorResult<Yield,R>.

I think calling .return() should infer R as undefined so you know you're going to get {done: true, value: undefined}.

For the next parameter, I think it has to be optional. Consider the language constructs for iterating (for-of, spread, etc). None of those constructs pass a parameter. If the generator really needs the parameter to be passed, then I think it is misleading to call it an iterator. It might be better to have a separate generator type for that, since the consumer will have to interact with the object in a different way.

Any generator that can be used with for..of must be prepared to see undefined as a result of yield, and IMO must include undefined in its input type (eg void), which will also allow calling .next() without a parameter. for..of on a generator that does not include undefined in its input type should be an error.

The reason Flow has the parameter to .next() as optional is because a generator first needs a call to .next() to start it, and the parameter provided to that call is ignored, but you can't express that a parameter is only optional for the first call.

@Igorbek
Copy link
Contributor

Igorbek commented Oct 6, 2016

@jesseschalken thanks a lot for the reference how Flow typed iterator/generator.
There're a few things to consider or think about:

As more I think about next's parameter then more I'm in favor of making it required.
Of course, for..of calls it without any parameter, which in fact means argument's type would be undefined. But what if I want to use push side of a generator and expect to get something pushed in? Of course, I would not be able to iterate it with for..of, and that's ok. If I wanted I would have added undefined to the domain of next's parameter type. So that for-of-able is an iterator which has undefined in the domain of I generic type.

function* a(): Iterator2<number, string, string> {
  const s = yield 1; // s is string, not string|undefined
  return s;
}
for (let i of a()) {
              ~~~ Iterator2<number, string, string> cannot be used in for..of
}

function* b(): Iterator2<number, string, string|undefined> {
  const s = yield 1; // s is string|undefined
  return s || "";
}
for (let i of b()) { // ok
}

@JsonFreeman
Copy link
Contributor Author

@Igorbek what about the first push, where you need not push anything?

@jesseschalken
Copy link
Contributor

jesseschalken commented Oct 6, 2016

return<R> - I personally don't like it, because it opens a way for consumer to cheat the contract. technically, a generator is not obligated to return exact value passed to return, it even can prevent closing and return done=false.

Yep, as I said:

if you wanted to handle the case of yield or return in finally correctly it would be return<R super Return>(value: R): IteratorResult<Yield,R>.

This would allow you to call .return(), which would fill R with Return|undefined causing the result for that call to be {done: false, value: Yield} | {done: true, value: Return|undefined}, covering all three cases of yield, return and neither in the finally block.

However, last I checked TypeScript didn't have super type constraints, so I'm not sure how to otherwise express that in TypeScript.

edit: You could just do return<R>(value: R): IteratorResult<Yield,Return|R>

they account for type variance 👍 ping #1394 #10717

Yep, TypeScript's function parameter bivariance is a huge pain and we're considering migrating to Flow soon for that reason among other strictness benefits.

what about the first push, where you need not push anything?

The parameter to next() should probably be optional for that reason, but for..of should nonetheless still demand an iterator that accepts undefined as input.

edit: It occurred to me that because the typing is structural and the input type for the generator is only mentioned as the parameter to next, if that parameter is optional then a generator which doesn't include undefined in its input type will be indistinguishable from one that does.

@zpdDG4gta8XKpMCd
Copy link

incorrect typing around return type just bit me, wow this issues is old

@brainkim
Copy link

brainkim commented Jun 1, 2019

If this is gonna be worked on, I think a nice easy win separate from typing the return value of generators would be to make the return and throw methods non-optional.
I’m writing a lot of code like:

// having this function return an iterator type with non-optional `return` and
// `throw` results in a type error
function* gen(): IterableIterator<number> {
  yield 1;
  yield 2;
  yield 3;
}

// need an exclamation mark because return is optional, despite the fact
// that the return method is always defined for generator objects.
gen().return!();

@oguimbal
Copy link

I'm curious: Why adding generic parameters to Iterable and AsyncIterable, but not to IterableIterator and AsyncIterableIterator ?

... i have been waiting for this feature for a long time, but not extending iterableIterators forbids using strongly typed yield* result.

Works using this hack, though:

image

@ilbrando
Copy link

@s-panferov unfortunately not. You are talking about the async-runner style of generator used by things like co right?

The generator proposal (#2873) doesn't offer much typing support for async-runner generators. In particular:

  • All yield expressions are typed as any, as you can see in the example comments below. This is actually a very complex problem and I tried tackling it with some ideas which are all there in Proposal for generators design #2873. This won't change with Number, enum, and boolean literal types #9407. And the TNext type in the OP above won't solve this either, since in an async runner there is generally not a single TNext type, but rather the type of each yield expression is a function of the type of that yield's operand (eg Promise<T> maps to T, Array<Promise<T>> maps to T[], etc). In particular the type of each yield expression is generally unrelated to the types of the other yield expressions in the body in an async-runner generator function.
  • The compiler infers a single TYield type that must be the best common type among all the yield operands. For async runners there often is no such best common type, so such generators often won't compile until at least one yield operand is cast to any. E.g. the function *bar example below doesn't compile for this reason.
  • Return types of generators are currently not tracked because without boolean literals, they can't be separated from the TYield type. This is what will be solvable once Number, enum, and boolean literal types #9407 lands.

Example code:

interface User {id; name; address}
interface Order {id; date; items; supplierId}
interface Supplier {id; name; phone}
declare function getUser(id: number): Promise<User>;
declare function getOrders(user: User): Promise<Order[]>;
declare function getSupplier(id: number): Promise<Supplier>;

function* foo() {
    let user = yield getUser(42); // user is of type 'any'
    let user2 = <User> user;
    return user2; // This return type is not preserved
}

function* bar() { // ERROR: No best common type exists among yield expressions
    let user = yield getUser(42);       // user has type 'any'
    let orders = yield getOrders(user); // orders has type 'any'

    let orders2 = <Order[]> orders;
    let suppliers = yield orders2.map(o => getSupplier(o.supplierId)); // suppliers has type 'any'

    let suppliers2 = <Supplier[]> suppliers;
    return suppliers2; // This return type is not preserved
}

As it is not an easy task to make TypeScript able to handle generator functions like they are used by Redux Saga, I have come up with a workaround that works for me.
You can see it here: https://github.com/ilbrando/redux-saga-typescript

@danielnixon
Copy link

@ilbrando another alternative: https://github.com/agiledigital/typed-redux-saga

@ilbrando
Copy link

Thank you @danielnixon this is a really clever solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet