Skip to content

added a safe exhaustiveness check#219

Closed
jjhiggz wants to merge 1 commit intogvergnaud:mainfrom
jjhiggz:safe-exhaustive
Closed

added a safe exhaustiveness check#219
jjhiggz wants to merge 1 commit intogvergnaud:mainfrom
jjhiggz:safe-exhaustive

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')

@gvergnaud
Copy link
Owner

#253 was merged, adding an optional fallback function to .exhaustive which covers the same use case as .safeExhaustive proposed here. Closing

@gvergnaud gvergnaud closed this Mar 29, 2025
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