From 0e7d3850712bc10c9b6b6a9d54c8ee4426a8cafb Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Sat, 9 Nov 2019 23:23:34 +0100
Subject: [PATCH 1/6] add failing test

---
 test/Components.re | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/test/Components.re b/test/Components.re
index 176716e..cab7657 100644
--- a/test/Components.re
+++ b/test/Components.re
@@ -237,4 +237,16 @@ module EmptyComponentWithOnMountEffect = {
 module ShouldAllowComponentProp = {
   let%component make = (~component, (), hooks) =>
     (<Div> component </Div>, hooks);
-}
\ No newline at end of file
+}
+
+module PartiallyAppliedComponent = {
+  let%component make = ((), ~arg: string, hooks) =>
+    (<Text title=arg />, hooks);
+};
+
+module PartiallyAppliedComponentConsumer = {
+  let%component make = ((), hooks) => {
+    let comp = <PartiallyAppliedComponent />;
+    (comp(~arg="foo"), hooks);
+  }
+};

From 6bb41ce930c76f05940f91aa6b97e1a064025cae Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Sat, 9 Nov 2019 23:53:48 +0100
Subject: [PATCH 2/6] allow labelled arguments after ()

---
 jsx/brisk_jsx.ml | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/jsx/brisk_jsx.ml b/jsx/brisk_jsx.ml
index 5e369c5..e2be13b 100644
--- a/jsx/brisk_jsx.ml
+++ b/jsx/brisk_jsx.ml
@@ -91,24 +91,33 @@ module Declaration_ppx = struct
         "nativeComponent"
 
   let transform_component_expr ~useDynamicKey ~attribute ~component_name expr =
-    let rec map_component_expression ({P.pexp_loc= loc} as expr) =
-      match_ func_pattern loc expr
-        ~with_:(fun lbl opt_arg pat child_expression ->
-          let make_fun_with_expr ~expr =
-            Ast_builder.pexp_fun ~loc lbl opt_arg pat expr
-          in
-          let loc = pat.Ppxlib.ppat_loc in
-          match (lbl, pat) with
-          | (Ppxlib.Labelled _ | Optional _), _ ->
-              make_fun_with_expr
-                ~expr:(map_component_expression child_expression)
-          | Ppxlib.Nolabel, [%pat? ()] ->
-              let loc = child_expression.pexp_loc in
-              make_fun_with_expr
-                ~expr:[%expr [%e component_ident ~loc] ~key [%e child_expression]]
-          | _ ->
-              Location.raise_errorf ~loc
-                "A labelled argument or () was expected")
+    let map_component_expression expr =
+      let rec loop ~seenUnit ({P.pexp_loc= loc} as expr) = 
+        try
+          match_ func_pattern loc expr
+            ~with_:(fun lbl opt_arg pat child_expression ->
+              let make_fun_with_expr ~expr =
+                Ast_builder.pexp_fun ~loc lbl opt_arg pat expr
+              in
+              let loc = pat.Ppxlib.ppat_loc in
+              match (lbl, pat) with
+              | (Ppxlib.Labelled _ | Optional _), _ ->
+                  make_fun_with_expr
+                    ~expr:(loop ~seenUnit child_expression)
+              | Ppxlib.Nolabel, [%pat? ()] ->
+                  make_fun_with_expr
+                    ~expr:(loop ~seenUnit:true child_expression)
+              | _ ->
+                if seenUnit then
+                  let loc = child_expression.pexp_loc in
+                  [%expr [%e component_ident ~loc] ~key [%e make_fun_with_expr ~expr:child_expression]]
+                else
+                  Location.raise_errorf ~loc
+                    "A labelled argument or () was expected")
+        with
+        | _ -> [%expr [%e component_ident ~loc] ~key [%e expr]]
+      in
+      loop ~seenUnit:false expr
     in
     let open P in
     let loc = expr.P.pexp_loc in

From cd70a90083807441c7de45c915ab31ee61b60136 Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Mon, 11 Nov 2019 13:47:05 +0100
Subject: [PATCH 3/6] disallow optional args after unit

---
 jsx/brisk_jsx.ml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/jsx/brisk_jsx.ml b/jsx/brisk_jsx.ml
index e2be13b..3b49058 100644
--- a/jsx/brisk_jsx.ml
+++ b/jsx/brisk_jsx.ml
@@ -100,7 +100,9 @@ module Declaration_ppx = struct
                 Ast_builder.pexp_fun ~loc lbl opt_arg pat expr
               in
               let loc = pat.Ppxlib.ppat_loc in
-              match (lbl, pat) with
+          | (Ppxlib.Optional _), _ when seenUnit ->
+              Location.raise_errorf ~loc
+                "Optional arguments not allowed after ()"
               | (Ppxlib.Labelled _ | Optional _), _ ->
                   make_fun_with_expr
                     ~expr:(loop ~seenUnit child_expression)

From 93ca44dc2f25444a33b9c9e1ce9423a4530ee40e Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Mon, 11 Nov 2019 13:47:20 +0100
Subject: [PATCH 4/6] don't swallow all errors maybe

---
 jsx/brisk_jsx.ml | 48 +++++++++++++++++++++---------------------------
 1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/jsx/brisk_jsx.ml b/jsx/brisk_jsx.ml
index 3b49058..f6637fc 100644
--- a/jsx/brisk_jsx.ml
+++ b/jsx/brisk_jsx.ml
@@ -79,11 +79,6 @@ module JSX_ppx = struct
 end
 
 module Declaration_ppx = struct
-  let func_pattern = Ppxlib.Ast_pattern.(pexp_fun __ __ __ __)
-
-  let match_ pattern ?on_error loc ast_node ~with_ =
-    Ppxlib.Ast_pattern.parse pattern ?on_error loc ast_node with_
-
   let attribute_name = function
     | `Component ->
         "component"
@@ -92,31 +87,30 @@ module Declaration_ppx = struct
 
   let transform_component_expr ~useDynamicKey ~attribute ~component_name expr =
     let map_component_expression expr =
-      let rec loop ~seenUnit ({P.pexp_loc= loc} as expr) = 
-        try
-          match_ func_pattern loc expr
-            ~with_:(fun lbl opt_arg pat child_expression ->
-              let make_fun_with_expr ~expr =
-                Ast_builder.pexp_fun ~loc lbl opt_arg pat expr
-              in
-              let loc = pat.Ppxlib.ppat_loc in
+      let rec loop ~seenUnit ({P.pexp_loc=loc; pexp_desc} as expr) = 
+        match pexp_desc with
+        | P.Pexp_fun (lbl, opt_arg, pat, child_expression) ->
+          let make_fun_with_expr ~expr =
+            Ast_builder.pexp_fun ~loc lbl opt_arg pat expr
+          in
+          let loc = pat.Ppxlib.ppat_loc in
+          ( match (lbl, pat) with
           | (Ppxlib.Optional _), _ when seenUnit ->
               Location.raise_errorf ~loc
                 "Optional arguments not allowed after ()"
-              | (Ppxlib.Labelled _ | Optional _), _ ->
-                  make_fun_with_expr
-                    ~expr:(loop ~seenUnit child_expression)
-              | Ppxlib.Nolabel, [%pat? ()] ->
-                  make_fun_with_expr
-                    ~expr:(loop ~seenUnit:true child_expression)
-              | _ ->
-                if seenUnit then
-                  let loc = child_expression.pexp_loc in
-                  [%expr [%e component_ident ~loc] ~key [%e make_fun_with_expr ~expr:child_expression]]
-                else
-                  Location.raise_errorf ~loc
-                    "A labelled argument or () was expected")
-        with
+          | (Ppxlib.Labelled _ | Optional _), _ ->
+              make_fun_with_expr
+                ~expr:(loop ~seenUnit child_expression)
+          | Ppxlib.Nolabel, [%pat? ()] ->
+              make_fun_with_expr
+                ~expr:(loop ~seenUnit:true child_expression)
+          | _ ->
+            if seenUnit then
+              let loc = child_expression.pexp_loc in
+              [%expr [%e component_ident ~loc] ~key [%e make_fun_with_expr ~expr:child_expression]]
+            else
+              Location.raise_errorf ~loc
+                "A labelled argument or () was expected")
         | _ -> [%expr [%e component_ident ~loc] ~key [%e expr]]
       in
       loop ~seenUnit:false expr

From 83e74c57f1dc074f06cf593f9817d2276e322f3e Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Wed, 13 Nov 2019 13:07:18 +0100
Subject: [PATCH 5/6] Better, typed example

---
 test/Components.re | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/test/Components.re b/test/Components.re
index cab7657..8206074 100644
--- a/test/Components.re
+++ b/test/Components.re
@@ -239,14 +239,28 @@ module ShouldAllowComponentProp = {
     (<Div> component </Div>, hooks);
 }
 
-module PartiallyAppliedComponent = {
-  let%component make = ((), ~arg: string, hooks) =>
-    (<Text title=arg />, hooks);
+module PartiallyAppliedComponent : {
+  type t;
+  type dispatcher = int => unit;
+  let make: (~key:int=?, ~title: string, unit) => t;
+  let render: (dispatcher, t) => element(node);
+} = {
+  type dispatcher = int => unit;
+  type t = (~dispatch:dispatcher) => element(node);
+
+  let%component make = (~title, (), ~dispatch as _:dispatcher, hooks) =>
+    (<Text title />, hooks);
+
+  let render = (dispatch, component) =>
+    component(~dispatch);
 };
 
-module PartiallyAppliedComponentConsumer = {
+module PartiallyAppliedComponentConsumer : {
+  let make: (~key:int=?, unit) => element(node);
+} = {
   let%component make = ((), hooks) => {
-    let comp = <PartiallyAppliedComponent />;
-    (comp(~arg="foo"), hooks);
+    let dispatch = _ => ();
+    let comp = <PartiallyAppliedComponent title="" />;
+    (PartiallyAppliedComponent.render(dispatch, comp), hooks);
   }
 };

From c7dd002118bf47620deeafb8a4a4b15a2c1c5d32 Mon Sep 17 00:00:00 2001
From: glennsl <glenn@slotte.net>
Date: Wed, 13 Nov 2019 14:04:46 +0100
Subject: [PATCH 6/6] formatting

---
 test/Components.re | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/test/Components.re b/test/Components.re
index 8206074..cddbc98 100644
--- a/test/Components.re
+++ b/test/Components.re
@@ -235,32 +235,35 @@ module EmptyComponentWithOnMountEffect = {
 };
 
 module ShouldAllowComponentProp = {
-  let%component make = (~component, (), hooks) =>
-    (<Div> component </Div>, hooks);
-}
+  let%component make = (~component, (), hooks) => (
+    <Div> component </Div>,
+    hooks,
+  );
+};
 
-module PartiallyAppliedComponent : {
+module PartiallyAppliedComponent: {
   type t;
   type dispatcher = int => unit;
-  let make: (~key:int=?, ~title: string, unit) => t;
+  let make: (~key: int=?, ~title: string, unit) => t;
   let render: (dispatcher, t) => element(node);
 } = {
   type dispatcher = int => unit;
-  type t = (~dispatch:dispatcher) => element(node);
+  type t = (~dispatch: dispatcher) => element(node);
 
-  let%component make = (~title, (), ~dispatch as _:dispatcher, hooks) =>
-    (<Text title />, hooks);
+  let%component make = (~title, (), ~dispatch as _: dispatcher, hooks) => (
+    <Text title />,
+    hooks,
+  );
 
-  let render = (dispatch, component) =>
-    component(~dispatch);
+  let render = (dispatch, component) => component(~dispatch);
 };
 
-module PartiallyAppliedComponentConsumer : {
-  let make: (~key:int=?, unit) => element(node);
+module PartiallyAppliedComponentConsumer: {
+  let make: (~key: int=?, unit) => element(node);
 } = {
   let%component make = ((), hooks) => {
     let dispatch = _ => ();
     let comp = <PartiallyAppliedComponent title="" />;
     (PartiallyAppliedComponent.render(dispatch, comp), hooks);
-  }
-};
+  };
+};
\ No newline at end of file