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

Type Guards are no longer working correctly all the time #54178

Open
MaksiRose opened this issue May 8, 2023 · 5 comments
Open

Type Guards are no longer working correctly all the time #54178

MaksiRose opened this issue May 8, 2023 · 5 comments
Labels
Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Milestone

Comments

@MaksiRose
Copy link

Bug Report

πŸ”Ž Search Terms

Type Guard

πŸ•— Version & Regression Information

  • This changed between versions 4.9.5 and 5.0.4

⏯ Playground Link

A simple example

πŸ’» Code

declare class Foo<Stuff extends boolean> {
  stuff: Stuff extends true ? string : string | undefined;
  isReady(): this is Foo<true> 
}
declare const x: Foo<boolean>;
if(x.isReady()) {
  x
//^? - const x: Foo<boolean>
// Should be Foo<true>
}

πŸ™ Actual behavior

The Type Guard didn't change the type, which is unintuitive behavior.

πŸ™‚ Expected behavior

The method should act as a type guard and change the type

@Andarist
Copy link
Contributor

Andarist commented May 8, 2023

Might be related to #53436

@graphemecluster
Copy link
Contributor

The same bug happens in the code snippet from #9619 (comment) and it still presents in the latest nightly build.
Could anyone (perhaps @RyanCavanaugh?) please bisect it?

@Andarist
Copy link
Contributor

@graphemecluster your issue (and this one here) is caused by: #52984 , the one that I linked to here (#53436) was broken before that though

@RyanCavanaugh RyanCavanaugh added the Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases label May 26, 2023
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone May 26, 2023
@RyanCavanaugh
Copy link
Member

Foo<true> isn't considered a strict subtype of Foo<boolean> due to the variance measurement coming out too conservative when looking at stuff. TS doesn't have smarts to identify that a conditional type with T in the true branch and T | U in the false branch is actually covariant on the tested type parameter - I'm not entirely sure how feasible it would be to do that but it would at least be possible.

Counterexample that works:

declare class Foo<Stuff extends boolean> {
  observ: Stuff;
  isReady(): this is Foo<true>; 
}

declare const x: Foo<boolean>;
if (x.isReady()) {
  x;
  //^? - const x: Foo<true>
}

In this case you can write a variance annotation to make it work as desired:

declare class Foo<out Stuff extends boolean> {

@graphemecluster
Copy link
Contributor

@RyanCavanaugh Understandable, but #52984 is still a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

4 participants