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

Contextual typing interferes with type argument inference #33738

Closed
ktsn opened this issue Oct 2, 2019 · 5 comments
Closed

Contextual typing interferes with type argument inference #33738

ktsn opened this issue Oct 2, 2019 · 5 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@ktsn
Copy link

ktsn commented Oct 2, 2019

TypeScript Version: 3.6.3, 3.7.0-beta

Search Terms: inference, wrong, type parameter

Code

interface Instance {
  test: string
}

type DataDef<Data, Props> = Data | ((this: Readonly<Props> & Instance) => Data)

export type ThisTypedComponentOptionsWithArrayProps<Data, PropNames extends string> =
  object &
  ComponentOptions<DataDef<Data, Record<PropNames, any>>, PropNames[]> &
  ThisType<Instance & Data & Readonly<Record<PropNames, any>>>;

type DefaultData<V> =  object | ((this: V) => object);
type DefaultProps = Record<string, any>;
export interface ComponentOptions<Data = DefaultData<Instance>, PropsDef = PropsDefinition<DefaultProps>> {
  props?: PropsDef;
  data?: Data;
  watch?: Record<string, WatchHandler<any>>;
}

export type ArrayPropsDefinition<T> = (keyof T)[];
export type PropsDefinition<T> = ArrayPropsDefinition<T>;

export type WatchHandler<T> = (val: T, oldVal: T) => void;

declare function test<Data, PropNames extends string = never>(options?: ThisTypedComponentOptionsWithArrayProps<Data, PropNames>): void;
declare function test(options?: never): void

test({
  data () {
    return {
      foo: true
    }
  },
  watch: {
    testWatch (value: boolean) {
      this.test // any
    }
  }
})

Expected behavior:
this.test in testWatch is of type string.

Actual behavior:
this.test in testWatch is of type any.

Looks like Instance keys are used as PropNames and they override actual instance type.

Note that if I do either following things, it behaves as expected:

  • Remove value´s type annotation.
  • Do not annotate this type of DataDef callback.
  • Do not use overload for test function.

Playground Link:
http://www.typescriptlang.org/play/#code/JYOwLgpgTgZghgYwgAgJIgM5jiJyDeAUMspFgFzJZSgDmhAvoYWAJ4AOKAInNlxDAA8PbABpkABSgB7dhgB8yALzIRcZAB9kACm1gAFsAyUAShDgATaSAA2rQVNkLkAMjSZsuCAEplitd7MEAAe7NJQYKQcKAAqhhgx0RYAwtIAtmEgEOAA8uxgwNYYAOrABgCCUFBwrI5ywrxw4nUAcnBpEBjIIZAgFl3UdIpKxMjSAEYAVhAIkS6jqRnW2WB5BUUNfAKbTchmCOEWDjLsbR0Y4jis8vLNJ2edANoAuorzJHFGiZyC6Fg4eDcalce3MVls9n2h2OsgeF2QVxu8gA3Mw2JxVAI4ABXGxgNSCABqwxIE2ms00Oj08UohN8SkUZJmYG8qPR3CxuLAdS6KihUCOgxAtEuIGuqJCYQiyFAkFgiBQi0yKzWhUwO2UmPgXIJf08SFukhOGH4ME1PNNoDKauEnLxPJuBFG7GNAH5KBaBKiSBZGu7VI1vcgAO68BD6f38wVgGjC8TFMP6AASOAsNmggkRKMYQVC4Ui7OQlWqtWNlpA1usghiJO0AGsIKxpGaYt4XhK89LC56YFb1iBqyTizUe32bTXUYRJfmohiE2Bwym+umoIPNdoAG5wGyUGLiaQ2CyE7e7+mKDfSYAWScWGY2OBQFAwbG4fukTpgHZ3WHtTrdYK9P0VAxnQmpZBu0DyNosj9hg-qfAkSRKssuT5GqJRlPow6lk4X5Gj+5zyN4lAXleqK3gg96Psgz6vmq75YNBaFFP64HQMRyCkRYaIftoRA+o0Oi+PxJDII+YDYlAIBOqJokwNI0iUDG2IQKMJBMOpoijKGC76JQIkkGQYDzuGOhbjYKmUOMCnpjgwlqaJBhGAAdEZyAAPTuQiYoORpyBMAwgTMEAA

Related Issues:
This is quite similar issue with #33164 but different as it can reproduce on v3.6.3 which the issue is already fixed.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Oct 30, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Oct 30, 2019
@sandersn sandersn added this to Not started in Rolling Work Tracking Dec 5, 2019
@sandersn
Copy link
Member

