Probable bug: Contextual inference and intellisense at call sites buggy in case of circular type parameter constraints like T extends M<T>

[devanshj]

Bug Report

🔎 Search Terms

Circular type parameter constraint, intellisense, call site inference, contextual inference
Related - #44428 #40439

🕗 Version & Regression Information

Tested with TS 4.3.4

⏯ Playground Link

Playground

💻 Code

declare const m: <T extends M<T>>(m: T) => T
type M<Self, K = Exclude<keyof Self, "k" | "w">> =
  { a?: number
  , b?: number
  , c?: number
  , d?: number
  , k?: { $: K }
  , w?: { $: "a" }
  }

declare const $1: <T>(t: T) => { $: T }
declare const $2: <R>(t: R extends { $: infer X } ? X : never) => R
declare const $3: <R, T extends (R extends { $: infer X } ? X : never)>(t: T) => R

m({
  a: 1,
  b: 2,
  k: $1("a"), // compiles: yes, completions: no
  w: $1("a")  // compiles: yes, completions: no
})

m({
  a: 1,
  b: 2,
  k: $2("a"), // compiles: no, completions: yes
  w: $2("a")  // compiles: yes, completions: yes
})

m({
  a: 1,
  b: 2,
  k: $3("a"), // compiles: no, completions: yes
  w: $3("a")  // compiles: yes, completions: yes
})

🙁 Actual behavior

See code comments

🙂 Expected behavior

All three cases or at least $2 and $3 should compile and user should be able to see completions "a" and "b" for the first parameter of $* functions

It seems to be bug to me especially because in case of k for $2 and $3, you do get the completions and even the tooltip shows correct type parameters, but somehow the return type isn't the one thats expected and it doesn't compile AND it works totally fine with w.

[devanshj]

Workaround. Although it doesn't work in my local more complex scenario, so will have to find another minimal repro where it fails.

declare const m: <T extends M<T>>(m: T) => T
type M<Self, K = Exclude<keyof Self, "k" | "w">> =
  { a?: number
  , b?: number
  , c?: number
  , d?: number
  , k?: { $: K }
  , w?: { $: "a" }
  }

declare const $4:
  < R
  , T extends (R extends { $: infer X } ? X : never)>
  ( t: T) => R & { $: T }

m({
  a: 1,
  b: 2,
  k: $4("a"), // compiles: yes, completions: yes
  w: $4("a")  // compiles: yes, completions: yes
})

[RyanCavanaugh]

I think my comment in #44879 applies equally in this scenario -- $3 is not a meaningful declaration.

It's a fairly normal occurrence for completions to show more options than might be strictly legal, since you might be "in the middle" of writing something and need to provide several completed items before the surrounding call becomes valid (the set of outcomes of which we have no ability to predict within a few hundred milliseconds).

$2 seems to be expecting that inference will cause the result type { $: "a" } to be produced, but this isn't an intended use of infer. It should really be written the way $1 is.

[devanshj]

It should really be written the way $1 is

Agreed (others are workarounds). I think it'd make sense that the user gets completions for the first parameter of $1 because if you simply write what is being returned by $1 you do get the completions. Am I wrong in thinking this?

Here's a real world example... Playground. assign is basically an abstraction for the user to write less stuff, so I'd ideally expect that c gets inferred as { foo: number } in second call too.

createMachine({
  context: { foo: 1 },
  entry: {
    type: "xstate.assign",
    exec: c => ({ foo: c.foo + 1 })
  }
})

createMachine({
  context: { foo: 1 },
  entry: assign(c => ({ foo: c.foo + 1 })) // c is any but should be { foo: number } instead
})

declare const createMachine: <C>(defintion: Machine<C>) => {}
type Machine<C> = {
  context: C,
  entry: {
    type: string,
    exec: (context: NoInfer<C>) => unknown
  }
}

declare const assign: <C>(f: (context: NoInfer<C>) => NoInfer<C>) =>
  { type: "xstate.assign", exec: (context: C) => NoInfer<C> }

type NoInfer<T> = [T][T extends any ? 0 : never];

Also Ryan may I take it as a compliment that these two issues were (afaict) left for you to triage and label? :P Or they are so ridiculous that no one understood? :') xD

[RyanCavanaugh]

... I'd ideally expect that c gets inferred as { foo: number } in second call too

If you want inference here, I'd remove the NoInfer wrapper around C in assign. It works as expected then -- NoInfer is doing its job.

[devanshj]

Here's a version with no NoInfers, doesn't seem to work (I tried many other selective adding/removing of NoInfers too but none seem to work)

This was only a corollary example I gave, my initial question "Am I wrong in thinking this?" is much simpler... Would it be wrong to expect "a" and "b" get suggested via completion for the reasons I gave above when user writes k: $1("")? (currently they don't)

(Reason for initial version - In assign there are four places from where C can be inferred, I explicitly "closed" 3 via NoInfer and only kept one open which is where I want C to be inferred)

[typescript-bot]

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[devanshj]

@RyanCavanaugh senpai could I get your word on this please? Because I'm still not sure what is the intended behaviour 😅 Thanks for your time!

[RyanCavanaugh]

@devanshj sorry, been snoozing on this notification because I knew it'd be interesting (usually implies time-consuming, but not this time). I would consider the linked repro there likely to be a bug; the contextual inference candidate for the assign call should not have fixed to unknown like that. Can you log a fresh issue with just that one?

[devanshj]

No problems! and done. Though, Ryan, I'm asking this for the third time xD - I expect "a" and "b" to be suggested in completions, would that be wrong to expect?

Here's the most minimal repro... Afaict the assign issue and this are a consequence of the same bug.

declare const m: (x: { a: X, b: X }) => {}
type X = { $: "foo" | "bar" }
declare const $: <T>(x: T) => { $: T }

m({
  a: { $: "foo" },
  b: $("")
  //    ^|
  // expected completions: foo, bar
  // actual completions: none
})

My very naive analysis is that to calculate type of first parameter of m, the return type of $ (and assign in other case) needs resolved, which is done eagerly resulting in such behaviors.

[RyanCavanaugh]

I don't have a way to describe "expected" completion results; completions are either incorrect (e.g. we show you a value that wouldn't work, which is nearly always a bug) or just "would be nicer if it happened more often" (e.g. we fail to show a list at all, which is not necessarily a bug). During an in-fact illegal call (which usually means inference has failed), we apply some heuristics in the language service to try to piece together what we think values that might make the call actually legal. If the inference needed is too complex, none of those heuristics will apply, and there won't be completions as a result. These are tricky to tweak but if you have a PR that fixes it, we'd probably merge it. I wouldn't consider this case to be a per se defect because inference has failed and without a working inference, we can only rely on the salvaging heuristics, which will always be imperfect.

[devanshj]

Thanks for the detailed explanation. Though I realized I missed an important nuance in my question. What I meant to ask was - Is it wrong to expect the type parameter to be inferred as "a" | "b" or in the above case "foo" | "bar"? (the completions would only be a consequence of that).

It does work with NoInfer in the above case, but not in the original description issue for k. I've opened an issue regarding this where I also sell it why it seems like a bug or if not a bug then sure is a good improvement and hence becomes a feature request. So perhaps you can answer my question there directly.