Skip to content

Commit

Permalink
Implement the type for the compose() function natively
Browse files Browse the repository at this point in the history
Summary:
`compose()` is a utility used a fair deal in the React community. Especially for projects that make heavy use of HOCs.

The `compose()` function is exported by [Redux](http://redux.js.org/docs/api/compose.html), [Recompose](https://github.com/acdlite/recompose/blob/master/docs/API.md#compose), Lodash (as [`flow`](https://lodash.com/docs/4.17.4#flow) and [`flowRight`](https://lodash.com/docs/4.17.4#flowRight)), and [Ramda](http://ramdajs.com/docs/#compose). All popular libraries in the JavaScript community.

Currently compose is typed with [multiple predicates](https://github.com/flowtype/flow-typed/blob/aff2bf770eeb3791aba0f231616a1872d0ffbcf5/definitions/npm/redux_v3.x.x/flow_v0.33.x-/redux_v3.x.x.js#L55-L104) or an [intersection](https://github.com/flowtype/flow-typed/blob/aff2bf770eeb3791aba0f231616a1872d0ffbcf5/definitions/npm/recompose_v0.24.x/flow_v0.47.x-v0.52.x/recompose_v0.24.x.js#L71-L127). This approach has multiple downsides. You may only support composing up to `n` functions, you cannot use spreads on arrays of unknown arity, and when using an intersection it is easy [to fall into a bad error message](https://github.com/istarkov/flow-compose-error). (The intersection error message may be a bug and should probably be improved anyway at some point, but I think the case for implementing `compose()` natively is stronger then just improving the error message.)

[istarkov](https://github.com/istarkov) also promised me a blog post if I implemented the type for compose natively.

Reviewed By: avikchaudhuri

Differential Revision: D5666855

fbshipit-source-id: 2d94c47df1d722933e21b0a693b0547958fe9efc
  • Loading branch information
Caleb Meredith authored and facebook-github-bot committed Aug 30, 2017
1 parent d2b8633 commit ab9bf44
Show file tree
Hide file tree
Showing 12 changed files with 316 additions and 19 deletions.
2 changes: 2 additions & 0 deletions src/typing/codegen.ml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ let rec gen_type t env = Type.(
| CustomFunT (_, ObjectAssign) -> add_str "Object$Assign" env
| CustomFunT (_, ObjectGetPrototypeOf) -> add_str "Object$GetPrototypeOf" env
| CustomFunT (_, ObjectSetPrototypeOf) -> add_str "Object$SetPrototypeOf" env
| CustomFunT (_, Compose false) -> add_str "$Compose" env
| CustomFunT (_, Compose true) -> add_str "$ComposeReverse" env
| CustomFunT (_, ReactPropType (React.PropType.Primitive (_, t))) ->
add_str "React$PropType$Primitive<" env
|> gen_type t
Expand Down
4 changes: 4 additions & 0 deletions src/typing/debug_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ and _json_of_custom_fun_kind kind = Hh_json.JSON_String (match kind with
| ObjectAssign -> "Object.assign"
| ObjectGetPrototypeOf -> "Object.getPrototypeOf"
| ObjectSetPrototypeOf -> "Object.setPrototypeOf"
| Compose false -> "Compose"
| Compose true -> "ComposeReverse"
| ReactPropType _ -> "ReactPropsCheckType"
| ReactCreateClass -> "React.createClass"
| ReactCreateElement -> "React.createElement"
Expand Down Expand Up @@ -1571,6 +1573,8 @@ and dump_t_ (depth, tvars) cx t =
| ObjectAssign -> "ObjectAssign"
| ObjectGetPrototypeOf -> "ObjectGetPrototypeOf"
| ObjectSetPrototypeOf -> "ObjectSetPrototypeOf"
| Compose false -> "Compose"
| Compose true -> "ComposeReverse"
| ReactPropType p -> spf "ReactPropType (%s)" (react_prop_type p)
| ReactCreateElement -> "ReactCreateElement"
| ReactCloneElement -> "ReactCloneElement"
Expand Down
121 changes: 102 additions & 19 deletions src/typing/flow_js.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3831,24 +3831,6 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
React.CreateClass (React.CreateClass.Spec [], knot, call_tout)));
Ops.set ops

(* Custom handling for React.createElement() *)
| CustomFunT (_, ReactCreateElement),
CallT (reason_op, { call_args_tlist = args; call_tout = tout; _ }) ->
resolve_call_list cx ~trace reason_op args (
ResolveSpreadsToCustomFunCall (mk_id (), ReactCreateElement, tout))

(* Custom handling for React.cloneElement() *)
| CustomFunT (_, ReactCloneElement),
CallT (reason_op, { call_args_tlist = args; call_tout = tout; _ }) ->
resolve_call_list cx ~trace reason_op args (
ResolveSpreadsToCustomFunCall (mk_id (), ReactCloneElement, tout))

(* Custom handling for React.createFactory() *)
| CustomFunT (_, ReactElementFactory component),
CallT (reason_op, { call_args_tlist = args; call_tout = tout; _ }) ->
resolve_call_list cx ~trace reason_op args (
ResolveSpreadsToCustomFunCall (mk_id (), ReactElementFactory component, tout))

| _, ReactKitT (reason_op, tool) ->
react_kit cx trace reason_op l tool

Expand Down Expand Up @@ -3886,10 +3868,19 @@ let rec __flow cx ((l: Type.t), (u: Type.use_t)) trace =
) call_args_tlist;
rec_flow_t cx trace (VoidT.why reason_op, call_tout);

| CustomFunT (_, (
Compose _
| ReactCreateElement
| ReactCloneElement
| ReactElementFactory _
as kind)),
CallT (reason_op, { call_args_tlist = args; call_tout = tout; _ }) ->
resolve_call_list cx ~trace reason_op args (
ResolveSpreadsToCustomFunCall (mk_id (), kind, tout))

| CustomFunT (reason, _), _ when function_like_op u ->
rec_flow cx trace (DefT (reason, AnyFunT), u)


(*********************************************)
(* object types deconstruct into their parts *)
(*********************************************)
Expand Down Expand Up @@ -9951,6 +9942,17 @@ and react_kit =
~filter_maybe

and custom_fun_call cx trace reason_op kind args spread_arg tout = match kind with
| Compose reverse ->
let tin = mk_tvar cx reason_op in
let tvar = mk_tvar cx reason_op in
run_compose cx trace reverse args spread_arg tin tvar;
let funt = FunT (
dummy_static reason_op,
dummy_prototype,
mk_functiontype [tin] ~rest_param:None ~def_reason:reason_op tvar
) in
rec_flow_t cx trace (DefT (reason_op, funt), tout)

| ReactCreateElement -> (match args with
(* React.createElement(component) *)
| component::[] ->
Expand Down Expand Up @@ -10043,6 +10045,87 @@ and custom_fun_call cx trace reason_op kind args spread_arg tout = match kind wi
| DebugPrint
-> failwith "implemented elsewhere"

(* Creates the appropriate constraints for the compose() function and its
* reversed variant. *)
and run_compose cx trace reverse fns spread_fn tin tout =
match reverse, fns, spread_fn with
(* Call the tail functions in our array first and call our head function
* last after that. *)
| false, fn::fns, _ ->
let reason = reason_of_t fn in
let tvar = mk_tvar_where cx reason (fun tvar ->
run_compose cx trace reverse fns spread_fn tin tvar) in
rec_flow cx trace (fn,
CallT (reason, mk_functioncalltype [Arg tvar] tout))

(* If the compose function is reversed then we want to call the tail
* functions in our array after we call the head function. *)
| true, fn::fns, _ ->
let reason = reason_of_t fn in
let tvar = mk_tvar_where cx reason (fun tvar ->
rec_flow cx trace (fn,
CallT (reason, mk_functioncalltype [Arg tin] tvar))) in
run_compose cx trace reverse fns spread_fn tvar tout

(* If there are no functions and no spread function then we are an identity
* function. *)
| _, [], None ->
rec_flow_t cx trace (tin, tout)

(* Correctly implementing spreads of unknown arity for the compose function
* is a little tricky. Let's look at a couple of cases.
*
* const fn = (x: number): string => x.toString();
* declare var fns: Array<typeof fn>;
* const x = 42;
* compose(...fns)(x);
*
* This would be invalid. We could have 0 or 1 fn in our fns array, but 2 fn
* would be wrong because string is incompatible with number. It breaks down
* as such:
*
* 1. x = 42
* 2. fn(x) = '42'
* 3. fn(fn(x)) is an error because '42' is not a number.
*
* To get an error in this case we would only need to call the spread
* argument twice. Now let's look at a case where things get recursive:
*
* type Fn = <O>(O) => $PropertyType<O, 'p'>;
* declare var fns: Array<Fn>;
* const x = { p: { p: 42 } };
* compose(...fns)(x);
*
* 1. x = { p: { p: 42 } }
* 2. fn(x) = { p: 42 }
* 3. fn(fn(x)) = 42
* 4. fn(fn(fn(x))) throws an error because the p property is not in 42.
*
* Here we would need to call fn 3 times before getting an error. Now
* consider:
*
* type Fn = <O>(O) => $PropertyType<O, 'p'>;
* declare var fns: Array<Fn>;
* type X = { p: X };
* declare var x: X;
* compose(...fns)(x);
*
* This is valid.
*
* To implement spreads in compose functions we first add a constraint based
* on tin and tout assuming that the spread is empty. Then we emit recursive
* constraints:
*
* spread_fn(tin) ~> tout
* spread_fn(tout) ~> tin
*
* The implementation of Flow should be able to terminate these recursive
* constraints. If it doesn't then we have a bug. *)
| _, [], Some spread_fn ->
run_compose cx trace reverse [] None tin tout;
run_compose cx trace reverse [spread_fn] None tin tout;
run_compose cx trace reverse [spread_fn] None tout tin

and object_spread =
let open ObjectSpread in

Expand Down
3 changes: 3 additions & 0 deletions src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,9 @@ module rec TypeTerm : sig
| ObjectGetPrototypeOf
| ObjectSetPrototypeOf

(* common community functions *)
| Compose of bool

(* 3rd party libs *)
| ReactPropType of React.PropType.t
| ReactCreateClass
Expand Down
5 changes: 5 additions & 0 deletions src/typing/type_annotation.ml
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,11 @@ let rec convert cx tparams_map = Ast.Type.(function
| "Object$SetPrototypeOf" ->
mk_custom_fun cx loc typeParameters ObjectSetPrototypeOf

| "$Compose" ->
mk_custom_fun cx loc typeParameters (Compose false)
| "$ComposeReverse" ->
mk_custom_fun cx loc typeParameters (Compose true)

| "React$PropType$Primitive" ->
check_type_param_arity cx loc typeParameters 1 (fun () ->
let t = convert_type_params () |> List.hd in
Expand Down
1 change: 1 addition & 0 deletions src/typing/type_normalizer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ let rec normalize_type_impl cx ids t = match t with

| FunProtoT _
| ExtendsT (_, _, _, _)
| CustomFunT (_, Compose _)
->
(** TODO **)
failwith (spf "Unsupported type in normalize_type_impl: %s" (string_of_ctor t))
Expand Down
2 changes: 2 additions & 0 deletions tests/compose/.flowconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[libs]
lib/
18 changes: 18 additions & 0 deletions tests/compose/basic.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// @flow

declare var compose: $Compose;
declare var composeReverse: $ComposeReverse;

(compose(n => n.toString())(42): empty); // Error: string ~> empty

(composeReverse(n => n.toString())(42): empty); // Error: string ~> empty

(compose(
n => n * 5, // Error: string cannot be multiplied.
n => n.toString(),
)(42): empty); // Error: number ~> empty

(composeReverse(
n => n * 5, // OK
n => n.toString(),
)(42): empty); // Error: string ~> empty
122 changes: 122 additions & 0 deletions tests/compose/compose.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
Error: basic.js:6
6: (compose(n => n.toString())(42): empty); // Error: string ~> empty
^ number. Could not resolve name

Error: basic.js:8
8: (composeReverse(n => n.toString())(42): empty); // Error: string ~> empty
^ number. Could not resolve name

Error: basic.js:10
v-------
10: (compose(
11: n => n * 5, // Error: string cannot be multiplied.
12: n => n.toString(),
13: )(42): empty); // Error: number ~> empty
----^ number. This type is incompatible with
13: )(42): empty); // Error: number ~> empty
^^^^^ empty

Error: basic.js:12
12: n => n.toString(),
^ number. Could not resolve name

Error: basic.js:17
17: n => n.toString(),
^ number. Could not resolve name

Error: recompose.js:23
23: c: Math.round(props.p), // Error: string ~> number
^^^^ identifier `Math`. Could not resolve name

Error: spread.js:8
v-------
8: (compose(
9: ...fns1,
10: )(42): empty); // Error: number ~> empty
----^ number. This type is incompatible with
10: )(42): empty); // Error: number ~> empty
^^^^^ empty

Error: spread.js:12
v-------
12: (compose(
13: ...fns1, // Error: string ~> number
14: )('foo'): empty); // Error: string ~> empty and number ~> empty
-------^ number. This type is incompatible with
14: )('foo'): empty); // Error: string ~> empty and number ~> empty
^^^^^ empty

