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

After P.nullish match, value can still be null

Open jmagaram opened this issue 1 year ago • 5 comments

Describe the bug Awesome library! See the code below. I expect that in the final match condition email can't be null anymore but TypeScript thinks it is still possible. So I need to do the safe string?.trim() operator. This works fine with regular TypeScript expression.

Code Sandbox with a minimal reproduction case https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAbziAhjAxgCwDRwApwC+cAZlBCHAOQwDOAtGGjAKZQB2VAUFzAJ5gW+NrQjs4AXkRc4cFqmAAbAFxxaMKMHYBzOAB847AK6LFAbi6ELXdGPVxtLGAFEFigIyS4ACjCq8ImIAlJIAfDLIaFi+QRGyAHQA7sAwmN5I8ihK-vHGpsC0mES43iESoXBG7AAmLCRaLNWxsgnJqd548SjsfCXAZRXA8ZlK8RqgpSEA9FNyUORQcXDDAB6YKEbqwABuLKXWtuz2ji5uAExevv6B7AMRYMNukhJSeYpwAPyVNXUN1XCqB4jRRjTQgfZcIA

type Person = {
  email: string | null;
};

const getEmail1 = (p: Person) =>
  match(p)
    .with({ email: P.nullish }, () => undefined)
    .with(P.any, (i) => i.email.trim()) // error
    .exhaustive();

const getEmail2 = (p: Person) =>
  p.email === null ? undefined : p.email.trim();

Versions

  • TypeScript version: 4.9.5
  • ts-pattern version: 4.1.4
  • environment: browser + version / node version / deno version

jmagaram avatar Feb 11 '23 05:02 jmagaram

Hey! That's an unfortunate, but expected behavior of the current version. I don't "deeply" exclude nested union types until you actually call .exhaustive() for performance reasons. More detail here https://github.com/gvergnaud/ts-pattern/releases/tag/v4.1.2

gvergnaud avatar Feb 11 '23 13:02 gvergnaud

I vaguely remember a Typescript bug in the past couple years about this where fixed it and enabled better narrowing. Is there anything I can do to work around this limitation? Like rewriting my matches hierarchically with otherwise clauses? Or do I just need to repeat some conditions I know to be true? As far as performance are you talking compile time performance? I wonder when compile time performance really becomes an issue and breaks auto-complete in the editor.

jmagaram avatar Feb 11 '23 16:02 jmagaram

This still doesn't work. According to that link you sent .otherwise's input also inherit narrowing. I'm still not understanding the limitations or how to work around them.

type Person = {
  email: string | null;
};

const getEmail1 = (p: Person) =>
  match(p)
    .with({ email: P.nullish }, () => undefined)
    .otherwise(i=>i.email.trim()) // error email is possibly null

jmagaram avatar Feb 11 '23 19:02 jmagaram

The type of narrowing you get in .otherwise(...) and .with(...) is a shallow one, meaning it only excludes members from the top level union type.

If your input type is a discriminated union of objects, then narrowing works:

type Entity = 
  | { type: 'user', name: string }
  | { type: 'org', id: string };

const f = (entity: Entity) => 
  match(entity)
    .with({ type: 'user' }, () => 'user!')
    .with({ type: 'user' }, () => 'user!')
    //                   ^ ❌ Does not type-check in TS-Pattern v4.1
    .with({ type: 'org' }, () => 'org!')
    .exhaustive()

But if your union is inside a data structure like an array or an object, narrowing between branches doesn't work:

type Input = {
  entity: 
     | { type: 'user', name: string }
     | { type: 'org', id: string }
};

const f = (input: Input) => 
  match(entity)
    .with({ entity: { type: 'user' } }, () => 'user!')
    .with({ entity: { type: 'user' } }, () => 'user!')
    //              ^ Does type check even if this is already handled
    .with({ entity: { type: 'org' } }, () => 'org!')
    .exhaustive()

In your particular example I would invert my logic to circumvent this limitation:

type Person = {
  email: string | null;
};

const getEmail1 = (p: Person) =>
  match(p)
    .with({ email: P.string }, (i) => i.email.trim())
    .otherwise(() => undefined)

gvergnaud avatar Feb 11 '23 20:02 gvergnaud

Oh, and yes, I was talking about compile time performance. Deep exclusions can be pretty costly because they tend to generate large union types. I'm a bit conservative to avoid making ts-pattern a footgun in terms of compilation time (which is an important aspect of the developer experience)

gvergnaud avatar Feb 11 '23 20:02 gvergnaud