ts-pattern icon indicating copy to clipboard operation
ts-pattern copied to clipboard

match does not add .otherwise type to return type

Open alextes opened this issue 2 years ago • 3 comments

Describe the bug Perhaps we can have somewhere in the docs a note on how to handle this pattern, perhaps its already there, perhaps there's a fix possible but I'm noticing the type in the otherwise branch is not added to the return type. Meaning in:

import { match, P } from "ts-pattern";

class CustomErrorA extends Error {}
class CustomErrorB extends Error {}

const failWithError = () => {
  if (Math.random() > 0.5) {
    return new CustomErrorA()
  }

  return new CustomErrorB()
}

const error = failWithError()

export const checkMyType = match(error)
  .with(
    P.instanceOf(CustomErrorA), e => e
  )
  .otherwise((e) => e);

checkMyType is only CustomErrorA, when it should probably be CustomErrorA | CustomErrorB.

Code Sandbox with a minimal reproduction case https://codesandbox.io/s/polished-bash-uimj6b?file=/src/index.ts

Versions

  • TypeScript version: 4.4.4
  • ts-pattern version: 4.0.1
  • environment: Node v17

alextes avatar Apr 09 '22 13:04 alextes

Tried a variation with .exhaustive but also didn't work:

import { match, P } from "ts-pattern";

class CustomErrorA extends Error {}
class CustomErrorB extends Error {}

const failWithError = () => {
  if (Math.random() > 0.5) {
    return new CustomErrorA();
  }

  return new CustomErrorB();
};

const error = failWithError();

export const checkMyType = match(error)
  .with(P.instanceOf(CustomErrorA), (e) => e)
  .with(P._, (e) => e)
  .exhaustive();

Perhaps helpful to also describe what I use this for. Many of my functions return E | A (Really Either<E, A> but we can keep it simple). In other words, an object that either contains an error if something went wrong or a value if all went right. So E is actually CustomErrorA, CustomErrorB etc. In some cases, a custom error like BadResponseError comes from low-level functions that fetch things, but the higher-level function is actually okay with a 403, in the case of this external API meaning "this user is not found". So I write a match, matching on this very narrow BadResponseError turn it into a WeirdApiUserNotFoundError error, and then callers higher up can decide what to do, which is very different from a BadResponseError with a 500 code for example. In this case, ts-pattern matches the custom error perfectly, but narrows the return type all the way to WeirdApiUserNotFoundError and completely disregards the .otherwise or in the example just given even the second .with branch. That means all other errors disappear from the return type.

Any way to handle this scenario with ts-pattern? Thanks for a great library. Pattern matching is sorely missed in JS!

alextes avatar Apr 09 '22 14:04 alextes

Hello!

Since Typescript has structural typing and not nominal typing, typescript will consider that CustomErrorA and CustomErrorB are the same type, and since TS-Pattern deduplicates types to give a cleaner output we loose track of one of the 2 error types. A simple fix is to add a tag property that will discriminate between the two cases in your subclass definition:


class CustomErrorA extends Error {
  tag = "CustomErrorA" as const;
}
class CustomErrorB extends Error {
  tag = "CustomErrorB" as const;
}

const checkMyType = match(error)
  .with(P.instanceOf(CustomErrorA), (e) => e)
  //  .with(P.instanceOf(CustomErrorB), (e) => e)
  // this doesn't type-check!
  .exhaustive();

// type of `checkMyType2` is `CustomErrorA | CustomErrorB`
const checkMyType2 = match(error)
  .with(P.instanceOf(CustomErrorA), (e) => e)
  //  .with(P.instanceOf(CustomErrorB), (e) => e)
  // this doesn't typecheck!
  .otherwise((e) => e);

https://codesandbox.io/s/friendly-framework-ofuf3z?file=/src/index.ts

Note: error instanceof MyCustomError doesn't work in Codesandbox because it doesn't have the transform builtin extends Babel plugin, so this example won't have the correct runtime behavior. See https://github.com/gvergnaud/ts-pattern/issues/68 for more info

gvergnaud avatar Apr 11 '22 08:04 gvergnaud

@gvergnaud you'll have to help me a little here I'm no expert on structural typing but it seems like the issue for me is more that things that are structurally not the same but subclasses collapse to their least specific superclass. Another example to clarify:

import { boolean } from "fp-ts";
import { pipe } from "fp-ts/lib/function";
import { match, P } from "ts-pattern";

class CustomErrorA extends Error {
  public code: number;
  constructor(message: string, code: number) {
    super(message);
    this.code = code;
  }
}

class CustomErrorB extends Error {
  public extraMessage: string;
  constructor(message: string, extraMessage: string) {
    super(message);
    this.extraMessage = extraMessage;
  }
}

class CustomErrorC extends Error {}

const failWithError = () => {
  if (Math.random() > 0.5) {
    return new CustomErrorA("error", 200);
  }

  return new CustomErrorB("things", "went wrong");
};

const error = failWithError();

export const checkMyType = match(error)
  .with(P.instanceOf(CustomErrorA), (e) => new CustomErrorC())
  .otherwise((e) => e);

export const checkMyTypeToo = pipe(
  Math.random() > 0.5,
  boolean.matchW(
    () => new CustomErrorA("a message", 200),
    () => new CustomErrorC()
  )
);

checkMyType becomes CustomErrorC the superclass, when there are two subclasses at play that are structurally different (not sure this counts as structurally different, but I imagine so).

fp-ts provides a lot of extra versions of their functions so boolean.match is the usual function you'd use except in this case where you want it to "widen" the type and use boolean.matchW.

If such a thing goes against ts-pattern and there's no way to do that, I'll just consider adding tags to classes or foregoing the use of ts-pattern where sub- superclasses are involved. It's not the worst considering you get an amazing pattern matching library in all other cases 😄 .

alextes avatar Apr 11 '22 09:04 alextes