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

Granular inference for generic function type arguments #20122

Closed
masaeedu opened this issue Nov 17, 2017 · 12 comments · May be fixed by #26349
Closed

Granular inference for generic function type arguments #20122

masaeedu opened this issue Nov 17, 2017 · 12 comments · May be fixed by #26349
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@masaeedu
Copy link
Contributor

Consider the following function, based on this StackOverflow question:

// Take a type and some properties to pluck from it. Ensure only functions can be plucked.
type PluckFuncs =
  <TComponent extends {[P in TKey]: Function }, TKey extends string>
  (props: TKey[]) => any

Currently it is not possible to use this function ergonomically. You must always supply both type arguments, even though TKey can comfortably be inferred from props.

It should be possible to invoke a generic function and specify a named subset of the type parameters, i.e. pluckFuncs<TComponent = { a: () => string }>(['a']), and have inference decide (or fail to decide) the remaining type parameters (in this case TKey).

In terms of the desired semantics, the inference should treat explicit type arguments no differently from the types of the function's value arguments, except for the fact that no values will be forthcoming at the invocation site.

In other words, doing this:

type PluckFuncs =
  <TComponent extends {[P in TKey]: Function }, TKey extends string>
  (props: TKey[]) => any

declare const pluckFuncs: PluckFuncs

pluckFuncs<{ a: () => string }>(['a']) // Won't work, forced to explicitly specify `TKey`

should behave no differently from when I just add a dummy value argument to serve as an inference site:

type PluckFuncs =
  <TComponent extends {[P in TKey]: Function }, TKey extends string>
  (props: TKey[], dummy?: TComponent) => any

declare const pluckFuncs: PluckFuncs

pluckFuncs(['a'], undefined as { a: () => string }) // Compiler is happy with this though

Aside:

The workaround one might come up with is to give TKey a default of keyof TComponent:

type PluckFuncs =
  <TComponent extends {[P in TKey]: Function }, TKey extends string = keyof TComponent>
  (props: TKey[]) => any

This seems like a sensible thing to do, but isn't really the semantics we're looking for. When you try to use it:

declare const pf: PluckFuncs

interface TestProps {a: number, b: Function, c: () => number}

// Good - missing property, should be an error
pf<TestProps>(['d'])

// Good - non-function property plucked, should be an error
pf<TestProps>(['a'])

// Bad - all plucked properties extend `Function`, but this is nevertheless an error
pf<TestProps>(['b', 'c'])

// We end up needing `TestProps` to not have any non-function properties whatsoever

// ...or to specify the second type argument explicitly
pf<TestProps, 'b' | 'c'>(['b', 'c'])

What's happened is that we only have one degree of freedom here. Defaulting TKey to a type derived from TComponent has ended up imposing an additional constraint on TComponent instead of constraining the parameter that uses TKey with respect to TComponent.

I think the behavior seen above is valid, and that default type arguments are a totally different ballgame. They shouldn't be mixed up in our problem with independent inference of type parameters when instantiating abstract function types.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 18, 2017

Duplicate of #10571?

@masaeedu
Copy link
Contributor Author

masaeedu commented Nov 18, 2017

@mhegazy Not really. I don't want any new syntax for skipping type parameters. You should simply be able to not supply anything for unnecessary type parameters and have it work.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 18, 2017

If i am not wrong you want named type arguments and partial inference. #10571, #19205 and #14400 all revolve around the same request, syntax aside.

@masaeedu
Copy link
Contributor Author

masaeedu commented Nov 18, 2017

They're all slightly different proposals. #10571 wants a syntax for marking type parameters as "to be inferred", #14400 wants to enforce a distinction between normal type parameters and those with defaults, and only infer the latter.

This issue is about simply making existing syntax have different semantics. I don't want any new keywords or syntax (although some future additions could be made to make things clearer), I just want the syntactically valid pluckFuncs<{ a: () => string }>(['a']) to also be semantically valid, and not produce the "Expected 2 type arguments, got 1" error. What exactly the semantic change should be I've tried to outline above with a mapping to TypeScript code that is valid today.

If it helps you track things, I'm fine with you folding this issue into either one, I just wanted to clarify what I'm proposing is distinct from those proposals.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Nov 18, 2017

