-
-
Notifications
You must be signed in to change notification settings - Fork 138
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
Exhaustive fallback #38
base: main
Are you sure you want to change the base?
Conversation
af08eab
to
2a9ce89
Compare
I kinda think that data at I/O boundaries is exactly when you would either validate your data (joi, yup, io-ts, zod, runtypes, etc...) and/or use |
@m-rutter Yup, that's a bit like data validation, but using Joi or equivalent might often be overkill. The example I used in this PR is a bit misleading, my bad (Joi would be a better fit indeed in that case). But consider this scenario (which is an actual problem I encounter quite often): I have a GraphQL endpoint (which you can consider to be strongly typed HTTP endpoint, if you dont know about GraphQL) that returns something like: type Notif = { type: 'create', /** create data */ } | { type: 'delete', /** delete data */ };
const notifications: Notif[] = // ... graphql network call here I know for a fact that the returned data will always have the expect shape. That's how GraphQL works (the Typescript types are generated from the endpoint), and I dont want to spend hours configuring a Joi schema to validate to data I get. I'm using ts-pattern to render this in react, as an exhaustive matching because I want the compiler to check that everything is handled: match(notifications)
.with({type:'create'}, d => <Something />)
.with({type:'delete'}, d => <Something />)
.exhaustive() Now, suppose that the person who maintains the GraphQL endpoint adds a new notification type type Notif = { type: 'create', /** create data */ } | { type: 'delete', /** update data */ } | {type: 'update' }; ... that breaks my rendering in production with a nasty error.
I'd just love to have a simple fallback that does not throw an error. In my case, I have to get back to plain old |
Speaking as just an observer, i also like to use switch statements to catch all possible cases and like that I can rely on doing something like: function assertUnreachable(_x: never): never {
throw new Error('Unknown component type');
}
switch(input) {
case 'string1':
return 'x';
case 'string2':
return 'y';
default:
throw assertUnreachable(input);
} If someone adds a new type to input, this will error out on build, making it easy to catch. When I saw this, i assumed we could do that in user land with match(input)
.with('string1', () => 'x')
.with('string2', () => 'y')
.otherwise((other) => throw assertUnreachable(other)); But i noticed the type of |
Well under the original There are two disconnects here: sourcse of inspiration and performance concerns. The library is trying to emulate pattern matching as you might find in ML languages like Ocaml, Rust, Haskell, etc... which all do exhaustive matching on ADTs. "Otherwise" is when you don't care about the value at all because you have already matched on the cases where you care about the value, or you are trying to stay forward compatible for new variants. It's very common in those languages to have non exhaustive otherwises to handle API changes when you cannot predict what an upstream API change might add (see the nonexhaustve enum pattern in rust). Performance wise a big difference in typescript is we doesn't have a built in concept of a Sum type (we emulate them with discriminated unions) and it's structurally typed which makes exhaustive pattern matching very Re graphql codegen: Seems unfortunate that you have a typed API schema and codegen, but your codegen and I/O layer is allowing invalid input to reach your leaves of the application. Another thing we don't have that most other statically types languages do is easy runtime validation at the I/O boundaries derived from our types. I guess I wonder if your graphql codegen is not already validating responses given it statically knows what is valid input? |
@m-rutter that makes sense, regarding performance concerns. Sticking to a switch statement for those times when I want those build failures may be just what I need to do for now, at least until Typescript has better Sum type representation. |
In case you are interested in the recent history of exhaustive: |
Well, yes and no. The real world is not about monads, IO boundaries, functional purity, and all that stuff. I use ts-pattern a bit. But I'm using it with Typescript. Not Haskell, nor Elm. Meaning that I am used to having SOME liberty of interpretation about what my data might be. And I like to leverage that to build resilient apps. So the only thing I said with this PR is "look, there is a real use case that prevents me from using ts-pattern, and which would make my life greater as a developper". Now, I really dont mind this PR to be closed, please do exactly whatever you feel right for this library. That was just a suggestion 😬 |
Your use case is completely valid in terms of wanting to be forwards compatible with upstream API changes. I'm not a maintainer so I don't get much say. I personally wouldn't be surprised if some accomodation is made by the maintainer that gives you opt in exhaustive checks combined with the behaviour of otherwise, but where the passthrough value is My feelings is that this is expressly the point of otherwise in trying to be forwards compatible with API additions from another source. This isn't about some expression of ideological purity, it's literally the point of otherwise. I would personally find it confusing if exhaustive had a fallback value. If my match doesn't cover all cases when I explicitly expect it to be exhaustive I want it to error and fail fast because my assumptions have been invalidated. If I want to be open in terms of forward compatible API changes outside my control (a very reasonable thing to want, and your notification ui example is a good example) I use otherwise. If you want fault tolerance that is called data validation and error handling. The library is not about error handling or data validation. If you are concerned about error handling wrap it in a try/catch. |
55f96e3
to
41c02f4
Compare
d86f4e2
to
6a0a9e0
Compare
Not realizing that this PR was up, I went ahead and built something similar. Curious what y'all think https://github.com/gvergnaud/ts-pattern/pull/219/files For me I personally like the idea of a This has definitely come across as a really really useful feature for me though so I'm extremely happy to see the support for it. |
I know it's been 3 years, but after being initially opposed to this feature, I now think this can make a lot of sense in some contexts. The reason why I didn't consider this a priority was that it could be emulated by wrapping an exhaustive match expression into a try catch, but I think we should probably embrace the unsoundness of TypeScript's type system and make catching unexpected values possible anyway. I also like the idea of introducing this feature as an overload, instead of making it a new method. I pulled out the bits of this PR which are still relevant in v5 in this PR: #253 I need to check that I can preserve the developer experience and the quality of error messages with an overloaded |
Hi !
Consider the following dummy case:
Currently,
.exhaustive()
checks that you handled every possible type, but does not provide any mechanism other than throwing an exception if the actual runtime value does not conform to this type. Nor any way to customize the thrown exception.Those are things i'd like to be able to do:
The main difference here compared to
.otherwise()
it that it still checks that every valid case declared in types has been covered, while providing the graceful fallback that.otherwise()
provides.(I know I could count on libs like Joi to handle data validation, but that I think that would be largely overkill and redundant)
What do you think ?