supermacro / neverthrow

Type-Safe Errors for JS & TypeScript
MIT License
4.06k stars 85 forks source link

Allow ok/err/okAsync/errAsync to accept zero arguments when returning void #584

Closed macksal closed 2 weeks ago

macksal commented 2 months ago

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. ok must be passed an argument, even if you explicitly specify the void type. This leads to passing a "dummy" value:

return ok<void>(undefined);

Meanwhile, in Promise-land, it's perfectly acceptable to call Promise.resolve() with no arguments, which returns a Promise<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 to void or a union containing void. See https://github.com/Microsoft/TypeScript/issues/29131

This PR adds additional overloads for ok, err, okAsync and errAsync that separate out the case where the argument type is void. This allows the following to work as expected:

return ok<void>();
return ok();
changeset-bot[bot] commented 2 months ago

🦋 Changeset detected

Latest commit: 7767eabe4dab1f40c037f7b617ae0117c81dd179

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | ---------- | ----- | | neverthrow | Minor |

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

petermakeswebsites commented 1 month ago

Would really appreciate this being implemented!

macksal commented 1 month ago

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.

macksal commented 13 hours ago

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.