Error: spread.js:12
v-------
12: (compose(
13: ...fns1, // Error: string ~> number
14: )('foo'): empty); // Error: string ~> empty and number ~> empty
-------^ string. This type is incompatible with
14: )('foo'): empty); // Error: string ~> empty and number ~> empty
^^^^^ empty

Error: spread.js:13
13: ...fns1, // Error: string ~> number
^^^^ string. This type is incompatible with the expected param type of
4: declare var fns1: Array<(number) => number>;
^^^^^^ number

Error: spread.js:16
v-------
16: (compose(
17: ...fns2, // Error: string ~> number
18: )(42): empty); // Error: number ~> empty and string ~> empty
----^ number. This type is incompatible with
18: )(42): empty); // Error: number ~> empty and string ~> empty
^^^^^ empty

Error: spread.js:16
v-------
16: (compose(
17: ...fns2, // Error: string ~> number
18: )(42): empty); // Error: number ~> empty and string ~> empty
----^ string. This type is incompatible with
18: )(42): empty); // Error: number ~> empty and string ~> empty
^^^^^ empty

Error: spread.js:17
17: ...fns2, // Error: string ~> number
^^^^ array
17: ...fns2, // Error: string ~> number
^^^^ string. This type is incompatible with the expected param type of
5: declare var fns2: Array<(number) => string>;
^^^^^^ number

