microsoft / TypeScript

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

Function overloading merging implementation signatures #52442

Closed pedrodurek closed 1 year ago

pedrodurek commented 1 year ago

Bug Report

If I have a function overloading where the second argument depends on the signature of the first argument, it merges all implementation signatures for the second argument.

Ideally, I want to solve this problem with union type rather than overloads, but tuple "loses" its signature when using conditional types. For example:

type Options = 'a' | 'b'

declare function func<T extends Options | Options[]>(arg: T extends any[] ? [...T] : T): T;

const result = func(['a', 'b']) 
// result type: ('a' | 'b')[] instead of  ['a', 'b']

šŸ”Ž Search Terms

function overloading, implementation signatures

šŸ•— Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about Function overloading.

āÆ Playground Link

Playground link with relevant code

šŸ’» Code

type Options = 'a' | 'b'

declare function func<T extends Options[]>(arg: [...T], arg2: T[0]): T;
declare function func<T extends Options>(arg: T, arg2: T | 'c'): T;

const result = func(['a', 'b'], 'c') // Type inferred for the second argument should be 'a', but it's returning 'a' | 'b' | 'c'
const result2 = func('a', 'c')  // Type inferred for the second argument should be 'a' | 'c', but it's returning 'a' | 'b' | 'c'

At least with function overloading I can get the appropriate return type for result ['a', 'b']

šŸ™ Actual behavior

Type inferred for the second argument is 'a' | 'b' | 'c'.

šŸ™‚ Expected behavior

Type inferred for the second argument should be 'a' in the first case and 'a' | 'c' in the second case.

const result = func(['a', 'b'], 'c')
// arg2: "a" as expected

const result2 = func('a', 'c')
// arg2: "a" | "c" as expected
RyanCavanaugh commented 1 year ago

This seems to all be working according to expectation.

type Options = 'a' | 'b'

declare function func<T extends Options[]>(arg: [...T], arg2: T[0]): T;
declare function func<T extends Options>(arg: T, arg2: T | 'c'): T;

const result = func(['a', 'b'], 'c'
// result: ["a", "b"] as expected

const result2 = func('a', 'c'
// result2: "a" as expected
pedrodurek commented 1 year ago

Hey @RyanCavanaugh. The problem is not the return type, it's actually with the second argument (arg2). In my case the second argument is constrained by the first one.

type Options = 'a' | 'b'

declare function func<T extends Options[]>(arg: [...T], arg2: T[0]): T;
declare function func<T extends Options>(arg: T, arg2: T | 'c'): T;

const result = func(['a', 'b'], 'c')
// arg2: "a" as expected

const result2 = func('a', 'c')
// arg2: "a" | "c" as expected
Screenshot 2023-01-26 at 6 56 48 PM
pedrodurek commented 1 year ago

Hey @RyanCavanaugh, do you still need more info?

RyanCavanaugh commented 1 year ago

Is this bug about what shows up in the completion list?

pedrodurek commented 1 year ago

Yes, the bug lies in the IDEs' code completion

RyanCavanaugh commented 1 year ago

As shown in #52516, it's not always the case that the best completion list is the one informed by the current inference of the call, so we have some logic that tries to be the most helpful. Since it's generally better to show more rather than less (since presumably the programmer knows how to write correct code), this is the intended behavior in this scenario.

pedrodurek commented 1 year ago

Hey @RyanCavanaugh, I'm not sure if this is related, but something is not right with the code completion for function overloading. Take a look at this different example here, where the first argument depends on the second one. The code completion simply loses the inference of the second argument (true or false) and fallback to boolean, which affects the code completion for the first argument.

Screenshot 2023-01-31 at 4 49 17 PM

ā˜ļø It's like if the second argument is inferring as the constraint value (boolean) rather than true or false, which will cause the first argument to be Option | Option2.

pedrodurek commented 1 year ago

Hey @Andarist, I'd love your input on this one if possible ā¤ļø ! Thank you in advance!

Andarist commented 1 year ago

I would have to think more about completions with overloads - but I certainly see how this request might be rather hard to implement. It's hard for me to reason about this without studying the related code more, so far I focused mostly on the type checking itself and not on completions. The problem with completions is that the algorithm uses the same functions to resolve the signature as the typechecker does - but the typechecker has to report errors when it doesn't find a match whereas completions try to select the best candidates for the request etc (so it can sometimes "match" the overload/signature that otherwise wouldn't be matched by the typechecker). There are some flags that make it behave differently in the case of a completion request but overall the algorithm isn't sprinkled with checking those all over the place (which is a good thing as, for the most part, the logic is the same). What I'm trying to say is that completions are kinda "best effort" thing in the algorithm there so it isn't that surprising that sometimes it yields results that might look a little bit off in certain more complex situations.

Eeeeither way... you can make it work with a single signature:

type Tuple<T> = [T?, ...T[]];

type Options = "a" | "b";

declare function func<T extends Options | Tuple<Options>>(arg: T): T;
const result = func(["a", "b"]);
//    ^? ['a', 'b']

declare function func2<T extends Options | Tuple<Options>>(
  arg: T,
  arg2: T extends any[] ? T[0] : T | "c"
): T;

// this only lists `a` as a valid completion here
func2(['a', 'b'], '')

// this lists both `a` and `c`
func2('a', '')

TS playground

pedrodurek commented 1 year ago

Thank you @Andarist. I really appreciate that! That definitely solves my problem! I also managed to solve it using the new const operator, but since the changes that I'm applying are for an open source (lib), I don't want to force people to upgrade their TS version yet to leverage those changes. The only edge case that I have is this one, which I think it's not exactly the same issue. I know that I'm asking too much ā¤ļø, but do you think there is a workaround for it?

Andarist commented 1 year ago

TS playground

type Options = "a" | "b";
type Options2 = "aa" | "bb";

type OptionResult<K extends boolean> = K extends true ? Options2 : Options;

declare function func<T extends OptionResult<K>, K extends boolean>(
  ...args: [option: T, foo: K] | [option: T, defaultValue: string, foo: K]
): string;

// this autocompletes `'aa' | 'bb'`
const result = func("", true);

// this autocompletes `'a' | 'b'`
const result2 = func("", false);

// this autocompletes `'aa' | 'bb'`
const result3 = func("", 'str', true);

// this autocompletes `'a' | 'b'`
const result4 = func("", 'str', false);

Note though that I think it's moslty a weird requirement for the former argument to autocomplete based on the latter one. It's not how people type in the argument - they type from the left to the right so the ability to autocomplete based on the latter argument (that usually wasn't even typed in yet) doesn't really bring a lot to the table. Conceptually it can/should work - but in practice it's not that useful.

pedrodurek commented 1 year ago

thank you very much @Andarist ā¤ļø . I know it's a weird requirement šŸ˜… . In the i18n library, we infer the translation keys based on the translation files and the given namespace, but users may change the namespace for a specific case, for example:

const translations = {
  common: {
    key1: {
      key2: ''
    }
  },
  dashboard: {
    key3: {
      key4: ''
    }
  }
}

const {t} = useTranslation('common');
t('key1.key2');
t('key3.key4', {ns: 'dashboard'}); // Here

If we change the ns, it'll infer different keys.

The library offers a lot of customisations, which makes the types quite complex: https://tsplay.dev/WokvMW

github-actions[bot] commented 1 year ago

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