microsoft / TypeScript

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

IntelliSense/Autocomplete only resolves the generics in the first overload of a generic function with multiple overloads #54965

Open Rodentman87 opened 1 year ago

Rodentman87 commented 1 year ago

Bug Report

🔎 Search Terms

Generic, Overload, Type Inference

🕗 Version & Regression Information

This is the behavior in every version I tried on the playground

⏯ Playground Link

Playground link with relevant code

(To see the issue in action, go to line 55 and add a comma to the parameters to trigger the completion and look at both overload options)

💻 Code

abstract class BaseMetric<ID extends string> {
  abstract valueType: any;

  constructor(public id: ID) {}
}

class NumberMetric<ID extends string> extends BaseMetric<ID> {
  declare valueType: number;
}

type MetricValueType<
  Metrics extends BaseMetric<string>[],
  ID extends Metrics[number]["id"]
> = Extract<Metrics[number], { id: ID }>["valueType"];

interface TrophyRoomOptions<Metrics extends BaseMetric<string>[]> {
  metrics: Metrics;
}

class TrophyRoom<
  // This is the metrics list that the user passes when initializing the class
  Metrics extends BaseMetric<string>[]
> {
  metrics: Metrics;

  constructor({ metrics }: TrophyRoomOptions<Metrics>) {
    this.metrics = metrics;
  }

  // The ID generic on this function is used to extract which metric we're looking to change so that the value type
  // can be inferred based on the metrics list passed in when creating the instance of the class. The type restriction
  // on ID makes sure that the user only enters an id of a metric that was passed to the instance at creation.
  // Overload A: takes the new value
  setMetric<
    ID extends Metrics[number]["id"],
    ValueType extends MetricValueType<Metrics, ID>
  >(id: ID, value: ValueType): void;
  // Overload B: takes a function that takes the old value and returns the new value
  setMetric<
    ID extends Metrics[number]["id"],
    ValueType extends MetricValueType<Metrics, ID>
  >(id: ID, value: (oldValue: ValueType) => ValueType): void;

  setMetric<
    ID extends Metrics[number]["id"],
    ValueType extends MetricValueType<Metrics, ID>
  >(id: ID, valueOrFunction: ValueType | ((oldValue: ValueType) => ValueType)): void {
    // ...
  }
}

const thing = new TrophyRoom({
  metrics: [new NumberMetric("test"), new NumberMetric("test2")],
});

🙁 Actual behavior

In the IntelliSense/Autocomplete for thing.setMetric() (when specifying "test" as the first argument), the type of the first overload gets simplified to:

setMetric(id: "test", value: number): void

and the type of the second overload doesn't get simplified at all

setMetric<ID extends "test" | "test2", ValueType extends MetricValueType<(NumberMetric<"test"> | NumberMetric<"test2">)[], ID>>(id: ID, value: (oldValue: ValueType) => ValueType): void

This behavior is not based on the second overload being more complex, if you swap the positions of the two overloads, the first one will get simplified to:

setMetric(id: "test", value: (oldValue: number) => number): void

while the second one doesn't get simplified

setMetric<ID extends "test" | "test2", ValueType extends MetricValueType<(NumberMetric<"test"> | NumberMetric<"test2">)[], ID>>(id: ID, value: ValueType): void

🙂 Expected behavior

Both overloads should get simplified to:

setMetric(id: "test", value: number): void

and

setMetric(id: "test", value: (oldValue: number) => number): void

regardless of their order in the overloads

Workaround

While it is possible in this case to just use a single definition for the method instead of the overloads:

setMetric<
  ID extends Metrics[number]["id"],
  ValueType extends MetricValueType<Metrics, ID>
>(id: ID, valueOrFunction: ValueType | ((oldValue: ValueType) => ValueType)): void

having the two overloads makes it clearer that the method has two separate behaviors based on whether you're passing a raw value or a function to update to the value.

Andarist commented 1 year ago

This is definitely a duplicate of some issue - but I can't find a good umbrella issue for this right now. In this comment here I noted that:

Fixing this might require some effort/a different approach to how completions are provided in cases like this. Note that, for example, getStringLiteralCompletionsFromSignature is already using a different - overload-capable~ - strategy to provide completion directly at argument positions. This one is nested in the argument and thus a simpler approach of getContextualType is used here.

In general, completions at positions nested in arguments usually don't take overloads into account today.