search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.17k stars 12.51k forks source link

💡 Yield Overrides #43632

Open arcanis opened 3 years ago

arcanis commented 3 years ago

Suggestion

🔍 Search Terms

yield, any, generator, saga

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

A new YieldType builtin generic type would instruct TypeScript to ignore the actual generator type in a yield expression, and to instead use whatever is used as generic parameter. The following would thus be true:

declare const val: YieldType<boolean>;

function* generator() {
  const res = yield val;
  assertType<res, boolean>();
}

📃 Motivating Example

Imagine you define an API where the user provides a generator and can interact with your application by the mean of "effects". Something akin to this:

run(function* () {
  const counter = yield select(state => state.counter);
});

Under the current model, TypeScript is forced to type counter as any, because in theory the generator runner may return whatever value it wants to the yield expression. It can be somewhat improved by adding an explicit type to the generator, but then all yield expressions will return the same type - which is obviously a problem when doing multiple select calls, each returning a different value type:

run(function* (): Generator<any, any, number | string> {
  const counter = yield select(state => state.counter);
  const firstName = yield select(state => state.firstName);
});

Not only does it prevent return type inference and yield value checks, but it also unnecessarily widens the type of both counter and firstName into number | string. The alternative is to write the expressed values at callsite:

run(function* () {
  const counter: number = yield select(state => state.counter);
  const firstName: string = yield select(state => state.firstName);
});

