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
feat: add a "tap"-method to perform side effects #456
base: master
Are you sure you want to change the base?
Conversation
e418170
to
5e1e2cb
Compare
5e1e2cb
to
372f1d9
Compare
Hey, @supermacro what do you think about this? |
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.
TODOs
- I had a question about handling failed promises for
.tap
onResultAsync
. - Update README to include docs on
.tap
for bothResult
andResultAsync
return new Err<T, E>(res.error) | ||
} | ||
|
||
await f(res.value) |
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.
What if the promise fails? Shouldn't this be wrapped in a try / catch? ResultAsync.fromPromise
could also be used here, which requires a handler function in the event the promise fails.
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.
Good question @supermacro , I'm honestly not sure. Maybe that's a good indicator that I should add a test that covers that(?)
I pretty much copied the map
implementation. The only difference is that instead of returning the result of the passed function, I execute the function and then return the previous value.
If I add a try-catch to the tap function, shouldn't I also add one to the map
and mapErr
functions? 🤔
@@ -709,7 +762,7 @@ describe('ResultAsync', () => { | |||
expect(mapAsyncFn).toHaveBeenCalledTimes(1) | |||
}) | |||
|
|||
it('Skips an error', async () => { | |||
it('Skips an error when mapping an asynchronous value', async () => { |
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.
👍
Hi,
created this to run side effects like logging.
Name "tap" is inspired by rxjs tap