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

feat(option): better return type for Option constructors #1834

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JalilArfaoui
Copy link

This makes some() constructor return a Some<A> and none return a None, more specific than actual Option<A>.

This enables this kind of code to work :

const updateObjectOrCreate = (
  someObject: Option<SomeType>,
  transformer: (session: SomeType) => SomeType
): option.Some<SomeType> =>
  pipe(
    someObject,
    option.getOrElse(
      constant(
        makeObject(someSaneDefaults)
      )
    ),
    transformer,
    option.some
  );

without adding à as option.Some<Session>

@samhh
Copy link
Contributor

samhh commented Feb 14, 2023

Returning specific sum members is a code smell IMO.

@JalilArfaoui
Copy link
Author

Why is that @samhh ? I was under the idea that we want the most precise types we can have, don’t we ?

@samhh
Copy link
Contributor

samhh commented Feb 14, 2023

I've yet to come across a scenario with sum types where it's not a code smell because of an issue with code structure somewhere else. Just because we can do something doesn't mean we should.

Less esoterically this might present an issue in some spots where currently widening isn't necessary. I know this was a consideration when designing sum-types though I don't have a specific example to hand.

@asjir
Copy link

asjir commented Mar 20, 2024

I'm afraid this might break with typescript 5.5
The reason being: define arr=T[] then typescript gives arr[i]: T even though it can be T | undefined when it's out of bounds, then the type predicate inference might give you:
O.fromNullable(arr[i]): Some<T> even though it's None if it's out of bounds

Note:
O.fromNullable has an explicit return type, so it's safe, the point is that if I had an analogous function without it in my code it could potentially break it

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