Skip to content
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

andTee() / andThrough() to handle side-effects a bit easier (from issue #445) #467

Merged
merged 5 commits into from
Aug 28, 2024

Conversation

untidy-hair
Copy link
Contributor

@untidy-hair untidy-hair commented Apr 3, 2023

closes #445

Hi @supermacro!
I opened an issue with a suggestion about 2.5 months ago (Jan 17) but I think you have been very busy and I thought maybe a real code would save your time :)

I got a feedback comment in the original issue thread and I ended up having these two methods.

  1. andTee()
andTee(f: (t: T) => unknown): ResultAsync<T, E>
// Usage: validate() below returns ResultAsync
validate(userInfoInput).andThen(buildUser).andTee(logEvent).andThen(notifyUserRegistration)....
  1. andThrough()
andThrough<F>(f: (t: T) => ResultAsync<unknown, F>): ResultAsync<T, E | F>
// Usage: validate() below returns ResultAsync
validate(userInfoInput).andThen(buildUser).andThrough(persistUser).andThen(notifyUserRegistration)....

Both return the original input okAsync<T> upon success, but in case of error, andTee() ignores the error whereas andThrough() returns the new error errAsync<F>. The former is suitable for such as logging when you don't want to stop the main logic upon trivial side-effect failure and the latter is suitable when you want to stop the main logic upon a critical side-effect failure such as DB insertion error.

Origianlly I was going to use the name andTee for no.2 above that passes error value in case of an error but considering the original meaning of "tee", even the error should not come back to the main "pipe" so I used the name for no.1 above instead. (But as I wrote in the original issue, there could be better names.)

And was also written in the original thread, this can be done with the current APIs but these two new methods will make it possible to write and understand each step of the logics more vividly (IMO) and I personally have cases I want to rewrite code with andThrough() for DB persistence as I explained here :)

Thanks in advance!

@brndt
Copy link

brndt commented May 29, 2023

@supermacro can you take a look on this please? It's quite useful feature to avoid boilerplate

@brndt
Copy link

brndt commented Sep 12, 2023

@supermacro bump :)

@supermacro
Copy link
Owner

#496

@untidy-hair
Copy link
Contributor Author

untidy-hair commented Dec 6, 2023

Hi @supermacro , have you had a chance to look at this PR? I'd appreciate your review! Thank you!

Copy link

changeset-bot bot commented Aug 25, 2024

⚠️ No Changeset found

Latest commit: 4b9d2fd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Collaborator

@m-shaka m-shaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution
Can you add docs about new features to README?

tests/index.test.ts Outdated Show resolved Hide resolved
tests/typecheck-tests.ts Outdated Show resolved Hide resolved
src/result.ts Outdated Show resolved Hide resolved
src/result.ts Outdated Show resolved Hide resolved
@untidy-hair
Copy link
Contributor Author

@m-shaka thank you for the comment, I will get back to you after making updates.

@m-shaka
Copy link
Collaborator

m-shaka commented Aug 26, 2024

@supermacro Hey Gio, I think andTee is a nice name, but what do you think of the name andThrough? Do you have any other ideas?

@untidy-hair
Copy link
Contributor Author

Hi @m-shaka and @supermacro , I updated the PR based on the suggestions as well as README. Thanks

@supermacro
Copy link
Owner

Both names are ok with me! Don't think it's fair for me to come in so late and opine strongly here. Great job everyone .. and thanks for your patience :)

@supermacro supermacro merged commit e72e9cd into supermacro:master Aug 28, 2024
8 checks passed
alexandresoro pushed a commit to alexandresoro/ouca that referenced this pull request Sep 15, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [neverthrow](https://github.com/supermacro/neverthrow) | dependencies | minor | [`7.0.1` -> `7.2.0`](https://renovatebot.com/diffs/npm/neverthrow/7.0.1/7.2.0) |

---

### Release Notes

<details>
<summary>supermacro/neverthrow (neverthrow)</summary>

### [`v7.2.0`](https://github.com/supermacro/neverthrow/blob/HEAD/CHANGELOG.md#720)

[Compare Source](supermacro/neverthrow@v7.1.0...v7.2.0)

##### Minor Changes

-   [#&#8203;562](supermacro/neverthrow#562) [`547352f`](supermacro/neverthrow@547352f) Thanks [@&#8203;sharno](https://github.com/sharno)! - change the return type of `safeTry` to be `ResultAsync<T, E>` instead of `Promise<Result<T, E>>` for better composability

### [`v7.1.0`](https://github.com/supermacro/neverthrow/blob/HEAD/CHANGELOG.md#710)

[Compare Source](supermacro/neverthrow@v7.0.1...v7.1.0)

##### Minor Changes

-   [#&#8203;467](supermacro/neverthrow#467) [`4b9d2fd`](supermacro/neverthrow@4b9d2fd) Thanks [@&#8203;untidy-hair
    ](https://github.com/untidy-hair)! - feat: add `andTee` and `andThrough` to handle side-effect

##### Patch Changes

-   [#&#8203;483](supermacro/neverthrow#483) [`96f7f66`](supermacro/neverthrow@96f7f66) Thanks [@&#8203;braxtonhall](https://github.com/braxtonhall)! - Fix `combineWithAllErrors` types

-   [#&#8203;563](supermacro/neverthrow#563) [`eadf50c`](supermacro/neverthrow@eadf50c) Thanks [@&#8203;mattpocock](https://github.com/mattpocock)! - Made err() infer strings narrowly for easier error tagging.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41Ny4zIiwidXBkYXRlZEluVmVyIjoiMzguNzIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/70
Reviewed-by: Alexandre Soro <[email protected]>
Co-authored-by: renovate <[email protected]>
Co-committed-by: renovate <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add andTee() implementation
5 participants