Allow ok/err/okAsync/errAsync to accept zero arguments when returning void #584
Conversation
🦋 Changeset detectedLatest commit: 7767eab The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Would really appreciate this being implemented! |
Please help by providing a code review and hopefully we can get attention from the maintainers @supermacro @m-shaka. I'm using this in production and have considered forking, but this is already a fairly niche library and I hope we could maintain some kind of central community. |
|
|
||
| export function err<T = never, E extends string = string>(err: E): Err<T, E> | ||
| export function err<T = never, E = unknown>(err: E): Err<T, E> | ||
| export function err<T = never, E extends void = void>(err: void): Err<T, void> |
There was a problem hiding this comment.
Is there really any valid use-case for void errors?
There was a problem hiding this comment.
I've never used one personally. Perhaps someone is using it as a generic "failure" while making a quick and dirty prototype.
I included the err overload for completeness, not for any specific use case.
I think it would be more surprising for the consumer if ok/err didn't work the same way. Do you see any downside to this?
There was a problem hiding this comment.
In my humble opinion, this is a case of YAGNI.
It feels wrong to be able to return an error which is void.
There was a problem hiding this comment.
My 2c...
I don't disagree with you @Papooch . I don't really see a use case for it either. Doesn't mean there isn't one.
However, I will say I think there's an overYAGNIation in libraries in general. So many times I've come across a feature I've wanted and it hasn't been implemented because of YAGNI. No human being can think of all the possible use cases, and in this case, it's not like adding that overload has any drawbacks.
At the end of the day though, I don't really mind either way - I'd just like to see the void Ok implemented! So whatever makes everyone else happy to expedite that process is fine by me! :)
There was a problem hiding this comment.
Imo say that symmetry + having lower friction with prototypes are good enough reasons to add it, and that there isn't really a cost to implementing it or maintaining it, so YAGNI doesn't really apply. If anything I'd guess that the lack of symmetry introduces an edge case that will make doing anything on both an Ok value and an Err value more convoluted.
There was a problem hiding this comment.
That's ok for me.
It looks like that Rust's Err accepts Unit type
There was a problem hiding this comment.
Thank you @macksal !
@supermacro will merge the PR if it's ok for him
|
|
||
| export function err<T = never, E extends string = string>(err: E): Err<T, E> | ||
| export function err<T = never, E = unknown>(err: E): Err<T, E> | ||
| export function err<T = never, E extends void = void>(err: void): Err<T, void> |
There was a problem hiding this comment.
That's ok for me.
It looks like that Rust's Err accepts Unit type
|
Thank you @supermacro @m-shaka for merging. Would you consider reviewing another PR of mine, #588? It is adding a commonly requested feature including test coverage. I will appreciate any comment on whether this feature could be accepted. |
When implementing a function returning a result, it's often that I need to construct an
Ok<void>object. However, it's not so ergonomic to do this.okmust be passed an argument, even if you explicitly specify thevoidtype. This leads to passing a "dummy" value:Meanwhile, in Promise-land, it's perfectly acceptable to call
Promise.resolve()with no arguments, which returns aPromise<void>.Interestingly, TS allows a parameter to be omitted if it has type
void. However, this does not work if the type is generic and has been expanded tovoidor a union containingvoid. See microsoft/TypeScript#29131This PR adds additional overloads for
ok,err,okAsyncanderrAsyncthat separate out the case where the argument type isvoid. This allows the following to work as expected: