-
Notifications
You must be signed in to change notification settings - Fork 21
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
add the 'do notation' function to Future #8
Conversation
Great, thank you for the contribution! 👍 Interestingly i had planned another function with the exact same signature, but named So we are adding the function, 100%. The only question is the name to use. Well, actually my version was a little different than yours:
So, my version is lazy, yours is eager, but the signature is the same. The lazy one won't be started until it's Right now I prefer the lazy one, the naming question remains. |
Nice addition |
Hi Emmanuel, thanks for the response! I agree with you that lazy one is better than mine 👍 Regarding naming question, I am not sure 100% what fits better for this case. I chose "do" because it was familiar to me as is the same in Haskell, but maybe ofLazyPromise is better. |
Maybe |
A bit off topic, but why are |
@qm3ster good point! Well if you use we also have onComplete, onFailure, onSuccess, I think it's clear that these should trigger the future, so be eager.. So again map & flatMap could be lazy if you end by an onComplete & so on. But I was a little unsure about map & flatMap being used standalone. I may yet change this, I'm afraid that people would expect the future to be triggered and it's not. People will likely do side-effects also through Future.map, as they do today in Promise.then, and expect them to be triggered. [agree about documenting that toPromise is eager, will do] |
Note that I'm thinking about minor behaviour changes to onComplete/onFailure/onSuccess so now would be a great time if we want to change some other things. Unsure what to do though. Future is still brand new, so I'm definitely open to changes, including relatively large ones. |
I think only things like |
I opened #9 to continue the laziness debate. Let's keep this PR discussion about the lazy constructor from promises. |
aha but we can discuss naming like of & ofPromise & ofCallback here. We can still make these changes at this point yes. |
renamed #9 to "Future api changes". here would be just about the building of Futures, which is a smaller debate. |
For best ergonomics, I think the following constructors are needed: // triggered beyond our control
declare const ofPromise: <T>(promise: Promise<T>) => Future<T>
// the spiciest, accepts `async` functions directly
declare const of: <T>(fn: () => Promise<T>) => Future<T>
// Provides node style callbacks. Throwing like `ofCallbackApi` isn't ergonomic,
// because you can only throw synchronously, but defer resolving.
// Basically equal to `of(promisify(fn))`
/* Example: */ ofCallback(cb => fs.readFile('/foo.txt', cb)) // is better
/* than writing */ of(() => promisify(fs.readFile)('/foo.txt'))
declare const ofCallback: <T>(cb: (err: any, result: T) => void) => Future<T>
// Don't know how to name it, but the benefit is obvious
declare const liftBBBB: <A extends any[], T>(
fn: (...args: A) => Promise<T>,
) => ((...args: A) => Future<T>)
// Example:
const prefix = liftBBBB(async (inp: string) => '_' + inp)
// typeof prefix === const prefix: (inp: string) => Future<string>
// Same signature as `new Promise()`
// for when you want to write a promise by hand
declare const lazyPromiseConstructor: <T>(
executor: (resolve: (T) => void, reject: (any) => void) => void,
) => Future<T> |
regarding the possible constructors suggested by @qm3ster:
So, I'll think about naming, especially if Future won't be lazy anymore. Will think about it (and comments will be possible in my PR if not before). |
|
ok, for Future.lift, I had to adapt a little your code because I'm using a class for Future, but it does seem to work very well: static lift<T extends any[],U>(fn: (...args: T)=>Promise<U>): (...args:T)=>Future<U> {
return args => Future.of(fn(...args));
} I don't think there's a need to do anything regarding So this looks good, however I think I'll delay a little the inclusion of this function in prelude.ts, because I don't want to rely on too new versions of typescript. Already when I added use of Regarding the rest of your comments, you can see the PR that I made. Thank you again for your comments! |
@emmanueltouzery I think |
hmm damn you're right! I was using Thank you for the very valuable feedback! |
You could include it for just unaries. Or just unaries and binaries. // Defined with overloads
function dift <T1,U>(fn:(a:T1)=>Promise<U>):(a:T1)=>Future<U>
function dift <T1,T2,U>(fn:(a:T1,b:T2)=>Promise<U>):(a:T1,b:T2)=>Future<U>
function dift <T1,T2,T3,U>(fn:(a:T1,b:T2,c:T3)=>Promise<U>):(a:T1,b:T2,c:T3)=>Future<U>
function dift <T1,T2,T3,T4,U>(fn:(a:T1,b:T2,c:T3,d:T4)=>Promise<U>):(a:T1,b:T2,c:T3,d:T4)=>Future<U>
// Body from 1986
{
return function () {
var args = arguments
return Future.of<U>(function () {
return fn.apply(undefined, args)
})
} as any
}
// Still works with arrow functions, despite `arguments` usage
const lool = dift(async (x: number, y: number) => x + y) |
We could have an overload version for now, good idea, but I'd rather wait a little for the real thing, typescript releases are very fast recently. For the name.. |
You can just retype this function later, not breaking any code, just breaking compatibility with very old tsc. |
well I would have rather waited just to avoid doing the work twice and also a little due to concern of potentially breaking some client code but I guess it's really safe, and also you've basically given the implementation. So we could include it in the next release then, the only thing we need is a name. I was thinking about I wanted to look how they name this in scala-land.. and it turns out... they say it's lifting if I understand correctly:
On the other hand for the lifting when all parameters are lifted besides the result, they say:
It is there => PartialFunction.lift would seem to be similar to what we are doing. Also see: So that would mean lifting is a general concept, of which functor lifting is only a sub-category. On the other hand this one says lifting is only "functor lifting": So, maybe lifting is an appropriate naming after all, or maybe not. Maybe only the scala community is abusing the terminology, or maybe there's a difference between what they do and what we want to do here. I am quite unsure :-( And then I come back to the names I mentioned with Otherwise in terms of typescript version compatibility, no hard rule, but I'm aiming to supporting the latest two versions. So currently that would fail because the better way you suggests fails on 2.9. Once 3.1 is released, it would work on 3.1 & 3.0, so the two latest versions, and I would include the better version. |
@RPallas92 you could have easily missed it in all the noise but in the end your PR was merged, so in master there is now a |
@emmanueltouzery many thanks! This lib is very useful for me. |
Added a "do notation" function in order to use async/await without caring to add try/catch blocks