facebook / flow

Adds static typing to JavaScript to improve developer productivity and code quality.
https://flow.org/
MIT License
22.09k stars 1.86k forks source link

Typecheck `compose` function #1477

Closed leoasis closed 7 years ago

leoasis commented 8 years ago

Just to avoid ambiguity: The compose function is a function that satisfies compose(f, g, h)(...args) == f(g(h(...args))).

How can I type check the compose function implementation with Flow? Since it could receive an arbitrary number of args, I was thinking on providing different signatures and provide types of all of them up to, say, 5 arguments, but not sure about the syntax for that.

This is what I tried, but it's not working:

declare function compose<T1>(
  f: (...args: any) => T1
): (...args: any) => T1

declare function compose<T1, T2>(
  f: (x: T1) => T2,
  g: (...args: any) => T1
): (x: T1) => T2

//... up to 5 args

export default function compose(...args) {
 // function implementation
}

Another approach that didn't work either was:

type Compose1<T1> = (
  f: (...args: any) => T1
) => (...args: any) => T1

type Compose2<T1, T2> = (
  f: (x: T1) => T2,
  g: (...args: any) => T1
) => (x: T1) => T2

//... up to 5 args

type Compose = Compose1 | Compose2 | Compose3 | Compose4 | Compose5;

const compose : Compose = (...funcs) => {
 // function impl
}

export default compose;

Any hints? Is this even supported?

alexeygolev commented 8 years ago

@leoasis Take a look at this typescript interfaces for ramda. There is a compose function that you can base your approach on. They use function overloading while as far as I understand it's recommended to use intersection type for flow. In fact you were close but you used union type for your composes

istarkov commented 8 years ago

Looks like that compose and similar functions with multiple typed arguments, is not possible declare in flow. @leoasis Have you found any solution?

alexeygolev commented 8 years ago

@istarkov check out ramda typings

istarkov commented 8 years ago

@alexeygolev Why do u write about rambda typings as typescript and flow is not the same things.

This and similar will not work in flow

        compose<V0, T1, T2>(fn1: (x: T1) => T2, fn0: (x0: V0) => T1): (x0: V0) => T2;
        compose<V0, V1, T1, T2>(fn1: (x: T1) => T2, fn0: (x0: V0, x1: V1) => T1): (x0: V0, x1: V1) => T2;
        compose<V0, V1, V2, T1, T2>(fn1: (x: T1) => T2, fn0: (x0: V0, x1: V1, x2: V2) => T1): (x0: V0, x1: V1, x2: V2) => T2;

as flow just get the first definition, also no luck with union/intersection types.

alexeygolev commented 8 years ago

@istarkov I meant this

istarkov commented 8 years ago

@alexeygolev Thank you, will try

istarkov commented 8 years ago

Looks like even ...rest: Array<void> should not help, here is simple non working example

declare function test<A, B>(a: A, b: B, ...rest: Array<void>) : B;
declare function test<A>(a: A, ...rest: Array<void>) : A;

const xxxx = test(1, 2); // flow detects that xxxx is a number
const yyyy = test(1); // flow detects that yyyy is void, as 1 declare used 
istarkov commented 8 years ago

oops this work

declare function test<A>(a: A, ...rest: Array<void>) : A;
declare function test<A, B>(a: A, b: B, ...rest: Array<void>) : B;

const xxxx = test(1, 2);
const yyyy = test(1);
istarkov commented 8 years ago

@alexeygolev Thank you a lot, all works fine!!!!

alexeygolev commented 8 years ago

@istarkov Let's first decide if we're talking about variadics in general or compose in particular

alexeygolev commented 8 years ago

@istarkov look through my tests for ramda in the same repo) might give you some more examples

istarkov commented 8 years ago

Looks like till now I start to live in ramda repo ;-)

leoasis commented 8 years ago

Nice! will check that out too then!

Macil commented 8 years ago

To be clear and to double-check my understanding: Flow doesn't currently support a way to make a correct type declaration for a compose function that can take any number of functions, right? I think that'd be a great feature for Flow.

alexeygolev commented 8 years ago

@AgentME do you know any type system (not only for JS) that can do it? Just to have a reference point

Macil commented 8 years ago

@alexeygolev No, I don't.

In https://github.com/facebook/flow/issues/1251#issuecomment-244284732, I proposed that the compose function might be popular enough to warrant its own special type like Promise.all currently has, and I also proposed an alternate more general solution involving a template with a special reduce type function that operates on the parameter types which could also be used to type functions like Promise.all.

nmn commented 7 years ago

You should use ...rest: [] instead of ...rest: Array<void>.

The latter allows undefined which can screw up arguments.length.

calebmer commented 7 years ago

Here is how you would type compose():

function compose<TValue, TArgs: Iterable<mixed>>(fn: (...TArgs) => TValue, ...args: TArgs): TValue {
  return fn(...args);
}

function myFn(a: number, b: string): string {
  return a + b;
}

const x: empty = compose(myFn, 'nope', 42);

Note that this would not work in TypeScript 😉

https://flow.org/try/#0GYVwdgxgLglg9mABBOBbADnAzgUwDwAqAagIYA2IOANIgQIIBOA5lgFyICSUODJARmXyoYADxwATAHySAFMDDsZAOhX1mWAJSIAvJNqkK1RCqUl17NSw0WDlRAG8AUIkQMcUEAyTzlKs1YBuRwBfR0dQSFgERFQATwAxMBkSdjAQVD4eGj52LCgGGDAma0Q8gqKHZ1d3TyQSRABqRD4g0McUMDzEEXYcDChYnWQ0TFwZOMSaAHIwOHQcKZoAFgAmDSCgA

Macil commented 7 years ago

@calebmer Sorry, that's a different compose function than this thread is about. This thread is about the compose function as defined in libraries like Ramda, transducers.js, and Lodash (as "flowRight"), which takes multiple functions and combines them into one function which calls all the given functions while passing each one's result as the next one's parameter.

R.compose(x => x+0.2, x => x*10, x => x+1)(5);
// 60.2
istarkov commented 7 years ago

@calebmer better to use ramda compose typedef from flowtyped, mixed is bad idea, type inference would not work, as an example recompose https://github.com/acdlite/recompose/tree/master/types use it

istarkov commented 7 years ago

BTW I found strange behavior of compose https://github.com/facebook/flow/issues/4342 Some errors not detected if compose defined in libdef file

calebmer commented 7 years ago

@AgentME sorry for the misunderstanding 😔

I recommend using function overloading in this case as is suggested by @istarkov. I’m going to be looking at typing Redux soon and if it makes sense then we may add a special case for compose().

@istarkov I’ll comment in #4342 👍

FezVrasta commented 6 years ago

Hey @calebmer did you end up with a solution for the compose use case?

TrySound commented 6 years ago

@FezVrasta $Compose and $ComposeReverse exist probably about ten versions.

FezVrasta commented 6 years ago

Oh thanks, sorry I did miss that.