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

Lookup type on this unexpectedly takes true branch of conditional type #37778

Closed
thetutlage opened this issue Apr 3, 2020 · 23 comments
Closed
Assignees
Labels
Bug A bug in TypeScript Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@thetutlage
Copy link

thetutlage commented Apr 3, 2020

  • Asked on Gitter
  • Tried my best to search existing issues
  • Tried questioning my understanding, but still the behavior feels weird to me
  • Finally, I am forced to create this issue (though I know, the issues list is too big)

TypeScript Version: v3.8.3 and Nightly
Search Terms: Generics within class body, Generics in class body

Code

type IsString<T extends any, K extends keyof T> = T[K] extends string ? T[K] : never

class Foo {
  public username!: string
  public age!: number
  public dob!: { [key: string]: any }

  public getStringAttr<T, K extends keyof T>(this: T, key: K): IsString<T, K> {
    return {} as unknown as IsString<T, K>
  }

  public someInternalFunction() {
    this.getStringAttr('dob')
  }
}

new Foo().getStringAttr('dob')

The above is just a dummy code to reproduce the issue with minimum effort.

  • The class Foo has a method getStringAttr, which accepts the name of the class property and returns it back if it is a string, or never if it is not a string

Expected behavior:

Regardless of where the getStringAttr method is called, it should have consistent behavior.

Actual behavior:

  • Calling getStringAttr by creating a new class instance works fine
  • Calling getStringAttr from a different class method (within the class body) makes it infer incorrect types
  • The getStringAttr('dob') should return never in both cases. But within the class body it returns string.
  • The getStringAttr('age') should return never in both cases and it does so.

Playground Link: https://www.typescriptlang.org/play/index.html?ssl=1&ssc=1&pln=17&pc=31#code/C4TwDgpgBAkgzgZWAJwJYDsDmAeAKlCAD2AnQBM4oBDdEAGigGkDjSKoBrCEAewDMouAHxQAvIIDajALosS5SnBQZMUAPySZUAFxR0EAG4RkAWABQ5gMYAbKnEoAxHjygBvc1ChgArgCNrqJZQ3nDG6FQAthAAhLpKaFgeXn4BQVSYMbro3hG+xkk+-oFQZDy+sW5QElwgccpY0ro0IFAAvuYFKcUZwEgJmACCwCh4DMxE8uw1-IJCABTAABaocLq4DDW6jACUuvB9KqNMIu5mnp7IEMDeyOhurdSU3ugc6DwA7nd2sIj1OOvHJLtCxnZJFIJwHhRGDoEi3KjWBzPSzAVA8dBzbZuJKeJYrAB0PQOWCGKDmAHJSr5ydsgeZgeZ9O8oE4eJjCVdiYNhsgKVSaUA

Related Issues: Cannot find any

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Apr 3, 2020
@RyanCavanaugh RyanCavanaugh changed the title Generics within the class body returns infers to incorrect types Lookup type on this unexpectedly takes true branch of conditional type Apr 3, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.0 milestone Apr 3, 2020
@thetutlage
Copy link
Author

Not to dictate. But does this really has to be part of 4.0? I am not sure, how release cycle really works, but there is a 3.9 iteration plan #37198 and I am happy to contribute, if it can be added to 3.9 vs waiting till August

@RyanCavanaugh
Copy link
Member

We have too many bugs to fix in 3.9 already. If you're capable of fixing this, feel free to, but this is likely a very complex issue.

@thetutlage
Copy link
Author

Okay, makes sense. Just wanted to know. Thanks for quick response 😄

@jack-williams
Copy link
Collaborator

jack-williams commented Apr 6, 2020

The getStringAttr('age') should return never in both cases and it does so.

Can you post code for this? I wasn't able to reproduced it.

I'm not sure this is a bug. Inside the method the type parameter this is used, and because type parameter constraints are ignored in conditional types, IsString<this, 'dob'> remains an unresolved type. Outside the class a concrete type Foo is used which can be resolved.

This shows the same behavior, but without the confusion of this types.

function foo<T extends { dob: { [key: string]: any } }>(t: T, w: IsString<T, 'dob'>) {
  w // IsString<T, 'dob'> because the conditional type is unresolved
}

foo(new Foo(), _); // second argument is expected to be never

@thetutlage
Copy link
Author

thetutlage commented Apr 8, 2020

I shared the link to the playground in the original post

@SpringNyan
Copy link

type Test<T> = T extends never ? 0 : 1;

class Foo {
  str!: string;
  result!: Test<this['str']>;

