-
Notifications
You must be signed in to change notification settings - Fork 4
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
Methods for option #49
Conversation
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
Co-authored-by: Leto Rodríguez <leto4100@gmail.com> Co-authored-by: Borja García <borjagarcia.dev@gmail.com>
I don't know how the ofSome method should behave with null/undefined, what do you think the behavior should be? const nullishSome = Option.ofSome(null) // ???
const undefinedSome = Option.ofSome(undefined) // ??? |
mmm maybe we could create this Type:
So, static ofSome<T>(value: NotNullable<T>): Some<T> {
return new Some(value);
} In this way, the TS compiler raises an error if you try to pass an |
From my point of view, this cannot be tested with Vitest or Jest with a test case. I mean, the test actually is the static analysis of code, if this finds an error it will notify the developer to fix it (as a red test case). In addition, You can try to run a build, this should fail. At the end, we can pass anything in runtime a function that allows statically a specific type as input parameter 😟 Could you verify my guess, running the |
Yeah this worked well! 😄 In the next commit the code will be ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like to see more people involved in the library.
I really appreciate your contribution @Sstark97 ❤️
Same, thanks for this contribution @Sstark97! ❤️ |
@Sstark97 if you consider PR is ready, you can remove Draft state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job @Sstark97!
I put some comments to solve some doubts on my side
… be used in any project
…-for-option # Conflicts: # src/try/try.ts
The merge-base changed after approval.
What type of PR is this? (check all applicable)
Description
In this PR I have added two methods for the Option cutie:
Added/updated tests?
have not been included
What gif best describes this PR or how it makes you feel?