Zero cost bindings for style merge (array/list) (facebook#347)

* Zero cost bindings for style merge (array/list) RN accept works out of the box array of styles (object|int from StyleSheet entries). It also recursively resolve the array of styles, so list should work too (as list are just recursive array). I tried to find "smart" name but at the end, using merge/flatten/combine/concat just make this more confusing. I am deprecating combine/merge as this is totally unsafe IRL (you cannot merge object of style with a stylesheet entry, which works with current types...) Thoughts? Idea for better naming? * bs-react-native-next: Remove StyleSheet.flatten in Style module * Zero cost bindings for style merge (array/list) fixup
jfrolich · Mar 23, 2019 · 4001fd1 · 4001fd1
1 parent 67ae8f5
commit 4001fd1
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 61 deletions.
diff --git a/bs-react-native-example/src/pages/ViewExample.re b/bs-react-native-example/src/pages/ViewExample.re
@@ -106,7 +106,7 @@ module ZIndexExample = {
             {ReasonReact.string("Tap to flip sorting order")}
           </Text>
           <View
-            style={fromList([
+            style={list([
               styles##zIndex,
               style([
                 marginTop(Pt(0.)),
@@ -117,7 +117,7 @@ module ZIndexExample = {
             <Text> {ReasonReact.string(zIndexStr(0))} </Text>
           </View>
           <View
-            style={fromList([
+            style={list([
               styles##zIndex,
               style([
                 marginLeft(Pt(50.)),
@@ -128,7 +128,7 @@ module ZIndexExample = {
             <Text> {ReasonReact.string(zIndexStr(1))} </Text>
           </View>
           <View
-            style={fromList([
+            style={list([
               styles##zIndex,
               style([
                 marginLeft(Pt(100.)),
@@ -139,7 +139,7 @@ module ZIndexExample = {
             <Text> {ReasonReact.string(zIndexStr(2))} </Text>
           </View>
           <View
-            style={fromList([
+            style={list([
               styles##zIndex,
               style([
                 marginLeft(Pt(150.)),
@@ -204,19 +204,18 @@ let examples: array(Example.t) =
               borderColor(String("#bb0000")),
               borderWidth(1.),
             ])}>
-            <View
-              style={fromList([styles##box, style([padding(Pt(5.))])])}>
+            <View style={list([styles##box, style([padding(Pt(5.))])])}>
               <Text style={style([fontSize(Float(11.))])}>
                 {ReasonReact.string("5px padding")}
               </Text>
             </View>
-            <View style={fromList([styles##box, style([margin(Pt(5.))])])}>
+            <View style={list([styles##box, style([margin(Pt(5.))])])}>
               <Text style={style([fontSize(Float(11.))])}>
                 {ReasonReact.string("5px margin")}
               </Text>
             </View>
             <View
-              style={fromList([
+              style={list([
                 styles##box,
                 style([
                   margin(Pt(5.)),

diff --git a/bs-react-native-next/src/Style.re b/bs-react-native-next/src/Style.re
@@ -252,6 +252,39 @@ external style:
   t =
   "";
 
-// Duplicated from StyleSheet.re for convenience.
-[@bs.module "react-native"] [@bs.scope "StyleSheet"]
-external flatten: array(t) => t = "";
+/*
+ <View style=array([|
+   styles##thing,
+   styles##whatever,
+ |])>
+   */
+external array: array(t) => t = "%identity";
+
+/*
+ <View style=arrayOption([|
+   Some(styles##thing),
+   Some(styles##whatever),
+   optionalStyle,
+   cond ? Some({something:"dynamic"}) : None
+ |])>
+ */
+external arrayOption: array(option(t)) => t = "%identity";
+
+/* list works too since RN accept recursive array of styles (list are just recursive arrays)*/
+/*
+ <View style=list([
+   styles##thing,
+   styles##whatever,
+ ])>
+   */
+external list: list(t) => t = "%identity";
+
+/*
+ <View style=listOption([
+   Some(styles##thing),
+   Some(styles##whatever),
+   optionalStyle,
+   cond ? Some({something:"dynamic"}) : None
+ ])>
+ */
+external listOption: list(option(t)) => t = "%identity";
diff --git a/bs-react-native/src/style.bs.js b/bs-react-native/src/style.bs.js
diff --git a/bs-react-native/src/style.re b/bs-react-native/src/style.re
@@ -81,27 +81,23 @@ let objectStyle = (key, value) => (key, Internals.Encoder.object_(value));
 let arrayStyle = (key, value) => (key, Internals.Encoder.array(value));
 
 let style = sarr => sarr |> Js.Dict.fromList |> to_style;
-let emptyStyle = Js.Dict.empty()->to_style;
 
-external fromArray: array(t) => t = "%identity";
-let fromList = styles => styles->Belt.List.toArray->arrayOfStyle;
-let merge = (a, b) => {
+external array: array(t) => t = "%identity";
+external arrayOption: array(option(t)) => t = "%identity";
+external list: list(t) => t = "%identity";
+external listOption: list(option(t)) => t = "%identity";
+
+/* deprecated */
+let flatten = array;
+let concat = styles => styles->Belt.List.toArray->arrayOfStyle;
+let combine = (a, b) => {
   let entries =
     Array.append(
       Js.Dict.entries(style_to_dict(a)),
       Js.Dict.entries(style_to_dict(b)),
     );
   Js.Dict.fromArray(entries) |> to_style;
 };
-let mergeOptional = (s, so) =>
-  so->Belt.Option.map(so => s->merge(so))->Belt.Option.getWithDefault(s);
-let optional = s => s->Belt.Option.getWithDefault(emptyStyle);
-
-/* deprecated */
-let flatten = fromArray;
-let concat = fromList;
-let combine = merge;
-let combineOptional = mergeOptional;
 
 /***
  * Layout Props

diff --git a/bs-react-native/src/style.rei b/bs-react-native/src/style.rei
@@ -25,20 +25,48 @@ type deg_animated('a) =
   | Animated(AnimatedRe.value('a));
 
 let style: list(styleElement) => t;
-external fromArray: array(t) => t = "%identity";
-let fromList: list(t) => t;
-let merge: (t, t) => t;
-let mergeOptional: (t, option(t)) => t;
-let optional: option(t) => t;
 
-[@deprecated "use Style.fromArray([|t|]) instead"]
+/*
+ <View style=array([|
+   styles##thing,
+   styles##whatever,
+ |])>
+   */
+let array: array(t) => t;
+/*
+ <View style=arrayOption([|
+   Some(styles##thing),
+   Some(styles##whatever),
+   optionalStyle,
+   cond ? Some({something:"dynamic"}) : None
+ |])>
+ */
+let arrayOption: array(option(t)) => t;
+/* list works too since RN accept recursive array of styles (list are just recursive arrays)*/
+/*
+ <View style=list([
+   styles##thing,
+   styles##whatever,
+ ])>
+   */
+let list: list(t) => t;
+/*
+ <View style=listOption([
+   Some(styles##thing),
+   Some(styles##whatever),
+   optionalStyle,
+   cond ? Some({something:"dynamic"}) : None
+ ])>
+ */
+let listOption: list(option(t)) => t;
+[@deprecated "Use Style.array([|t|]) instead"]
 let flatten: array(t) => t;
-[@deprecated "use Style.merge(t, t) instead"]
-let combine: (t, t) => t;
-[@deprecated "use Style.fromList([t]) instead"]
+[@deprecated "Use Style.list([t]) instead"]
 let concat: list(t) => t;
-[@deprecated "use Style.mergeOptional(t, option(t)) instead"]
-let combineOptional: (t, option(t)) => t;
+[@deprecated
+  "This method is unsafe as it doesn't work well with StyleSheet values."
+]
+let combine: (t, t) => t;
 
 type alignContent =
   | FlexStart