Skip to content
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

Merged
merged 2 commits into from
Oct 13, 2023
Merged

Take a first stab at async results #87

merged 2 commits into from
Oct 13, 2023

Conversation

jstasiak
Copy link
Collaborator

@jstasiak jstasiak commented Oct 11, 2023

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.

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

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
@jstasiak jstasiak changed the title [LUN-2855] Take a first stab at async results Take a first stab at async results Oct 11, 2023

andThen<T2, E2>(
mapper: (val: T) => Result<T2, E2> | Promise<Result<T2, E2>> | AsyncResult<T2, E2>
): AsyncResult<T | T2, E | E2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is E here?

Copy link
Collaborator Author

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.

* 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> {
Copy link
Member

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, ?

Copy link
Collaborator Author

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.

Copy link
Member

@rbruggem rbruggem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@jstasiak jstasiak merged commit 3f55d15 into master Oct 13, 2023
@jstasiak jstasiak deleted the asyncresult branch October 13, 2023 08:07
Copy link

@raguiar9080 raguiar9080 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jstasiak added a commit that referenced this pull request Mar 4, 2024
A follow up to [1].

Two more convenience composition methods in AsyncResults and I was
missing recently.

The sync Result has them already (with different signatures, of course).

[1] 3f55d15 ("Take a first stab at async results (#87)")
jstasiak added a commit that referenced this pull request Mar 4, 2024
A follow-up to [1].

Two more convenience composition methods in AsyncResults and I was
missing recently.

The sync Result has them already (with different signatures, of course).

[1] 3f55d15 ("Take a first stab at async results (#87)")
jstasiak added a commit that referenced this pull request Mar 4, 2024
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)")
jstasiak added a commit that referenced this pull request Mar 4, 2024
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)")
jstasiak added a commit that referenced this pull request Mar 4, 2024
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)")
@jstasiak jstasiak mentioned this pull request Mar 4, 2024
jstasiak added a commit that referenced this pull request Mar 4, 2024
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)")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants