traverse1984 / oxide.ts

Rust's Option<T> and Result<T, E>, implemented for TypeScript.
MIT License
517 stars 19 forks source link

Chaining methods don't allow varying secondary type parameter #19

Open Alexander-Prime opened 1 year ago

Alexander-Prime commented 1 year ago

The Option- and Result-chaining methods (and, andThen, or, and orElse) don't allow the type params that they aren't mapping to differ from the type of this. That is, Result<T, E>.andThen takes (usually infers) a type parameter that will replace T, but there isn't a way to add a new error type to E. This makes it difficult to chain Results unless they all fail with the lowest common denominator (usually a basic Error).

For a realistic example, if I want to fetch some resource and then parse it with Zod, TS complains about my error types being mismatched:

// Ignoring impl and async concerns...
declare function safeFetch(url: string): Result<string, HTTPError>;
declare function safeParse(data: string): Result<unknown, ZodError>;

safeFetch("https://example.com").andThen(safeParse);
//                                       ~~~~~~~~~
// Type 'Result<unknown, ZodError<any>>' is not assignable to type
// 'Result<unknown, HTTPError>'.

The same problem exists the other way around for or and orElse. If I say Some("a").or(Some(1)), instead of the Option<string | number> I expect, I get a TS error that boils down to Type 'string' is not assignable to type 'number'.

This should be fixable with a second type param on these method signatures:

- andThen<U>(this: Result<T, E>, f: (val: T) => Result<U, E>): Result<U, E>;
+ andThen<U, F>(this: Result<T, E>, f: (val: T) => Result<U, F>): Result<U, E | F>;
traverse1984 commented 1 year ago

Hi,

I'm incredibly busy at the moment and sadly working on improvements has ended up on the back burner.

In this case, the issue you raise exists because in Rust you wouldn't be able to have a string | number type. The type signatures for the core functions are copied from Rust.

I am reluctant to change them - this library promises Rust's Option and Result types. There are other Result and FP libraries which may serve you better if this is what you need.

Always keen to hear other opinions though.

Alexander-Prime commented 1 year ago

Yeah, I realized not long after submitting this that it may have been a deliberate choice.

For what it's worth, I still think this would be a win for ergonomics. The extra plumbing to make fail().andThen(failDifferently) work is a bit noisy:

// Either this, with a union like `SomeError | OtherError`
(fail() as Result<Something, MyErrorUnion>)
  .andThen(failDifferently);

// Or this, mimicking Rust's pattern of varying error types via enum
fail()
  .mapErr(wrapError)
  .andThen(x => failDifferently(x).mapErr(wrapError));

But frankly, neither of these is a showstopper for me, and there's probably better patterns I'm missing.

If other opinions sway you and you decide to support this after all, I'm happy to open a PR some weekend or another. Otherwise I don't mind closing this.

traverse1984 commented 1 year ago

I was thinking about this today. I think I'm decided that I'm not going to expand the type signatures at this time, but maybe this pattern helps:

class MyError extends Error {}
class MyOtherError extends Error {}

function fail(): Result<number, Error> {}
function failDifferent(): Result<number, MyError> {}
function failOther(): Result<number, MyOtherError> {}

fail().andThen(failDifferent).andThen(failOther);

Now, I grant that you don't retain complete type information on the possible error types.... even if you had that, you'd still have to check the error type if you wanted to perform different actions depending on the error.

crimx commented 11 months ago

Bumped into this issue too. I think it makes sense that oxide sticks to the Rust type. I made another one @wopjs/tsur that is more TypeScript flavored as complement.