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

Filter with "something is smallerType" don't work as negative #58996

Open
Noriller opened this issue Jun 24, 2024 · 5 comments Β· May be fixed by #59155
Open

Filter with "something is smallerType" don't work as negative #58996

Noriller opened this issue Jun 24, 2024 · 5 comments Β· May be fixed by #59155
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Milestone

Comments

@Noriller
Copy link

πŸ”Ž Search Terms

filter is type narrow negative

πŸ•— Version & Regression Information

  • This starts on version 5.5

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.2#code/C4TwDgpgBAggdgSwLYEMA2UC8UDeAoKKAIwCcIVgALALimBIFcIAaPAXwG489RIoAlAPYBjANZZcBYmQo0oAM3QBnFuy49w0AMqCkEKgjgBzCfGTooAHwEjR6+QzjDgCQXCgIlZ1GgAUS3X1KQyNaHT0DYwBKWgCI4OMPJVhEH0lCMmAGEnc4oJCAOlJyKnZuYTclYDoIKoBGMMDIk2x8QmLZWnomVjZyyurgWuAAJkb4kIk26RK5RTQVXvUKuCq6YKUASSUAeXFsAG0h+uYaqpGAXQL5BDQhkl9qzAA+JO90R6iorgB6H8IAYQAHoAfhS5jQBwu-VWgw22wA6iQ3C0oEdhnVTsdRlcbncIA8nq8AISed5+YBfX7-QHAsG+clWGxiKJQvBAA

πŸ’» Code

type Animal = {
  breath: true,
};

type Rock = {
  breath: false,
};

type Something = Animal | Rock;

function isAnimal(something: Something): something is Animal {
  return something.breath
}

const test1: Something = {
  breath: true,
}

const test2: Something = {
  breath: false,
};

const thisIsOk = [test1, test2].filter(t => isAnimal(t));
//       ^? Animal[]

const thisIsWrong = [test1, test2].filter(t => !isAnimal(t));
//       ^? (Animal | Rock)[]

πŸ™ Actual behavior

const thisIsOk = [test1, test2].filter(t => isAnimal(t));
//       ^? Animal[]

const thisIsWrong = [test1, test2].filter(t => !isAnimal(t));
//       ^? (Animal | Rock)[]

πŸ™‚ Expected behavior

const thisIsOk = [test1, test2].filter(t => isAnimal(t));
//       ^? Animal[]

const thisIsShouldBe = [test1, test2].filter(t => !isAnimal(t));
//       ^? Rock[]

Additional information about the issue

Filtering is working only if "positive", but if the "is" is used as a negative then it don't type narrow.

@jcalz
Copy link
Contributor

jcalz commented Jun 24, 2024

This is independent of filter():

function positive(t: Something) {
  return isAnimal(t)
}
// function positive(t: Something): t is Animal

function negative(t: Something) { 
  return !isAnimal(t)
}
// function negative(t: Something): boolean

I'm surprised that #57465 didn't do this. Inside negative() it certainly seems that !isAnimal(t) narrows in the way we expect:

function negative(t: Something) {
  if (!isAnimal(t)) {
    ((t));
    //^? (parameter) t: Rock
  } else {
    ((t));
    //^? ^?(parameter) t: Animal
  }
  return !isAnimal(t)
}

@fatcerberus
Copy link

fatcerberus commented Jun 24, 2024

@jcalz I don't think there's anything #57465 could have done to help here. We don't have negated types so you can't say t is not Animal--not even explicitly.

@jcalz
Copy link
Contributor

jcalz commented Jun 24, 2024

But we don't need t is not Animal, only t is Rock.

@jcalz
Copy link
Contributor

jcalz commented Jun 24, 2024

I certainly didn't mean that t => !isAnimal(t) should return t is Rock no matter what Something is, that would be bonkers. Yes, I'd expect Exclude<Something, Animal>, which in your example code was Rock.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases labels Jun 26, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jun 26, 2024
MichalMarsalek added a commit to MichalMarsalek/TypeScript that referenced this issue Jul 6, 2024
@MichalMarsalek MichalMarsalek linked a pull request Jul 6, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@jcalz @fatcerberus @RyanCavanaugh @Noriller and others