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

prevent passing a specific error type for .fromThrowable without providing an error mapping function #583

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

boyeln
Copy link

@boyeln boyeln commented Sep 12, 2024

As discussed in #567 (comment), the current type signature of fromThrowable allows users to pass a specific error value type, without providing an error mapping function that produces the provided error type:

const safeParse = Result.fromThrowable<typeof JSON.parse, number>(JSON.parse);
      // ^ (text: string, reviver?: (this: any, key: string, value: any) => any) => Result<any, number>

This PR tries to solve that by adding some overloads to the function, only allowing users to specify the error type when a matching error mapping function is provided.

Copy link

changeset-bot bot commented Sep 12, 2024

🦋 Changeset detected

Latest commit: 6736079

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

@@ -0,0 +1,5 @@
---
'neverthrow': minor
Copy link
Author

Choose a reason for hiding this comment

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

This might be considered a breaking change, since users that use the function like described in the PR will get red squiggles. However, I would argue that this is a "type bug", and shouldn't be considered a valid use case

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd agree to consider this a bug fix

Comment on lines +25 to +31
/**
* Wraps a function with a try catch, creating a new function with the same
* arguments but returning `Ok` if successful, `Err` if the function throws
*
* @param fn function to wrap with ok on success or err on failure
* @param errorFn when an error is thrown, this will wrap the error result
*/
Copy link
Author

Choose a reason for hiding this comment

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

It's not the most elegant to have to maintain two versions of the documentation, however I'm not aware of any other way of doing it. Please let me know if there's a better way!

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.

3 participants