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

inferred generic type param of higher order function is too wide #29873

Closed
ferdaber opened this issue Feb 12, 2019 · 10 comments
Closed

inferred generic type param of higher order function is too wide #29873

ferdaber opened this issue Feb 12, 2019 · 10 comments

Comments

@ferdaber
Copy link

ferdaber commented Feb 12, 2019

TypeScript Version: typescript@next

Search Terms: higher order function component generic inference

Code

declare function higherOrderFunction<
  F extends (options: BaseOptions) => any,
  O extends BaseOptions
>(fn: F): F
declare function fn(options: { foo: string; baz: number }): any
const test = higherOrderFunction(fn) // type error

Expected behavior:
Generic params of higherOrderFunction are inferred to be typeof fn and BaseOptions & { bar: string }, respectively.

Actual behavior:
Type error is resulted because generic params of higherOrderFunction are inferred to be (options: BaseOptions) => any and BaseOptions, respectively:

Argument of type '(options: { foo: string; baz: number; }) => any' is not assignable to parameter of type '(options: BaseOptions) => any'.
  Types of parameters 'options' and 'options' are incompatible.
    Property 'baz' is missing in type 'BaseOptions' but required in type '{ foo: string; baz: number; }'

Playground Link: link

Related Issues: Not that I saw, but this relates closely to how React HOCs are constructed, where we require the type of the component itself to resolve default props and prop types using JSX.LibraryManagedAttributes.

Current workaround:
Kinda gross but using a conditional type that references the the generic type parameter inside its own constraint works, which feels very hacky and weird to me??

declare function higherOrderFunction<
  F extends (options: BaseOptions & (F extends (options: infer O) => any ? O : {})) => any
>(fn: F): F
@dragomirtitian
Copy link
Contributor

dragomirtitian commented Feb 12, 2019

I think this is the expected behavior.

With strictFunctionTypes (as without this option the error does not appear) function parameter types checked contravariantly (#18654). This means that:

class Animal { a: any }
class Dog extends Animal { d: any }

type Y = ((a: Animal) => void) extends ((a: Dog) => void) ? "Y" : "N"
type N = ((a: Dog) => void) extends ((a: Animal) => void) ? "Y" : "N" 

So basically if the parameter is of a derived type the relationship is inversed, the function with the base type argument (Animal) is a subtype of the function with a derived argument (Dog)

So this would actually work, as typeof fn is a subtype of (options: BaseOptions) => any :

interface BaseOptions {
  foo: string,
  baz: number
}

declare function higherOrderFunction<
  F extends (options: BaseOptions) => any,
  O extends BaseOptions
>(fn: F): (options: BaseOptions & { bar: string }) => any
declare function fn(options: { foo: string }): any
const test = higherOrderFunction(fn)

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Feb 12, 2019

One more point. Your workaround removes the constraint that the parameter be derived from BaseOptions, it not any better than declare function higherOrderFunction<F extends (options: any) => any>(fn: F): F

A workaround that preserves both the restriction and the full function type could be:

declare function higherOrderFunction<F extends (options: O) => any, O extends BaseOptions>(fn: F & ((o: O) => any)): F


declare function fn(options: { foo: string; baz: number }): any
const test = higherOrderFunction(fn)

declare const fnWithProps: {
  (options: { foo: string; baz: number }): any
  defaultProps: {
    foo: string
  }
}
const testWithProps = higherOrderFunction(fnWithProps) //  { (options: { foo: string; baz: number; }): any; defaultProps: { foo: string; }; }

And you could pick the extra properties from the function, and modify the signature if need. For example:

    interface BaseOptions {
      foo: string
    }
    type Omit<T, K extends keyof T> = Pick<T, Exclude<keyof T, K>> 
    declare function higherOrderFunction<F extends (options: O) => any, O extends BaseOptions>(fn: F & ((o: O) => any)): Pick<F, keyof F> & ((o: Omit<O, 'foo'>) => ReturnType<F>)


    declare function fn(options: { foo: string; baz: number }): any
    const test = higherOrderFunction(fn)

    declare const fnWithProps: {
      (options: { foo: string; baz: number }): any
      defaultProps: {
        foo: string
      }
    }
    const testWithProps = higherOrderFunction(fnWithProps)

@ferdaber
Copy link
Author

Yes sorry I mistyped the workaround, it needs the additional intersection against the BaseProps type.

I looked at your first response and I don't see the signature of higherOrderFunction being different than my example, other than the return type (which I did change after the fact in my example to reduce ambiguity).

Issue here is that explicitly providing the generic type params still satisfied the constraints, but I wish that the inference engine would have inferred that type right away instead of prioritizing the wider base type:

// this works
const test = higherOrderFunction<typeof fn, BaseOptions & { bar: string }>(fn)

@ferdaber
Copy link
Author

ferdaber commented Feb 12, 2019

With the upcoming partial inference feature in 3.4 the pain may be lessened by doing this, but still doesn't resolve the issue fully:

const test = higherOrderFunction<typeof fn, _>(fn)

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Feb 12, 2019

@ferdaber My point is that it is not a priority issue. The way function types work (under strictFunctionTypes), any function type that has a parameter that is a subtype of BaseOptions will not be a subtype of (o:BaseOptions)=>any. Since parameter types relate contravariantly the type relationship is inversed. This means that (o:BaseOptions & { otherProp: any })=>any is not a subtype of (o:BaseOptions)=>any and thus is not valid as the type parameter F.

The example demonstrated that reverse is true, that if F is constrained to for example (o:BaseOptions & { otherProp: any })=>any, and you use as an argument a function of type (o:BaseOptions)=>any, F will be inferred to (o:BaseOptions)=>any since (o:BaseOptions)=>any is a subtype of (o:BaseOptions & { otherProp: any })=>any

@ferdaber
Copy link
Author

Ok, yeah that's fair, this would not work:

const test = higherOrderFunction<typeof fn, BaseOptions>(fn)

Since we're contravariant for function parameter types.

What, then, would be the construct and ordering of generic type params such that we use O for covariant type checking, and F for inferring the actual parameter's type?

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Feb 12, 2019

@ferdaber I think the version in my second comment does a pretty good job

declare function higherOrderFunction<F extends (options: O) => any, O extends BaseOptions>(fn: F & ((o: O) => any)): F

If gives the compiler a site to infer O from by using the & ((o: O) => any) in the parameter type, but F will be inferred to the full function type (ie signature and any extra props). O will accept any type that extends BaseOptions and since F is tied to the O actually inferred at call site it does not reject functions for the covariant parameter type reason stated before.

BTW, this: const test = higherOrderFunction<typeof fn, BaseOptions>(fn) also fails with your original signature you just get an error on typeof fn instead of the argument. (under strictFunctionTypes)

@ferdaber
Copy link
Author

ferdaber commented Feb 12, 2019

Ah! I like that approach, sorry it was difficult to grok at first and understand why this is happening, but I see why that works now.

I think this signature below is even simpler and still works; do you see any reason why this would fail?

declare function higherOrderFunction<O extends BaseOptions, F>(fn: ((options: O) => any) & F): F

This makes it somewhat more explicit that F is simply grabbing the full type of fn and not really being used for constraint checking.

@dragomirtitian
Copy link
Contributor

@ferdaber Yeah I think you are right, I just left in the original constraint and didn't think about removing it, but your version looks cleaner. I believe it will work just as well.

@ferdaber
Copy link
Author

Wonderful! This is awesome. Thanks so much for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants