Refactor Result type #683
Merged
Refactor Result type #683
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ShanaryS
reviewed
May 13, 2024
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| mapErr<R>(mapper: (u: U) => R): Result<T, R> { | ||
| return this as unknown as Result<T, R>; | ||
| mapErr(): OkResult<T> { |
There was a problem hiding this comment.
Is the return type intentional? I'm not familiar with JS or TS internals so I can't comment much.
There was a problem hiding this comment.
yeah - the idea behind mapErr is that it's a no-op if the result is an OkResult, but it will do the transformation supplied if the result is an ErrResult.
mmgoodnow
force-pushed
the
mg-refactor-result-class
branch
from
1f56148
to
2a0f871
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current Result type is okay, but it doesn't actually leverage Typescript's ability to catch bugs at build time. This PR does a few things:
OkResult | ErrResultunwrapOrThrow(err?: Error)has been separated into two methods:unwrap()this method ONLY exists on OkResult. In order to call it, you need to useisOkorisErrto do type narrowing so that TypeScript knows the method exists.unwrapOrThrow(err: Error)This is a convenience method to throw an error if the result is an ErrResult. It is like the old version ofunwrapOrThrow, except the argument is now required. This exists on BOTH OkResult and ErrResult.unwrapOrThrow()with no arguments to useunwrap(), and kept calls ofunwrapOrThrow(new Error('error message'))with one argument to useunwrapOrThrow().unwrapErr()this method ONLY exists on ErrResult. In order to call it, you need to useisOkorisErrto do type narrowing so that TypeScript knows the method exists.isOkandisErrare now type predicates which means they will do type narrowing. Once you callresult.isOk(), typescript "grants you access" to calling theunwrap()method.isOkfunction (not an instance method on either of the result classes) because TypeScript doesn't understand when you.filter(r => r.isOk()). It understandsfilter(isOk)though.