Add toGenerator utility function #1543
Conversation
|
Please note that I've not regenerated the docs (which causes a single test failure), as when I do there are many changes relating to the name of my fork project, rather than the real one. |
The toGenerator function is intended for wrapping a promise-returning function to create a generator-returning function. This can then be used to obtain a strongly-typed return value in async actions using `yield*`.
fruitraccoon
force-pushed
the
feature/to-generator
branch
from
d6cd893
to
3960c14
Compare
|
Thanks @fruitraccoon! i will review it at the weekend i believe |
| const m = M.create() | ||
|
|
||
| const result = await m.testAction() | ||
| ensureNotAny(result) |
There was a problem hiding this comment.
Now that the repo is upgraded to ts 3.9, we may use
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-9.html#-ts-expect-error-comments
for this kind of assertions
There was a problem hiding this comment.
I don't think I can use @ts-expect-error in this case? If I understand correctly, it's for when typescript wants to report an error, but in the context of the test it's the intended code.
In this test I'm wanting the opposite, as what I'm testing for is that the type of the returned value is "not any", which never normally shows up as an error of course. With the current code, if I change one of the yield* calls to just be yield, a typescript error appears (as intended).
I've simplified the check from two functions to one, and added a comment. Let me know if I've misunderstood though!
There was a problem hiding this comment.
What you cloud do:
// result return type is string. if there is no type error here, means we have wrong return type
// @ts-expect-error
const notAny: number = result
;
const isString: string = result;There was a problem hiding this comment.
I didn't think of that approach - nice!
I couldn't think of a way to have a "one-liner" to do both checks though (a la the ensureNotAnyType function). Would you prefer to keep the tests as they are now, or switch them to the technique above?
|
Would you mind adding also: /**
* @experimental
* experimental api - might change on minor/patch releases
*
* Convert a promise to a generator yielding that promise
* This is intended to allow for usage of `yield*` in async actions to
* retain the promise return type.
*
* Example:
* ```ts
* function getDataAsync(input: string): Promise<number> { ... }
*
* const someModel.actions(self => ({
* someAction: flow(function*() {
* // value is typed as number
* const value = yield* promiseToGenerator(getDataAsync("input value"));
* ...
* })
* }))
* ```
*/
function* promiseToGenerator<R>(p: Promise<R>): Generator<Promise<R>, R, R> {
return yield p as any;
}And a test |
The new name is more accurate, based on the description of generators from MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator > The Generator object is returned by a generator function
Use a single function that ensures the type passed is of the expected type, and is NOT the typescript `any` type.
|
No problem, I've added the extra function to convert a promise. However I've renamed both it and the original function, as when I look at MDN for generators:
So my original function name choice was not ideal. So now the original function is called But let me know if you prefer the original names, something shorter, or something different altogether. |
I personally prefer verbose names (promiseToGenerator) So i think we can take out the PR from draft mode? |
|
Merged! thank you! |
|
Released as v3.17.1, I'm trying to figure out how to update the website |
The
toGeneratorfunction is intended for wrapping a promise-returning function to create a generator-returning function. This can then be used to obtain a strongly-typed return value in async actions usingyield*(rather thanyield).This approach has been discussed in this Spectrum thread.