Error: spread.js:21
v-------
21: (compose(
22: ...fns3, // Error: Cannot get p on number
23: )(x1): empty); // Error: number ~> empty and object ~> empty
----^ number. This type is incompatible with
23: )(x1): empty); // Error: number ~> empty and object ~> empty
^^^^^ empty

Error: spread.js:21
v-------
21: (compose(
22: ...fns3, // Error: Cannot get p on number
23: )(x1): empty); // Error: number ~> empty and object ~> empty
----^ object literal. This type is incompatible with
23: )(x1): empty); // Error: number ~> empty and object ~> empty
^^^^^ empty

Error: spread.js:22
22: ...fns3, // Error: Cannot get p on number
^^^^ number. Could not resolve name

Error: spread.js:27
v--------
27: ((compose(
28: ...fns3,
29: ))(x2): empty); // Error: object ~> empty
-----^ object type. This type is incompatible with
29: ))(x2): empty); // Error: object ~> empty
^^^^^ empty


Found 17 errors
3 changes: 3 additions & 0 deletions tests/compose/lib/recompose.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
declare module 'recompose' {
declare export var compose: $Compose;
}
25 changes: 25 additions & 0 deletions tests/compose/recompose.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @flow
*
* This test was taken from:
* https://github.com/istarkov/flow-compose-error
*/

import { compose } from 'recompose';

// shared code between bad/good Compose
type Comp<A> = (a: A) => void;
type HOC<A, B> = (a: Comp<A>) => Comp<B>;

function myEnhancer<A, B>(mapper: B => A): HOC<A, B> {
return (comp: Comp<A>) => (props: B) => comp(mapper(props));
}

const enhancer: HOC<*, { p: number, e: string }> = compose(
myEnhancer(props => ({
p: `${props.p * 3}`,
})),
myEnhancer(props => ({
c: Math.round(props.p), // Error: string ~> number
}))
);
Loading

3 comments on commit ab9bf44

@lewisf
Copy link

@lewisf lewisf commented on ab9bf44 Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you thank you thank you!

@jcready
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wasn't the documentation added in this commit?

@ryami333
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or any subsequent commit, for that matter?

Please sign in to comment.