gvergnaud / ts-pattern

🎨 The exhaustive Pattern Matching library for TypeScript, with smart type inference.
MIT License
11.86k stars 123 forks source link

After P.nullish match, value can still be null #145

Open jmagaram opened 1 year ago

jmagaram commented 1 year ago

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

gvergnaud commented 1 year ago

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

jmagaram commented 1 year ago

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 commented 1 year ago

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
gvergnaud commented 1 year ago

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(input)
    .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 commented 1 year ago

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)