  foo() {
    return this.result;
  }

  bar(): string {
    return this.result; // Type is '0 | 1'
  }
}

const result = new Foo().foo(); // Type is 1

Also hit error which may caused by this issue.

@weswigham
Copy link
Member

@SpringNyan no...? Test returns 0 or 1, neither of which are strings... the error on bar there seems perfectly valid.

@SpringNyan
Copy link

@weswigham I means the error should be Type '1' is not assignable to type 'string', but not '0 | 1'.

Another comparison:

type Test<T> = T extends never ? 0 : 1;

class Foo {
  str!: string;
  result!: Test<this['str']>;
  testStr!: Test<string>;

  result0(): 0 {
    return this.result; // Type '0 | 1' is not assignable to type '0'.
  }

  result1(): 1 {
    return this.result; // ok, but should be error if the result is inferred as '0 | 1' as above
  }

  testStr0(): 0 {
    return this.testStr; // Type '1' is not assignable to type '0'.
  }

  testStr1(): 1 {
    return this.testStr; // ok
  }
}

@weswigham
Copy link
Member

this['str'] is open ended and generic - it acknowledges that you could subclass Foo with something where the property str is never.

@SpringNyan
Copy link

@weswigham Thanks for the explanation. I'm now using type Test<T> = T extends infer U ? U extends never ? 0 : 1 : never; to workaround this case.

@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Aug 31, 2020
@thetutlage
Copy link
Author

thetutlage commented Sep 4, 2020

Any change of this getting fixed in the next release? 🙂

@thetutlage
Copy link
Author

Checking back to see if it can be prioritised

@thetutlage
Copy link
Author

Still waiting for it 🙂

@thetutlage
Copy link
Author

Did it make its way to 4.2?

@thetutlage
Copy link
Author

@RyanCavanaugh Is there any way to understand how bugs are prioritized? Moving it to next version everything is not really helping.

I know, I should be entitled on an open source project. But after maintaining a few for years, atleast, we can have some conversation to understand the reasoning behind the delay and if you need something on my end to help it move forward

@RyanCavanaugh
Copy link
Member

@thetutlage we fix bugs in milestones as quickly as we can. Sometimes we're not able to get through every bug in a milestone, so it moves to the next milestone. I'm sorry you encountered a delay on this bug but as a team staffed with humans, we're just doing as much as we can while still delivering a few features in each release.

We do not apply a global sorting algorithm to prioritize bugs, but data I look at would be:

  • Is this a regression? No, the behavior dates back to at least 3.3
  • How many users have reported encountering the problem, and over what length of time? In other words, how frequently is the problem encountered? Looks like approximately 1 over the multiple years (?) this behavior has been in the product
  • What's the impact? This looks like a corner case around this types since there isn't any other reported manifestation
  • Is a workaround available? I haven't checked, but probably
  • Is this even definitely a bug? It's not super clear to me TBH, but it's likely

So on net it's marginal for this one to be scheduled into a milestone as opposed to in the "eventually fix" backlog. That's my assessment of the priority here.

@RomainLanz
Copy link

Maybe you are not aware of this, but this bug creates an issue in AdonisJS ORM.
Many people are encountering this bug, not only one. We tell them to wait since we created this issue.

Some discussions about it:

@danpospisil
Copy link

I am have encountered this issue as well. So I hope it will get fixed soon.

@thetutlage
Copy link
Author

Any chance this will be fixed in 4.5.0?

@thetutlage
Copy link
Author

Hello 👋

Do I have to do anything special to get this fixed. It is a bug and I don't think that a bug should be in demand to get attention

@manansharma91
Copy link

Faced this issue today.

Such a big company like Microsoft is rescheduling a bug for 2 years with no explanation altogether. No wonder why teams (Deno) are slowing moving away from TS

@thetutlage
Copy link
Author

How about moving the milestone to 4.7?

@RyanCavanaugh
Copy link
Member

This seems to be fixed now

type IsString<T extends any, K extends keyof T> = T[K] extends string ? T[K] : never

class Foo {
  public username!: string
  public age!: number
  public dob!: { [key: string]: any }

  public getStringAttr<T, K extends keyof T>(this: T, key: K): IsString<T, K> {
    return {} as unknown as IsString<T, K>
  }

  public someInternalFunction() {
    let d = this.getStringAttr('dob');
    // d: IsString<this, "dob"> (this is a correct deferral)
  }
}

const d = new Foo().getStringAttr('dob')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests

9 participants