-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve flow() typing #1825
Improve flow() typing #1825
Conversation
Impact: before this MR, flow() only need a generic type when the passed function does not have any parameter. Now, it's always required (for a good reason). |
src/api/flow.ts
Outdated
export function flow(generator: Function) { | ||
export function flow<T, U extends any[]>( | ||
generator: (...args: U) => FlowIterator<any> | ||
): CancellablePromise<U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why <U>
? And why CancellablePromise instead of function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No significant meaning here. It can also be changed to <R, A>
to follow the previous naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it cannot. Right result type is (...args: U) => CancellablePromise<any>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I missed the () =>
part. Strange that no tests are failing.
edit: nvm, the tests are in .js
There's a way to make flow properly detect the return argument as well (except when the return argument is a promise, but that can be circumvented with a fake function such as castFlowReturn or similar) |
src/api/flow.ts
Outdated
export function flow(generator: Function) { | ||
export function flow<T, U extends any[]>( | ||
generator: (...args: U) => FlowIterator<any> | ||
): (...args: U) => CancellablePromise<U> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<U>
again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
See: #1816