traverse1984 / oxide.ts

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

Unexpected Type Strictness with `unwrap_or` #2

Closed alexlafroscia closed 2 years ago

alexlafroscia commented 2 years ago

While checking out this library by trying to convert usage of ts-results to this package instead, I ran into an error with unwrap_or. The following code does not compile, though I would expect it should:

import { type Option } from 'oxide.ts';

type Name = string | undefined;

interface User {
  name: Name;
}

declare function getName(person: User): Option<string>;

const name: Name = getName({ name: 'Alex' }).unwrapOr(undefined);

Argument of type 'undefined' is not assignable to parameter of type 'string'.

TypeScript Playground link

It seems odd to me that unwrap_or would demand it's passed a value that matches the type of the Option that is being unwrapped. I can understand that maybe the types should normally be the same, but not that this library has a strong opinion on the matter.

If this was the code instead, I would expect an error to occur because undefined doesn't match the types expected by Name, but I would expect that the assignment itself would be the type error, not the usage of unwrap_or

import { type Option } from 'oxide.ts';

- type Name = string | undefined;
+ type Name = string;

interface User {
  name: Name;
}

declare function getName(person: User): Option<string>;

const name: Name = getName({ name: 'Alex' }).unwrapOr(undefined);

All that to say: I think that it would make sense that unwrap_or can accept any value, and that the return type of unwrap_or becomes the union of the Option's type and the unwrap_or argument's type. If they're the same, great! If they're not the same, that should be OK too.

traverse1984 commented 2 years ago

Thanks for taking a look. This behaviour is as intended to match the Rust implementation.

It's worth noting that T | undefined is similar to Option<T>. That might fit better here.

If you definitely want this behaviour, you could technically use unwrapUnchecked() for the Option type, but it's not an idiomatic solution. You could also:

const p1 = getName({ name: "Alex" });
const name: Name = p1.isSome() ? p1.unwrap() : undefined;

Hopefully this makes sense and you can see how this scenario evades the purpose of the Option. Once you have the Name type you're back to T | undefined and will probably need another assertion later on, so holding T as an Option never actually helped.