-
Notifications
You must be signed in to change notification settings - Fork 8
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
Take a first stab at async results #87
Conversation
It's very common to see this pattern in our (Lune) code: const thing: Promise<Result<T, E>> = ... // Now we want to return an error if Err but if Ok we want to // process it further with asynchronous code, so first await it: const result = await thing // Then manualy branch on the variant: if (result.isErr()) { return result } // Only now we can process it: const anotherThing: Promise<Result<U, E>> = asyncFunction(result.value) // But wait! We want to process it with *another* function (asyncAnotherFunction). // That means another await/if/return/process block. // ... This PR attempts to make this whole thing much better and now the following will be possible: return new AsyncResult(thing) .andThen(asyncFunction) .andThen(asyncAnotherFunction) * Why a separate type? Because I didn't find a way to fit promises/asynchronous mappers in the existing API in a way that provides static type safety and works correctly at runtime. * Why only AsyncResult and not AsyncOption? Both makes sense but AsyncResult is the biggest bang for the buck for us. * Why only the two methods (andThen and map)? We need these the most, the rest can be added later (when/if we decide we need them). * Why not add async versions of the methods directly to Result (asyncMap, asyncAndThen etc.)? I decided it would result in too much duplication. * Why allow synchronous functions to be composed with AsyncResult? For convenience – once you're in the AsyncResult realm it'll be the easiests to just stay in it and compose as many things as possible before finally converting back to Result. Part-of: https://linear.app/lune/issue/LUN-2855/add-async-support-to-ts-results-es
docs/reference/api/asyncresult.rst
Outdated
|
||
andThen<T2, E2>( | ||
mapper: (val: T) => Result<T2, E2> | Promise<Result<T2, E2>> | AsyncResult<T2, E2> | ||
): AsyncResult<T | T2, E | E2> |
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.
What is E
here?
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.
The type of the original error (if the result is an error (type E
) then andThen()
keeps it and doesn't call mapper
.
src/asyncresult.ts
Outdated
* await badResult.andThen(async (value) => Ok(value * 2)).promise // Err('boo') | ||
* ``` | ||
*/ | ||
andThen<T2, E2>(mapper: (val: T) => Result<T2, E2> | Promise<Result<T2, E2>> | AsyncResult<T2, E2>): AsyncResult<T | T2, E | E2> { |
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 is T
required in AsyncResult<T | T2,
?
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.
It's not, good spot. Whatever mapper()
returns is our only possible success value type here.
Fixed.
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.
Nice.
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.
A piece of AsyncResult API that I find missing every now and then. A follow-up to [1] and [2]. The purpose, like with the other methods, is to more easily compose pieces of potentially asynchronous code together. [1] 3f55d15 ("Take a first stab at async results (#87)") [2] b5f8af9 ("Implement AsyncResult or and orElse (#109)")
A piece of AsyncResult API that I find missing every now and then. A follow-up to [1] and [2]. The purpose, like with the other methods, is to more easily compose pieces of potentially asynchronous code together. [1] 3f55d15 ("Take a first stab at async results (#87)") [2] b5f8af9 ("Implement AsyncResult or and orElse (#109)")
Something that comes up every now and then and I've been missing it from the ts-results-es API: an asynchronous counterpart to Option. Also a counterpart to AsyncResult added in [1]. This addition completes the sync/async, Option/Result matrix. [1] 3f55d15 ("Take a first stab at async results (#87)")
Something that comes up every now and then and I've been missing it from the ts-results-es API: an asynchronous counterpart to Option. Also a counterpart to AsyncResult added in [1]. This addition completes the sync/async, Option/Result matrix. [1] 3f55d15 ("Take a first stab at async results (#87)")
It's very common to see this pattern in our (Lune) code:
This PR attempts to make this whole thing much better and now the following will be possible:
I already implemented more methods so I kept the implementations commented out just so
we don't have to do the same work again.
Part-of: https://linear.app/lune/issue/LUN-2855/add-async-support-to-ts-results-es