Here's a simpler repro that gives some clues as to what is going on:

type I<K extends string> = { p: string } & Record<K, number>;
type Thusly<Data, K extends string> =
    ThisType<Data & I<K>> & {
        data?: (this: I<K>) => void,
        watch?: { [s: string]: (val: any) => void }
    }
declare function test<K extends string = never>(options?: Thusly<unknown, K>): void;
declare function test(options?: never): void
test({
  data () {},
  watch: {
      m (value: boolean) {
          var x: never
          var x = this.p
      }
  }
})

In the error case, this.p is now of type never and the type argument of test is inferred to be "p".
In the correct case, this.p is still of type string and the type argument is inferred to be never.
It appears that K is instantiated as never in the correct case and in I, Record<never, number> = {}. In the error case it's K="p", resulting in Record<"p", number> = { p: number }. Then { p: string } & { p: number } = { p: never }. Quick info for the type of this backs this up.

Also, the error only reproduces if some type (I haven't figured out which one yet) is not a subtype of value -- value: boolean causes the repro, as does InnerHTML, but unknown does not.

The error is entirely down to the result of inference; using "p" as the type argument always gives the wrong result and never always give the right one. Changing the three variables you give above determines which type argument is inferred but nothing else about the behaviour of this.p.

So I think this can be broken down into two problems:

  1. Why does inference flip between "p" and never when changing the 3 things outlined above?
  2. What should the type of this be given type arguments 'p' and never, respectively?

For (2), I think the current behaviour is correct, actually. That means a smaller repro for (1) is this:

type I<K extends string> = { p: string } & Record<K, number>;
type Thusly<Data, K extends string> =
    ThisType<Data & I<K>> & {
        data?: (this: I<K>) => void,
        watch?: { [s: string]: (val: any) => void }
    }
declare function test<K extends string = never>(options?: Thusly<unknown, K>): K;
declare function test(options?: never): void;
var bad: "p"
var bad = test({
  data () {},
  watch: {
      m (value: boolean) {}
  }
})
var good: never
var good = test({
  data () {},
  watch: {
      m () {}
  }
})

Right now I guess that this repro needs

  1. a subtype check (from the way we check overloads).
  2. this-type inference from the source object literal -- watch needs to be context sensitive.
  3. this-type inference to the target type -- data needs to have a this parameter.

(2) might actually be something to do with assignability, and (3) might be something to do with intersections in ThisType, since removing Data & from ThisType's argument also makes the repro stop working.

@sandersn
Copy link
Member

Briefly, I can get rid of ThisType as long as D & I<K> is somewhere in Thusly, as well as change the parameter name of data:

type I<K extends string> = { p: string } & Record<K, number>;
type Thusly<Data, K extends string> = {
    data: (x: Data & I<K>) => void,
    watch: { [s: string]: (val: any) => void }
}
declare function test<K extends string = never>(options?: Thusly<unknown, K>): K;
declare function test(options?: never): void;
var bad: "p"
var bad = test({
  data (x) {},
  watch: {
      m (value: boolean) {}
  }
})
var good: never
var good = test({
  data (x) {},
  watch: {
      m () {}
  }
})

So there doesn't appear to be this-type inference here; just contextual typing interfering with type inference.

@sandersn
Copy link
Member

Works on 2.3 - 3.5 — although after sleeping on it, I think that the inference from bad is probably correct and good is not.

I'll bisect 3.5 – 3.6 to see which PR broke things.

@sandersn sandersn changed the title Type parameter inference chooses wrong type from callback 'this' type Contextual typing interferes with type argument inference Jan 23, 2020
@sandersn
Copy link
Member

The break happens at #32558. However, I believe the bad type is correct and that the real bug is that contextual typing still prevents inference from working correctly in the good case.

I'm going to put this in the backlog since I think it's low priority. @ahejlsberg you might be interested in this, especially if I've mis-diagnosed the role of contextual typing.

@sandersn sandersn added the Bug A bug in TypeScript label Jan 23, 2020
@sandersn sandersn removed their assignment Jan 23, 2020
@sandersn sandersn modified the milestones: TypeScript 3.8.1, Backlog Jan 23, 2020
@sandersn sandersn removed the Needs Investigation This issue needs a team member to investigate its status. label Jan 23, 2020
@sandersn sandersn removed this from Not started in Rolling Work Tracking Jan 23, 2020
@RyanCavanaugh
Copy link
Member

It's string now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants