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

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

Open
jmagaram opened this issue Feb 11, 2023 · 5 comments
Open

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

jmagaram opened this issue Feb 11, 2023 · 5 comments

Comments

@jmagaram
Copy link

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
@gvergnaud
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

gvergnaud commented Feb 11, 2023

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
Copy link
Owner

gvergnaud commented Feb 11, 2023

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)

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

No branches or pull requests

2 participants