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

orElse should accept new Ok types #411

Open
ghost opened this issue Aug 26, 2022 · 1 comment · May be fixed by #484
Open

orElse should accept new Ok types #411

ghost opened this issue Aug 26, 2022 · 1 comment · May be fixed by #484

Comments

@ghost
Copy link

ghost commented Aug 26, 2022

Current signature:

class Result<T, E> {
  orElse<A>(
    callback: (error: E) => Result<T, A>
  ): Result<T, A> { ... }
}

Suggested signature:

class Result<T, E> {
  orElse<T2, E2>(
    callback: (error: E) => Result<T2, E2>
  ): Result<T | T2, E2> { ... }
}

Example scenario:

const getAnimal = (name: string): ResultAsync<Dog | Cat, Error> =>
  getDog(name)
    .orElse((err) => err instanceof NotFoundError ? getCat(name) : errAsync(err))

Current workaround:

const getAnimal = (name: string): ResultAsync<Dog | Cat, Error> =>
  (getDog(name) as ResultAsync<Dog | Cat, Error>)
    .orElse((err) => err instanceof NotFoundError ? getCat(name) : errAsync(err))
@rudolfbyker
Copy link

Here is a generalized example of a very common use case for me:

function foo(): ResultAsync<string, Error> {
  return okAsync("string");
}

class FooError extends Error {}

class BarError extends Error {}

function test1() {
  return foo().orElse((e) => {
    if (e instanceof BarError) {
      // Handle the error. Change the OK type.
      return okAsync(null);
    }

    if (e instanceof FooError) {
      // Handle the error. Keep the OK type.
      return okAsync("foo");
    }

    // Let the error pass through.
    return errAsync(e);
  });
}

function test2() {
  const intermediateVariable: ResultAsync<string | null, Error> = foo();

  return intermediateVariable.orElse((e) => {
    if (e instanceof BarError) {
      // Handle the error. Change the OK type.
      return okAsync(null);
    }

    if (e instanceof FooError) {
      // Handle the error. Keep the OK type.
      return okAsync("foo");
    }

    // Let the error pass through.
    return errAsync(e);
  });
}

test1 gives Type 'null' is not assignable to type 'string'., but test2 works. The only difference is an intermediate variable, where I have explicitly stated that I want to be able to change the OK type later, from string to string|null. This makes the code harder to understand, maybe even misleading, because intermediateVariable can't actually give you null.

Another workaround is to write a helper function that allows me to work with ResultAsync objects in the synchronous domain:

export function manipulateResultAsync<T1, E1, T2, E2>(
  ra: ResultAsync<T1, E1>,
  callback: (r: Result<T1, E1>) => Result<T2, E2>
): ResultAsync<T2, E2> {
  return new ResultAsync((async () => callback(await ra))());
}

Then I can do this:

function test3() {
  return manipulateResultAsync(foo(), (r) => {
    if (r.isOk()) {
      return r;
    }

    const e = r.error;
    if (e instanceof BarError) {
      // Handle the error. Change the OK type.
      return ok(null);
    }

    if (e instanceof FooError) {
      // Handle the error. Keep the OK type.
      return ok("foo");
    }

    // Let the error pass through.
    return err(e);
  });
}

None of this is elegant.

@braxtonhall braxtonhall linked a pull request Jun 29, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants