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

Exhaustive fallback #38

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

Conversation

oguimbal
Copy link
Contributor

@oguimbal oguimbal commented Jul 5, 2021

Hi !

Consider the following dummy case:

const value = fs.readFileSync('some-file', 'utf8') as AorB;

const value = match(value)
    .with('a', () => 'something')
    .with('b', () => 'other thing')
    .exhaustive();

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:

// throw a custom exception
.exhaustive(() => {throw new Error('Your configuration file is corrupted')});

// return a default value
.exhaustive(v => {
     console.warn('The config file is corrupted, found value : ' + v);
     return defaultConfig;
});

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 ?

@m-rutter
Copy link
Contributor

m-rutter commented Oct 19, 2021

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 .otherwise. Trying to use .exhaustive at an I/O boundary and not expecting an error seems kinda nonsensical at least where I'm standing. The universe of possible values at an I/O boundary is infinite.

@oguimbal
Copy link
Contributor Author

oguimbal commented Oct 19, 2021

@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 could use .otherwise(() => <div>Unkown notification type</div>) , but if I do that, I'm losing those lovely typescript messages telling me that I forgot to handle a case (I will not see that a new case has been added to the union if I recompile my project).
  • I could use Joi or equivalent, but it's time consuming & overkill, and IMO, defeats the purpose of having a library that allows us to write nice functional oriented code.

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 switches because of that, and that's such a pitty.

@paul-sachs
Copy link

paul-sachs commented Oct 19, 2021

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 other isn't narrowed by preceding with operations.

@m-rutter
Copy link
Contributor

Well under the original otherwise API it never gave the original value in the first place. It was added because of an ergonomic concern around unbound values coming from function calls.

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
computationally challenging (see some of the pre-3.0 issues and PRs when exhaustive basically was unusable). Using ts-pattern's exhaustive is not a cheap operation, and often times its not possible or desirable to call exhaustive in all cases when dealing with complex types. Making otherwise narrow would force ts-pattern to do a lot more work when conceptually at this point you shouldn't care about the value. Doing the narrowing also would mean I literally could not use ts-pattern for many of my use cases because I would regularly hit the depth limit.

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?

@paul-sachs
Copy link

@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.

@m-rutter
Copy link
Contributor

m-rutter commented Oct 19, 2021

In case you are interested in the recent history of exhaustive:

#16

@oguimbal
Copy link
Contributor Author

your codegen and I/O layer is allowing invalid input to reach your leaves of the application.

Well, yes and no.
It does validate the data received, but I dont want it to be so strict about checking union types... If it were, in the example I mentioned above, that would mean that my "notifications" screen would break each time the API author adds a new notification type. Which would be kind of supid, if you ask me. I want the types to break. Not my app.

The real world is not about monads, IO boundaries, functional purity, and all that stuff.
Those are great theoretical tools which are the building blocks of great languages.
But I feel that being too dogmatic about those killed the very languages that were supposed to leverage them.
Or at least relegated them to being some nerdy reasearch languages. Which Typescript is not. It gives some room to pragmatism.

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.
Now, if you tell me that the way I'm requesting my data is not fault-intolerant enough to use ts-pattern... well, that looks a bit like those guys who think that there is no room for loose coupling and idiosyncraties in programming. I've had that argument countless times with beardy purists in my past 15 years as a developper, and that doesnt speak to me.

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".
If you are trying to explain to me that fault tolerance isnt a use-case, well... what can I say ? 🤷

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 😬

@m-rutter
Copy link
Contributor

m-rutter commented Oct 20, 2021

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 unknown. Perhaps as another method. Call it .nonExhaustive. But I don't think any kind of exhaustive pattern matching exists in any language behaving like that.

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.

@jjhiggz
Copy link

jjhiggz commented Jan 28, 2024

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 safeExhaustive function because it's a bit more clear what the intent of that function is, in other words when I need that safety I want to make sure I have it and don't miss it because I forgot to plug in an argument to exhaustive. I do really like your approach too though, it feels very concise. I also supplied an optional second argument for handling that error so you could plug it into your own logging utilities.

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.

@gvergnaud
Copy link
Owner

gvergnaud commented May 23, 2024

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 exhaustive function type, but this is likely to land in the next minor release

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.

None yet

5 participants