In terms of the desired semantics, the inference should treat explicit type arguments no differently from the types of the function's value arguments, except for the fact that no values will be forthcoming at the invocation site.

Either I'm misunderstanding this or I strongly disagree. If you write

function fn<T>(x: T) { }

you should not be able to invoke it like this:

fn<Dog>(someAnimal);

Even though we would infer Animal from the candidate set Dog, Animal

@masaeedu
Copy link
Contributor Author

masaeedu commented Nov 18, 2017

@RyanCavanaugh Not following you. You're already able to call fn<Dog>(someAnimal); today, nothing prohibits that.

I'm also not proposing that that behavior of that should change in any way that I can think of. The semantics of the change I'm proposing are identical to adding a dummy?: { <every type parameter name>?: <corresponding type parameter> } to each generic function, and replacing all explicit applications of type arguments with application of a dummy value undefined as { ... }.

Could you make a full snippet I can stick in playground?

@RyanCavanaugh
Copy link
Member

type Animal = { move(): void };
type Dog = Animal & { woof(): void };

declare function fn<T>(x: T);
const a: Animal = { move() { } };
// Correct (IMHO) error today
fn<Dog>(a);

@masaeedu
Copy link
Contributor Author

masaeedu commented Nov 18, 2017

I'm not entirely clear why this isn't an error, but pretending function contravariance worked in TypeScript, change the above proposal to emitting a dummy?: { <every type parameter name>: (x: <type parameter>) => void }, and emitting corresponding functions (x: <type arg>) => void on the dummy value argument.

This would prevent assignability of (x: Dog) => void to (x: Animal) => void and cause your desired type error (assuming functions were contravariant). Obviously this doesn't actually work, but I'm not proposing you literally do this desugaring, I'm just trying to specify how it should behave.

@masaeedu
Copy link
Contributor Author

masaeedu commented Nov 18, 2017

Actually in the latest version that is an error, I was just using an old TypeScript. So how about something like that?

Full example for you to try:

type Animal = { move(): void };
type Dog = Animal & { woof(): void };

const fn = <T>(x: T, dummy?: { t?: (x: T) => {} }) => { }

const a: Animal = { move() { } };
fn(a, undefined as any as { t: (x: Dog) => {} });

Which produces:

[ts]
Argument of type '{ t: (x: Dog) => {}; }' is not assignable to parameter of type '{ t?: ((x: Animal) => {}) | undefined; } | undefined'.
  Type '{ t: (x: Dog) => {}; }' is not assignable to type '{ t?: ((x: Animal) => {}) | undefined; }'.
    Types of property 't' are incompatible.
      Type '(x: Dog) => {}' is not assignable to type '((x: Animal) => {}) | undefined'.
        Type '(x: Dog) => {}' is not assignable to type '(x: Animal) => {}'.
          Types of parameters 'x' and 'x' are incompatible.
            Type 'Animal' is not assignable to type 'Dog'.
              Type 'Animal' is not assignable to type '{ woof(): void; }'.
                Property 'woof' is missing in type 'Animal'.

I've confirmed that it also works as expected for the pluckFn example above.

@masaeedu masaeedu changed the title Granular inference for generic type parameters Granular inference for generic function type arguments Nov 18, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 20, 2017

This issue is about simply making existing syntax have different semantics. I don't want any new keywords or syntax (although some future additions could be made to make things clearer), I just want the syntactically valid pluckFuncs<{ a: () => string }>(['a']) to also be semantically valid, and not produce the "Expected 2 type arguments, got 1" error. What exactly the semantic change should be I've tried to outline above with a mapping to TypeScript code that is valid today.

this will be a breaking change for defaults. which might not be a bad break, but we will need to do additional analysis on our real-world code bases and see if any popular patterns that would be affected by this.

@insidewhy
Copy link

insidewhy commented May 6, 2018

With #23696 you can infer U in <T, U>(a: U) by using blah<T = string>(1). Seems a bit wordy though given blah<string>(4) is an error right now. Would much prefer it to be equivalent to the "named type" version.

@RyanCavanaugh
Copy link
Member

Looking at it again, I really don't see a difference between this and #10571 that doesn't imply a massive breaking change.

PluckFuncs can be written cromulently either in the presence of a true inference source (two type parameters on a single argument rarely makes sense), or as a curried function, or with #10571. I don't see any other path forward on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants