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

There are cases where unwanted patterns match. #228

Closed
gitsunmin opened this issue Mar 14, 2024 · 8 comments
Closed

There are cases where unwanted patterns match. #228

gitsunmin opened this issue Mar 14, 2024 · 8 comments

Comments

@gitsunmin
Copy link

gitsunmin commented Mar 14, 2024

Describe the bug
There are cases where unwanted patterns match.

CASE 1

import { match } from 'ts-pattern';

const user1 = { name: 'Gitsunmin' };

const result = match(user1)
  .with({}, () => true)
  .otherwise(() => false);

console.log('result:', result); // true

the expected value: false;

a resultant value: true

CASE 2

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

const user1 = {};

const result = match(user1)
  .with({ name: P.nullish }, () => true)
  .otherwise(() => false);

console.log('result:', result); // false

the expected value: true;

a resultant value: false

Versions

  • TypeScript version: 5.0.2
  • ts-pattern version: 5.0.8
  • environment: bun 1.0.30
@gitsunmin gitsunmin changed the title There is a case where the unwanted Pattern matches. There are cases where unwanted patterns match. Mar 14, 2024
@gvergnaud
Copy link
Owner

gvergnaud commented Mar 14, 2024

Both behaviors are expected. TS-Pattern follows the semantic of corresponding object types:

  • The {} type contains every object, including { name: string }
  • The { name: null | undefined } has a required property name, so {} isn't assignable to it.

See:

const x: {} = { name: 'a' } // ✅ type-checks.
const y: { name: null | undefined } = {} // ❌ doesn't type-check.

@JUSTIVE
Copy link
Contributor

JUSTIVE commented Mar 15, 2024

if the {} matches all object types, then is there any way to match {} (an empty object) exactly?

@gvergnaud
Copy link
Owner

You can write custom matchers with P.when:

const emptyObject = P.when(
  (value: unknown) => value && typeof value === 'object' && Object.keys(value).length === 0
)

@gitsunmin
Copy link
Author

@gvergnaud
I've read the comments you've posted, and I have a suggestion I'd like to make.

How about creating "P.empty"?

.with(P.empty, () => true)

It could be used for more versatile applications by matching when "Array", "object", "Map", "Set" are empty.

If this seems like a good idea, I will go ahead and create a pull request.

Thank you for reading!

@JUSTIVE
Copy link
Contributor

JUSTIVE commented Mar 17, 2024

how about P.object.empty for a more precise expression? A P.empty could be ambiguous in some situations:

const x:Array<string> | Set<number> | Partial<SomeObject> = {}

match(x)
.with(P.object.empty, ()=>"fallback")
.otherwise(()=>"Meh")

would be great to suggest this idea on the separated issue, since this issue's topic has been closed.

@nullndr
Copy link

nullndr commented May 31, 2024

I would like to reopen this to handle the following Typescript cases:

type Bar = {
  type: "bar";
  value: "a" | "b";
};

type Foo = {
  type: "foo";
  value: "x" | "z";
};

declare const foobar: Bar | Foo;

match(foobar)
  .with({ type: "bar", value: "a" }, () => {})
  .with({ type: "bar", value: "b" }, () => {})
  .with({ type: "bar", value: "x" }, () => {}) // does not make sense
  .with({ type: "bar", value: "z" }, () => {}) // does not make sense
  .with({ type: "foo", value: "a" }, () => {}) // does not make sense
  .with({ type: "foo", value: "b" }, () => {}) // does not make sense
  .with({ type: "foo", value: "x" }, () => {})
  .with({ type: "foo", value: "z" }, () => {});

This looks like due to the fact that the type KnownPattern simply relies on the key's type used, creating an union, without having knowledge of the full pattern itself.

This case could be fixed with the following:

match(foobar)
  .with({ type: "bar" }, ({ value }) =>
    match(value)
      .with("a", () => {})
      .with("b", () => {})
      .exhaustive(),
  )
  .with({ type: "foo" }, ({ value }) =>
    match(value)
      .with("x", () => {})
      .with("z", () => {})
      .exhaustive(),
  );

But this is way more verbose than what it could be.

@JUSTIVE
Copy link
Contributor

JUSTIVE commented Jun 6, 2024

@nullndr I guess maybe the topic of this issue was too ambiguous. since this issue thread handles P.object.empty case and runtime behaviors, I'd rather open a new one that fits yours.
By the way, I'm not sure it's right to filter out patterns that don't make sense.
as shown below:

image

also here's the code for you, you should try yourself

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

const x: string = "hello world";

match<string,string>(x)
.with(P.number,()=>"impossible")
.otherwise(()=> "always")

the pattern P.number doesn't make sense but it's still a valid pattern, whether it matters on runtime or not.

@nullndr
Copy link

nullndr commented Jun 6, 2024

@JUSTIVE thank you, I will open a new issue where I will respond to your comment.

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

4 participants