But it then requires the user code to have an explicit requirement on the exact types, which prevents accessing various refactoring tools and generally leads to worse UX (after all that's why TS has type inference in the first place). The last option is to change the API:

run(function* () {
  const counter = yield* selectS(state => state.counter);
  const firstName = yield* selectS(state => state.firstName);
});

By using a select variant supporting yield*, library code would be able to define a return type that TS would be able to use. However, it requires all libraries to adopt a non-idiomatic runtime pattern just for the sake of TypeScript, which is especially problematic considering that it would lead to worse performances.

The main point is that in all those cases, the library code already knows what's the value returned by the yield select(...) expression. All we need is a way to transmit this information to TypeScript.

💻 Use Cases

Redux-Saga (21.5K ⭐, 6 years old)

The redux-saga library heavily use generators (there are various reasons why async/await isn't enough, you can see them here). As a result, its collection of effects are all typed as any, leaving their users mostly unprotected on segments of code that would significantly benefit from types (for instance the select utility mentioned above can return any state slice from a larger one - typing the return as any leaves the door open to refactoring mistakes, typos, etc).

Additionally, under noImplicitAny, TS will put red underline on the whole select call, hiding important errors that could arise within the selector itself. In some of my own application files, about a third of the lines have red squiggles because of this issue:

image

MobX (23.8K ⭐)

https://mobx.js.org/actions.html#using-flow-instead-of-async--await-

Note that the flowResult function is only needed when using TypeScript. Since decorating a method with flow, it will wrap the returned generator in a promise. However, TypeScript isn't aware of that transformation, so flowResult will make sure that TypeScript is aware of that type change.

Others

💻 Try it Out

I've implemented this feature (minus tests, and I'm not a TS contributor so some adjustments may be needed):

https://github.com/microsoft/TypeScript/compare/master...arcanis:mael/yield-override

Here's a playground: Before | After ✨

cc @Andarist

nmain commented 3 years ago

Why is this better than using : Generator<..., ..., T>? That can be enforced at call sites too:

Typescript Playground

Your motivating example could be rewritten to type run as something like run: (f: (props) => Generator<...>) => void and the type would be inferred inside the anonymous function* as well.

Andarist commented 3 years ago

@nmain in your demo it's not possible to receive a type dependent on the yielded type. This can be done when using yield delegates (yield *) but not with yield alone which is used to create "effects" system (like redux-saga, koa etc)

nmain commented 3 years ago

I see, yes, that is useful. Unfortunately your proposal needs a call thunk to do that. For instance, if my generator runner communicates back a string when yielded a number, and a number when yielded a string, there's no clean way to express that:

function* foo() {
  // not so pretty
  var a = yield (5 as number & YieldOverride<string>);
  var b = yield ("foo" as string & YieldOverride<number>);
}
arcanis commented 3 years ago

Exactly, which defers the responsibility of providing the types to the user (at every callsite!), which is what this proposal attempts to avoids. The YieldOverride primitive would be interpreted by TypeScript as "when it goes through a yield expression, this value must resolve to T, regardless what the generator thinks".

The following would thus work as expected: TypeScript playground.

Andarist commented 3 years ago

That is right but also - maybe not a super common use case. Thunks are usually pretty common for those kinds of patterns.

However, this is a valid criticism and maybe an alternative approach could be figured out. For instance, we could "dispatch" to the injected type using conditional types like this:

type Cond<T> = T extends string ? number : T extends number ? string : never;

function* foo(): YieldOverride<Cond> {
  var a: string = yield 5;
  var b: number = yield "foo";
}
vaukalak commented 3 years ago

@Andarist I think this is right direction. However I'd suggest to keep all 3 params of generator (T, TReturn and TNext). The only addition needed here is to allow TNext to be a resolver type for T:

type InferResolve<Yielded> =
  Yielded extends CallEffect ?
    CallEffectResolve<Yielded> :
    Yielded extends SelectEffect ?
      SelectEffectResolve<Yielded> :
      never;

type ReduxSaga = Generator<
  CallEffect | SelectEffect,
  void,
  InferResolve // ts complains that should be parametrized.
>;

function* betterFoo(): ReduxSaga {
  const value = (yield sampleCallEffect); // should be "string" one day
}

full example

Imho if Generator generic third param could be (take yielded value and infer resolved type), that would be enough.

nmain commented 3 years ago

One use case for this is Mobx's flow, which uses generator functions in which yield foo has a runtime type equal to await foo.

arcanis commented 3 years ago

@andrewbranch I've implemented this feature (here), do you mind if I open a PR?

Current usage:

type SelectEffect = {type: `select`};
declare function select<T>(): SelectEffect & YieldType<T>;

// Just some test utilities
type AssertEqual<T, Expected> = [T, Expected] extends [Expected, T] ? true : false;
const assertEqual = <U>() => <V>(val: V, expected: AssertEqual<U, V>) => {};

function* foo() {
  const x: SelectEffect = select<number>();
  assertEqual<SelectEffect>()(select<number>(), true);
  assertEqual<number>()(yield select<number>(), true);
}

(to be clear, suggestions in this thread have their own merits, however I have no idea how I would implement them; the YieldType approach is simple enough, works for most real-life cases, and has the benefit to have a precedent in the form of ThisType)

andrewbranch commented 3 years ago

I think since we haven’t decided on a direction to go for this issue, it’s unlikely that your PR will get much attention from the team, which might mean it gets closed after a while. This issue is still the best place to discuss the problem and try to promote it from “Awaiting More Feedback” to something more committed. In my view, having a prototype implementation doesn’t help its chances, since it doesn’t fall into the category of “it’s a good idea but we don’t know how or don’t have time to implement it.” But generally, it’s not something you need to ask permission for as long as you’re aware that it may not go anywhere. We tend to advise against implementing uncommitted things just to avoid wasted work, but that’s not relevant here since you’ve already done it 😁

arcanis commented 3 years ago

Makes sense! What would you say is the current action item, however? Your team hasn't weighted yet, so it's difficult to know what's the type of feedback you're looking for. Is it a case of getting the idea upvoted? Or is there a concrete problem that should be addressed in the proposal?

andrewbranch commented 3 years ago

At one point I jotted down this progression:

I think in this case, the problem clarity is good, the popularity is lacking, the applicability is not yet proven (does anyone outside redux-saga and koa care about this?), the level of pain the problem causes could be elaborated a bit (so all the yield expressions in redux-saga are typed as any, but how often do people need to use their results?), and the replaceability of the proposed solution is unclear to me, but I see other ideas being discussed in this thread so I think the design space seems very open still. Maybe @rbuckton can help give some specific guidance, because any shortcomings of Generator typings is something I guarantee he’s thought about a lot.

Andarist commented 3 years ago

so all the yield expressions in redux-saga are typed as any, but how often do people need to use their results?

Most of the time - most of the yielded expressions return something meaningful to the "caller".

EmrysMyrddin commented 3 years ago

Another example of library using generators as co-routine is https://github.com/tj/co. It has 11.5k stars, so it's a pretty popular library and has a stable API since a lot of time.

Their is lot of other library attempting to do the same but with less adoption. But we have to take in account that while Typescript doesn't allow to type this kind of library, their popularity will be by design limited. A Typescript project will probably not choose a library which insert any every where in your code.

so all the yield expressions in redux-saga are typed as any, but how often do people need to use their results?

Both library heavily uses yield expressions results. You can think of it basically as a plugable async/await system which allows to await something else than a Promise.

treybrisbane commented 3 years ago

As much as I'd love a solution to TypeScript's generator typing problems, I don't feel like this is the direction we should go.

The main problem I see with this proposal is that it makes the yielded value responsible for defining a yield expression's type. In practice though, it's the thing that's "running" a generator (e.g. co) that's ultimately responsible for defining a yield expression's type (based on the yielded value), because it's the thing that provides a yield expression's result (when calling next).

IMO, the best way to tackle this is via the approach mentioned by @rbuckton here, or via the alternative he mentioned here. Both of these approaches express the type of a yield expression as some (type-level) function of the type of a yielded value, which correctly models the runtime behaviour.

arcanis commented 3 years ago

The main problem I see with this proposal is that it makes the yielded value responsible for defining a yield expression's type. In practice though, it's the thing that's "running" a generator (e.g. co) that's ultimately responsible for defining a yield expression's type (based on the yielded value), because it's the thing that provides a yield expression's result (when calling next).

Yes, it would be sounder if the generators were in charge of defining the returned value, there's no argument over that. However, TypeScript's (non-)goals explicitly allow for this kind of "incorrectness":

Non-goals

  • Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.

In this case, the solution you mention is blocked by this other issue https://github.com/microsoft/TypeScript/issues/1213 opened since 2014. While it perhaps will be addressed someday, there's no indication that it'll be anytime soon, or even that it won't suffer from a drawback preventing it from being used in this particular case. In the meantime, generators suffer from subpar developer experience that hinder their adoption and negatively affect our codebases.

By contrast, letting yielded expressions annotate the value that will be sent back is, admittedly, somewhat backward, but it can be easily implemented with few changes to the codebase (and thus doesn't significantly increase the maintenance cost for the TS team), and solves most of the practical use cases seen in popular tools.

It can also be noted that both approaches aren't incompatible: should high order type parameters become a thing, Type Override being a thing won't prevent generators to improve (at worst they'd just become slightly less relevant).

arcanis commented 3 years ago

@andrewbranch I've updated the OP with more information and context, does that help?

Something to note is that this issue becomes more important with TS 4.3, because Saga-based projects now yield a bunch of previously non-reported errors, making the migration quite painful (literally hundreds of places that need to be addressed, and the best way for that is to type the returned generator as any, which makes the fix even worse than the problem ...):

expression implicitly results in an 'any' type because its containing generator lacks a return-type annotation
andrewbranch commented 3 years ago

@arcanis can you give a small code example of something using redux-saga that gained a new implicit any error in 4.3?

arcanis commented 3 years ago

Sure, here's an example: playground. Seems like it started throwing in 4.2 rather than 4.3, but the point still stands.

This is just one little piece, of course - upgrading required us to add hundreds of ts-ignore statements because of this one pattern. The warning itself isn't the problem though, we'd just prefer if we were able to properly type this segment of the codebase in the first place.

neurosnap commented 3 years ago

@arcanis SagaIterator is slightly better than typing with any for saga generators. It still doesn't address the core issue here, but I'm sure you could come up with a codemod to at least "fix" the errors without using an escape hatch like any or ts-ignore.

arcanis commented 2 years ago

Ping @andrewbranch? Did the example help?

cowboyd commented 2 years ago

Would love to see this gain some traction since all of the use-cases mentioned in the description are really powerful. This is why concepts like structured concurrency are going mainstream in languages like Java and Swift.

In the case of Effection (mentioned in the description) it's one of our biggest points of confusion that trips up new users, and now with the dominant mind-share of TypeScript within the JS ecosystem, I'd argue one of the biggest barriers to adoption. In fact, the reason I came upon this issue in the first place, was because I'm typing up a guide on using Effection with TypeScript.

The main problem I see with this proposal is that it makes the yielded value responsible for defining a yield expression's type. In practice though, it's the thing that's "running" a generator (e.g. co) that's ultimately responsible for defining a yield expression's type (based on the yielded value), because it's the thing that provides a yield expression's result (when calling next).

This is the point exactly. Despite being responsible for providing the value, the exported types that come with "the thing running the generator" currently may have absolutely no say in what the type of that value will be.

However, if you look at all the use-cases above, they pertain to frameworks whose entire raison d'être is to place runtime constraints on how generators will compose. For example, in Effection, the code driving the generator will never ever ever call next() with anything but the return value of the invoked generator, the type of which is strictly known. The same is true of both ember-concurrency and redux-saga, and if it weren't, it would be a critical bug that would garner the full attention of those project's developers.

We take it as an article of faith that a browser, or the "thing driving the async function" will yield the value produced by a promise and so we consider an await expressions type-safe, but we are still putting our trust in that the runtime. Generators by contrast, must always have this "thing running them" that is ever-present, but completely invisible to the type system, and so I'd argue that any type-system that doesn't account for it, or doesn't allow it to become visible is unsound in practice if not in theory.

And in truth, we've seen this happen on our own projects where a manual type annotation becomes "stale" and program that successfully type checks is actually incorrect. On the other hand, allowing the runtime to give the compiler hints about type-inference would actually produce more correct code, more often.

In short, JavaScript has an extension mechanism to the manner in which expressions produce values, but TypeScript does not have a concomitant extension mechanism for expressing the types of those values.

cowboyd commented 2 years ago

@arcanis I'm assuming YieldType would be assignable from any type? I ask because I'm trying to imagine how to write the operation type in Effection so that it still places the right constraints on the actual generators (and not just the yielded values). In other words:

  1. If the type parameter is T, then the return type of the generator must be T
  2. the generator may only yield operations and nothing else.
export type Operation<T> = YieldType<T> & Iterator<Operation<any>, T>;

function* yes(): Operation<boolean> {
  let value = yield (function*() { return true; })();
  return value;
}

function* no(): Operation<boolean> {
  // @ts-expect-error
  return "false";
}

function* alsoNo(): Operation<boolean> {
  // @ts-expect-error
  yield 5;

  return false;
}
andrewbranch commented 2 years ago

@rbuckton would be the best person to give some thoughts on this, though I think he may still be on vacation this week.

cowboyd commented 2 years ago

Any thoughts? If there's anything I can do personally to move this (or some alternative) forward, I would very much like to. I just don't know which direction to push.

arcanis commented 2 years ago

@andrewbranch if I open it as a PR, would it be possible to pack it to make it usable in a playground? This way people would be able to play with the proposal.

Andarist commented 2 years ago

@arcanis this PR is packed like this so i assume that it would be possible

andrewbranch commented 2 years ago

Yep, feel free to ping me on the PR asking for that 👍

arcanis commented 2 years ago

Here's a playground of the feature: Before | After ✨

cowboyd commented 2 years ago

Fantastic @arcanis! I gave it a try on a paired down version of Effection types, and it works marvelously. There was one case I did have a question about. In Effection, Promise is a valid operation type, and so you can yield to it, but I couldn't find a way to say "in the context of this generator, if you yield a Promise<T>, treat it as though it were Promise<T> & YieldType<T>.

Obviously don't want to monkey patch the global promise type, but since the type of the generator is recursive, and can only yield other operations. e.g. Iterator<Operation<unknown>, T, unknown> it seems like it should be something that is possible to be inferred?

Andarist commented 2 years ago

It's an inherent trait of this design - it's the yielded value/type that decided what gets "injected" back to the generator. So, as you have said, it's somewhat impossible to overload existing types with a custom/unwrapped/whatever yielded type. So to make it work you kinda have to convert the promise using a wrapper function, like here (I'm pretty sure that you know that, I'm just posting this as a reference for others).

Note that with your Operation type I've expected an error here but it doesn't raise one 🤔

cowboyd commented 2 years ago

It's an inherent trait of this design - it's the yielded value/type that decided what gets "injected" back to the generator

@Andarist Right, but in this case because the TYield type of my operation is itself Operation, so in order for the generator function to type check, you cannot yield Promise<T> because that is not an operation, however Promise<T> & YieldType<T> is an operation. So I was wondering if that could then be used to infer the yield type.

It's a bit circular, but what I'm wondering is why it can't figure out from the type of the generator that the yielded value is actually Promise<T> & YieldType<T>(which can therefore be used to infer the left hand side of the expression.)

cowboyd commented 2 years ago

Note that with your Operation type I've expected an error here but it doesn't raise one 🤔

Hmm... that is definitely odd.

Jack-Works commented 2 years ago

I support this as a worth-solving problem. Yield-generator itself in JavaScript can be used to emulate powerful control flow structures (as many examples described above). Lacking type support will limit the user experience in this case.

I have a use case https://github.com/Jack-Works-forks/rfcs/blob/cancel-async-effects/text/0000-cancel-async-effects.md#basic-example I proposed an auto-cancellation effect API but it is lacking support from the type system.

useEffect(function* (_signal) {
    setLoading(true)
    // React automatically stops if the deps array outdated.
    setText(yield getData(props.id))
}, [props.id])

In the case above, yield T will be always T by the semantic of useEffect I proposed. But there is no way to modal it in the type system.

I believe YieldType<T> is not enough, it would be better if the type system can behave this way:

type UnboxPromise<T> = T extends PromiseLike<infer Q> ? Q : never
declare function co<T, TR>(g: () => Generator<T, TR, UnboxPromise<T>>): Promise<TR>

// contextual typing for the generator function
co(function* () {
    // T = Promise<number>, UnboxPromise<T> results in number
    const a = yield Promise.resolve(1)
    //    a: number

    // T = Promise<string>, UnboxPromise<T> results in string
    const b = yield Promise.resolve("")
    //    b: string

    return Promise.resolve(true)
}) //return: Promise<boolean>

This requires the type system to specially handle generator function to provide an ad-hoc multiple type evaluation.

arcanis commented 2 years ago

@rbuckton would be the best person to give some thoughts on this, though I think he may still be on vacation this week.

Ping @rbuckton, do you have any thoughts on this?

scorbiclife commented 2 years ago

In response to this concern:

Given the discussion there, I personally feel that this is not the right approach—this is essentially just a type assertion, but piggy-backing on non-type-assertion syntax in such a way that nobody will recognize that it’s just as unsafe. The next step is not finding someone to champion this PR; we are still on the very first step: exploring possible solutions that could be driven to consensus. https://github.com/microsoft/TypeScript/pull/47548#issuecomment-1167579213

I am not sure but I think I found one way of implementing a type-safe method for annotating and calling generators:

// This does not contradict the type system, hopefully?

interface MonadicGenerator<YieldCount extends number, YieldList extends unknown[], Return extends any, NextArgsList extends unknown[][]> {
    next<Index extends YieldCount>(...args: NextArgsList[Index]): IteratorReturnResult<Return>;
    next<Index extends number = number>(...args: NextArgsList[Index]): IteratorYieldResult<YieldList[Index]>;
}

const genFunc = function* () {
    const a = yield new Just(5);
    const b = yield new Just("6");
    return new Just(a + parseInt(b, 10));
} as () => MonadicGenerator<2, [Maybe<number>, Maybe<string>], Maybe<number>, [[undefined], [number], [string]]>

function naiveDoNotation<T0, T1, R> (
    genFunc: () => MonadicGenerator<2, [Maybe<T0>, Maybe<T1>], Maybe<R>, [[undefined], [T0], [T1]]>
) {
    const gen = genFunc();
    const { value: ma } = gen.next<0>(undefined);
    return ma.bind((a) => {
        const { value: mb } = gen.next<1>(a);
        return mb.bind((b) => {
            const { value: result } = gen.next<2>(b);
            return result;
        })
    })
}

Link to full playground: https://github.com/microsoft/TypeScript/issues/36967#issuecomment-1241492260 Edit: Improved the code in https://github.com/microsoft/TypeScript/issues/32523#issuecomment-1242874821

I was not aware of this issue here when I was writing this code so this demo doesn't use YieldType but maybe this idea can be incorporated to this proposal somehow?

cowboyd commented 2 years ago

For what it's worth, we're migrating Effection away from using bare yield statements because it turns out that you can get what you want without any modifications to TypeScript by always consuming the return value of a generator which is strongly typed.

The motivating example becomes:

run(function* () {
  const counter = yield* select(state => state.counter);
  const firstName = yield* select(state => state.firstName);
});

The secret is to make the generators themselves composable as delimited continuations with shift()/reset(). The technique is demonstrated here https://github.com/thefrontside/continuation

With these as primitives, you can build any higher set of operations that fully consume the generator and therefore use TReturn for all type inference.

wnadurski commented 2 years ago

Following this blogpost: https://www.matechs.com/blog/abusing-typescript-generators I have implemented a generic do notation that makes use of fp-ts implementation of HKT and type classes. This is how it can be used:

it('works for option', () => {
  const result = Do(O.Monad)(function* (_) {
    const x = yield* _(O.some(2))
    const y = yield* _(O.some('asd'))

    return y + x.toString()
  })

  expect(result).toEqual(O.some('asd2'))
})

it('works for IO', () => {
  const someSideEffect = jest.fn()

  const result = Do(IO.Monad)(function* (_) {
    const x = yield* _(IO.of(2))
    yield* _(() => someSideEffect())

    return 'asd' + x.toString()
  })

  expect(result()).toEqual('asd2')
  expect(someSideEffect).toHaveBeenCalled()
})

it('works for Task', async () => {
  const someSideEffect = jest.fn().mockReturnValue(Promise.resolve(undefined))

  const result = Do(T.Monad)(function* (_) {
    const x = yield* _(T.of(123))

    yield* _(someSideEffect)

    const y = yield* _(T.of(10))

    return x + y
  })

  expect(await result()).toEqual(133)
  expect(someSideEffect).toHaveBeenCalled()
})

Here is full code: https://gist.github.com/wnadurski/51a3e8cf06c8fe0cc3dd50b87fd90365

tadhgmister commented 1 year ago

Note that with your Operation type I've expected an error here but it doesn't raise one thinking

@Andarist This is just a limitation of typescript where if the return type of a generator has a conditional non iterator type it can't check the yield sites. (playground) it has nothing to do with this proposal and is present in the latest version of typescript.

In the case above, yield T will be always T by the semantic of useEffect I proposed. But there is no way to modal it in the type system.

I believe YieldType is not enough, it would be better if the type system can behave this way:

@Jack-Works you are basically describing #36967, in fact with parameter inference you could probably specify the expected yield parameter in your library function and have it automatically inferred for the generator function passed directly in the same way that inference works here

I couldn't find a way to say "in the context of this generator, if you yield a Promise<T>, treat it as though it were Promise<T> & YieldType<T>.

@arcanis I don't see how this actually fixes anything unless you are specifically using a library that maintains complete control over the type at every single yield site, if my library just uses Promise or even strings or similar objects at the yield site then you still end up needing some way to inject the YieldType into the type within the generator.

Like pretend I had a library that the yielded values were just strings to denote labels in a database that is lazily loaded as needed and the list of valid labels is statically known, I don't think this proposal actually benefits this.

interface A { a: number }
interface B { b: string }
function* current(){ 
  // doesn't actually ensure the "A"->A, "B"->B logic
  const x: A = yield "A" as const
  const y: B = yield "B" as const
  assertType<A>(x); assertType<B>(y);
}
function* using43632(){
  // unclear whether this is actually any better...
  const x = yield ("A" as "A" & YieldType<A>);
  const y = yield ("B" as "B" & YieldType<B>);
  assertType<A>(x); assertType<B>(y);
}

This is obviously a toy example but this is exactly analogous to the case of yielding Promises which is very much a desired usecase.

I feel like an approach that decouples the library yield contract from the type of yielded value is a better approach such as #36967 (playground showing equivalent code in both cases)

Note that in your PR the TSend generic is currently evaluating to unknown which makes it really easy to write an unsound runner code. in the playground above current has a TSend value of A&B since that is the only type that'd satisfy all yield positions so a runner would need to either satisfy all of them or use type assertions, this is basically what I get at in https://github.com/microsoft/TypeScript/issues/36855#issuecomment-589081258