-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
π‘ Yield Overrides #43632
Comments
Why is this better than using Your motivating example could be rewritten to type |
@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 ( |
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>);
} |
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. |
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";
} |
@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
Imho if |
One use case for this is Mobx's flow, which uses generator functions in which |
@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 |
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 π |
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? |
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 |
Most of the time - most of the yielded expressions return something meaningful to the "caller". |
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
Both library heavily uses yield expressions results. You can think of it basically as a plugable |
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 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 |
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":
In this case, the solution you mention is blocked by this other issue #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). |
@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 |
@arcanis can you give a small code example of something using redux-saga that gained a new implicit any error in 4.3? |
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 |
@arcanis SagaIterator is slightly better than typing with |
Ping @andrewbranch? Did the example help? |
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.
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 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 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. |
@arcanis I'm assuming
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;
} |
@rbuckton would be the best person to give some thoughts on this, though I think he may still be on vacation this week. |
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. |
@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. |
Yep, feel free to ping me on the PR asking for that π |
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, 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. |
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 |
@Andarist Right, but in this case because the 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 |
Hmm... that is definitely odd. |
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, I believe 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. |
In response to this concern:
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: #36967 (comment) I was not aware of this issue here when I was writing this code so this demo doesn't use YieldType |
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 With these as primitives, you can build any higher set of operations that fully consume the generator and therefore use |
Following this blogpost: https://www.matechs.com/blog/abusing-typescript-generators 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 |
@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.
@Jack-Works you are basically describing #36967, in fact with parameter inference you could probably specify the expected
@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 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 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 |
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 ayield
expression, and to instead use whatever is used as generic parameter. The following would thus be true:π 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:
Under the current model, TypeScript is forced to type
counter
asany
, because in theory the generator runner may return whatever value it wants to theyield
expression. It can be somewhat improved by adding an explicit type to the generator, but then allyield
expressions will return the same type - which is obviously a problem when doing multipleselect
calls, each returning a different value type:Not only does it prevent return type inference and yield value checks, but it also unnecessarily widens the type of both
counter
andfirstName
intonumber | string
. The alternative is to write the expressed values at callsite: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:
By using a
select
variant supportingyield*
, 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 asany
, leaving their users mostly unprotected on segments of code that would significantly benefit from types (for instance theselect
utility mentioned above can return any state slice from a larger one - typing the return asany
leaves the door open to refactoring mistakes, typos, etc).Additionally, under
noImplicitAny
, TS will put red underline on the wholeselect
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:MobX (23.8K β)
https://mobx.js.org/actions.html#using-flow-instead-of-async--await-
Others
Ember Concurrency (663 β)
Effection (138 β)
GenSync (29 β)
π» Try it Out
I've implemented this feature (minus tests, and I'm not a TS contributor so some adjustments may be needed):
master...arcanis:mael/yield-override
Here's a playground: Before | After β¨
cc @Andarist
The text was updated successfully, but these errors were encountered: