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

added a safe exhaustiveness check #219

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jjhiggz
Copy link

@jjhiggz jjhiggz commented Jan 27, 2024

I haven't done much open source, but I thought this might be a really useful function for me personally.

Here is a link to the issue that I opened

@MatanYadaev
Copy link

@gvergnaud What's your opinion about this feature? We need a way to catch both compile-time and runtime errors. Currently exhaustive and otherwise provide just one of the errors (compile-time and runtime, respectively)

@gvergnaud
Copy link
Owner

I'm still considering it, but I'm not sure what's the best option in terms of API complexity / typesafety yet. In the meantime, this work around doesn't seem too bad:

try {
  return match(...).with(...).exhaustive()
} catch (e) {
  return defaultValue
}

@MatanYadaev
Copy link

MatanYadaev commented Apr 1, 2024

@gvergnaud it looks great imo. Are you ok with the breaking changes? Don't you prefer a backward-compatible change?

EDIT: I wasn't aware that .exhaustive() throws an error currently. I thought you suggested it as a PR. I dived into the code and found it.
Maybe the solution is to mention it in the docs because I think all related issues aren't aware of that. At least I wasn't aware.
And a suggestion, maybe it should throw a custom error, instead of a generic Error, so it will be easy to catch. (for example error instanceof PatternMatchingError instead of error.message.includes('Pattern matching error')

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