-
Notifications
You must be signed in to change notification settings - Fork 105
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
OrElse callback should be able to change Ok type #484
Conversation
@supermacro Any updates on this? Would be really great to have this behavior for |
+1 |
🦋 Changeset detectedLatest commit: 378b3e5 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 |
It is possible to make this PR not a breaking change - orElse<U, A>(f: (e: E) => Result<U, A> | ResultAsync<U, A>): ResultAsync<U | T, A>
+ orElse<A, U = T>(f: (e: E) => Result<U, A> | ResultAsync<U, A>): ResultAsync<U | T, A> That way anyone who has already explicitly annotated the type arguments The downside is of course that this is really awkward to use, especially as it swaps the order or the ok value type and the err value type. I would guess that would be super error prone and surprising to users. |
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.
LGTM!
@supermacro if the PR is merged, changesets will create the release PR of v8.0.0.
I'll leave the timing of the merge up to you.
Unfortunately this would be a breaking change, as anyone who explicitly annotated the types like
orElse<A>(foo)
will need to update their code toorElse<U, A>(foo)