supermacro / neverthrow

Type-Safe Errors for JS & TypeScript
MIT License
3.82k stars 79 forks source link

.match with functions that return Result<., .> #594

Open timvandam opened 1 week ago

timvandam commented 1 week ago

Currently it does not seem possible to handle both Ok and Err at the same time. Note that this behavior is different from .andThen(...).orElse(...) or .orElse(...).andThen(...) as the first of the two chained functions may have its return value handled by the second of the two. As far as I am aware there is no neat way of handling this currently. .match comes close, but returns a Promise rather than ResultAsync when applied to ResultAsync.

I propose adding the following:

declare class ResultAsync<T, E> implements PromiseLike<Result<T, E>> {
  fork<A, B>(ok: (t: T) => Result<A, B> | ResultAsync<A, B>, _err: (e: E) => Result<A, B> | ResultAsync<A, B>): ResultAsync<A, B>
}

I am willing to submit a PR if this idea is accepted

timvandam commented 1 week ago

The pattern I have adopted in the meantime is:

fromSafePromise(x.match(asyncFn1, asyncFn2).then(id)).andThen(id)

Where id is an identity function that serves to 1) flatten the promise returned by the match branch handler (required because match does not expect async functions. a asyncMatch may be handy for this case), and 2) to flatten the Result (required because I return Result inside of the match branch handlers)

paduc commented 1 week ago

Hi @timvandam,

I’m having a hard time understanding what you want to do. Could you provide a more complete example ?

timvandam commented 1 week ago

Hi @timvandam,

I’m having a hard time understanding what you want to do. Could you provide a more complete example ?

What I’m trying to achieve is behavior analogous to Promise.then with 2 arguments: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#onrejected. This is not the same as chaining .then and .catch (and similarly .andThen and .orElse can nit be applied to achieve the same behavior in a simple way)

You provide two functions that handle the ok/err cases and return a (Async)Result

paduc commented 1 week ago

Thanks @timvandam. Reading this also helped me grasp the topic.

Wouldn't the best solution be to mimick Promise.then and accept a second function argument to Result.andThen, instead of adding a new fork ?

timvandam commented 1 week ago

@paduc Yes that would be fine too. I personally would prefer fork because its much clearer in my opinion. And neverthrow does not necessarily follow the Promise naming (e.g. by using orElse rather than something like catch) so I think it would be nicer

macksal commented 1 week ago

I like this because it kind of completes the "toolbox".

For mapping to a result:

For mapping to a value/err:

On the other hand, can you already accomplish the same thing by just returning a ResultAsync from both branches of your match?

timvandam commented 6 days ago

@macksal achieving the behavior of fork by using andThen and orElse is possible, but not a nice pattern, e.g.:

resultAsync
  .andThen(async (val) => {
    if (await condition(val)) {
      return ok(val);
    }

    return err({ passthrough: true, error: new Error('condition not met') });
  })
  .orElse(async (val) => {
    if ('passthrough' in val && val.passthrough) return err(val.err);

    // normal error handling logic here
  })

The difference between fork and a combination of andThen and orElse is also clear by considering the differences for Promise: in .then(.).catch(.), the catch function can capture exceptions thrown in the then function. Similarly, in .catch(.).then(.), handling an error in the catch function will cause the then function to run next. This is clearly different from .then(a, b) because a and b will never both be called. The same difference would apply for fork, andThen, and orElse.

Edit:

I misread your question - you are talking about match. This is not possible as match returns a Promise type rather than a ResultAsync. However you probably could return a Result, wrap the match in fromSafePromise and then flatten the ResultAsync<ResultAsync, never> with .andThen(x => x)

https://github.com/supermacro/neverthrow/blob/ac52282cda18cb0413a990d7e01bed54cbae658b/src/result-async.ts#L186-L188

macksal commented 6 days ago

@timvandam understood. It works normally for sync but not for async. fork seems justifiable to me.

paduc commented 6 days ago

In my opinion, it's valuable to have an API with a clear distinction between the Result/ResultAsync "world" (eg andThen, orElse, map, mapErr, ... all methods with a Result => Result signature) and the Promise world, for which neverthrow offers entries (eg fromPromise and other Promise => Result) and exits (eg unwrap, match and other Result => Promise).

When there is a back and forth between these worlds, it starts to smell like bad code.

I'm all for adding methods like fork that helps us keep our code in the Result/ResultAsync side.