microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.48k stars 12.43k forks source link

with infer in template string, suggestion works in 4.6, but not work in 4.7 #49680

Closed xxleyi closed 1 year ago

xxleyi commented 2 years ago

Bug Report

🔎 Search Terms

suggestion pathof

🕗 Version & Regression Information

4.7

⏯ Playground Link

Playground link with relevant code

💻 Code


// get from https://github.com/microsoft/TypeScript/issues/20423#issuecomment-812564842
type PathOf<T, K extends string, P extends string = ""> =
  K extends `${infer U}.${infer V}`
    ? U extends keyof T ? PathOf<T[U], V, `${P}${U}.`> : `${P}${keyof T & (string | number)}`
    : K extends keyof T ? `${P}${K}` : `${P}${keyof T & (string | number)}`;

declare function consumer<K extends string>(path: PathOf<{a: string, b: {c: string}}, K>) : number;

// suggestion works well in 4.6, and not work in 4.7
consumer('b.')

🙁 Actual behavior

image

🙂 Expected behavior

image

rbuckton commented 2 years ago

This doesn't seem to be due to infer, but instead is a direct result of the changes in #48410. @andrewbranch, is this an intended consequence of that change? Prior to this change (or if I skip the call to runWithInferenceBlockedFromSourceNode), we instantiate K with the string literal type "b.". However, with runWithInferenceBlockedFromSourceNode we instead end up instantiating K with string.

rbuckton commented 2 years ago

Note that we have no suggestions in 4.7.4 for "b." because the suggestion list we return includes "a" and "b", and "b." has essentially already filtered out the possible suggestions.

andrewbranch commented 2 years ago

No, not intended. The nonlinearity of conditional types makes it impossible to guess with any certainty whether a wider or narrower instantiation is going to produce better completions... maybe we ought to do both and concatenate the results.

dmitri-gb commented 2 years ago

We have also come across a somewhat similar case where the list of suggestions in TS 4.7 (and 4.8) is no longer the same as it was in TS 4.6.

declare function pick<T extends object, K extends keyof T>(obj: T, ...keys: K[]): Pick<T, K>;

const obj = { aaa: 1, bbb: '2', ccc: true };

pick(obj, 'aaa', ''); // suggestions inside the empty string
                      // TS 4.6: aaa, bbb, ccc
                      // TS 4.7: aaa

Playground link

It is worth noting that if I rewrite pick and make keys a proper array instead of a rest parameter, then the list of suggestions will include all the properties just as in TS 4.6.

Another way to get around the issue is to use a slightly different definition for pick:

declare function pick<T extends object, K extends (keyof T)[]>(obj: T, ...keys: K): Pick<T, K[number]>;

It's unfortunate that the (seemingly) more natural definition of pick looses its ability to provide useful suggestions. I'm curious if this is something that the TS team would consider a regression and try to revert, or is this how we should expect things to work from now on?

wwwouter commented 1 year ago

I'm experiencing the same thing. It worked with 4.6.x, and it still works with 4.9.0-dev.20221007 and 4.8.1-rc

4.6.4

image

4.7.4:

image

It compiles fine with "id" as second argument with both versions, it seems to be the language server that is acting up.

itsUndefined commented 1 year ago

Can this task get a higher priority?

Andarist commented 1 year ago

@dmitri-gb's and @wwwouter's cases are somewhat different from the OP. The problem with those 2 is that the type param is being used as rest but only the "current" argument is being "blocked". If all arguments typed through this rest would get blocked then it would work OK. At the same time... it's not that hard to imagine that a different piece within a given call could fix the inference in a similar way, so fixing this specific case by blocking sibling arguments that are typed through the same rest wouldn't necessarily fix the whole issue. OTOH, this particular use case is probably more common than those other cases so perhaps it would still be worth exploring this as a solution right now.