Replies: 19 comments 10 replies
-
@glennsl thanks so much for such detailed description of the concerns and ideas. In general, I agree with your concerns, and I think there should be a less intrusive way to solve lowercase elements creation, hopefully without losing too much ergonomics in the way.
I think you nailed in this paragraph the crux of the issue. Essentially using plain functions with labelled arguments is not an option because given the amount of attributes that each element has (even if there was a separate function for each element), the number of zeroes in the optional params that are erased would have a lot of impact on the resulting bundle size. This is the exact same problem that happens with I like the idea of using let t = h "a" [%props { href = "https://foo.com"; target = "_blank" }]
let t = h "a" [%props [ href, "https://foo.com"; target, "_blank" ]] i think it can also be a sequence of tuples...quite the hack 😄 let t =
h "a"
[%props
href, "https://foo.com";
target, "_blank"] In any case, any of the suggested solutions (except the last one) would still require the ppx to check every function application ast node ( For completion, there's an additional approach that could be to use functions all the way down. And for props, pass a list where each prop is created with another small helper function, e.g. For custom elements, there could be a custom constructor: That way, at least for OCaml syntax, the only preprocessing required would be to define uppercase components. This has a lot of upsides on the maintainability and error handling side, as you well mentioned. Below is the snippet used in the proposal, with the "only functions" approach. The api is inspired from bucklescript-tea. let t =
React.Fragment.make
[
a
[ href "https://github.com/jihchi/jsoo-react-realworld-example-app"; target "_blank" ]
[ i [ className "ion-social-github" ] []; React.string "Fork on GitHub" ];
footer
[
div
[ className "container" ]
[
Link.make ~onClick:(Link.location Link.home) ~className:"logo-font" [ React.string "conduit" ];
span
[ className "attribution" ]
[
React.string "An interactive learning project from ";
a [ href "https://thinkster.io" ] [ React.string "Thinkster" ];
React.string ". Code & design licensed under MIT.";
];
];
];
] I don't think that for a first version we should cover all elements. For example, The cons of this proposal are shared with some of the ones mentioned in "Props syntax extension":
But I believe this syntax would be more familiar for users, and it's less noisy. What do you think? |
Beta Was this translation helpful? Give feedback.
-
One extra thing about skipping ppx usage is that things like documentation would become trivial, which I know @davesnx has been fighting with as part of #95 explorations. |
Beta Was this translation helpful? Give feedback.
-
Here's a small proof of concept, seems to work fine: #115. |
Beta Was this translation helpful? Give feedback.
-
Wow, that's so much simpler! And seems really obvious in retrospect, but then that's how these things usually go isn't it. There's still a few more downsides with this that I think is worth mentioning for a proper comparison though:
|
Beta Was this translation helpful? Give feedback.
-
I think that's reasonable, since it would be very easy to define your own, and then upstream them gradually.
I think that's absolutely fine too, yeah. |
Beta Was this translation helpful? Give feedback.
-
With some minor fixes, it actually is valid. You can use a tuple directly in extension points. See https://astexplorer.net/#/gist/844f0950b7ddb4ba13f4ff2e45946e96/730abae12847cb04af0bfa9b7d5a0f675d255145 |
Beta Was this translation helpful? Give feedback.
-
Some comments:
Actually this could be seen as an advantage 😄 . Yesterday, when looking at the element tree I realized it was hard to differentiate between children and props. By using an array, the distinction is (visually) much more obvious without the need of using noisier solutions (like adding labelled argument let%component make () =
Fragment.make
[
a
[|
href "https://github.com/jihchi/jsoo-react-realworld-example-app";
target "_blank";
|]
[
i [| className "ion-social-github" |] [];
React.string "Fork on GitHub";
];
footer [||]
[
div
[| className "container" |]
[
span
[| className "attribution" |]
[
React.string "An interactive learning project from ";
a
[| href "https://thinkster.io" |]
[ React.string "Thinkster" ];
React.string ". Code & design licensed under MIT.";
];
];
];
]
Yeah. We can go more or less granular as we want (e.g. one module with everything, subset by html or svg, etc). Probably when using it we will realize more what's needed and what's a reasonable way to split them. I think a module with "everything inside" will always be useful, the over time it can be polished as needed.
Yeah, we can do that, but it would add noise to all cases. I wonder if in these cases it'd be easier to just redefine functions (initially in user land) by using Another thing is that for 1-attribute-elements we would create a list or array unnecessarily. In e.g. bonsai they solve this using
I think it is becoming clear from your thoughts that it might make sense to split Html and Svg in separate modules.
The inlining concern is a really good one. I will try to test, including latest changes in jsoo about inlining. |
Beta Was this translation helpful? Give feedback.
-
If we put them into separate modules ( For example: open Svg
style [| className "foo"; Props.style "bar" |] [ ... ]
I've seen other libraries (in languages that don't have labeled arguments) define separate prop-less functions using a trailing underscore as the convention. E.g. I'd go with the simplest option for now, just leaving it as a mandatory unlabeled argument, as it seems like premature optimization at this point. If over time we find that it's too noisy or that there's to much overhead to creating lots of empty arrays, we can revisit. It should be fairly easy to migrate if we use the array syntax for props. It's good to get all these considerations and alternatives on the table early though, so we can keep them in the back of our minds as we try this out.
Yes, but even more important to split props and elements I think, as those are considered separate namespaces, more likely to be used together and to need qualification to disambiguate. As mentioned above I think |
Beta Was this translation helpful? Give feedback.
-
I did some experiments myself on top of your POC, by the way. If you want to have a look, it's here. |
Beta Was this translation helpful? Give feedback.
-
This post makes a good point - PPXes without intuitive semantics is one of the reasons why they are frowned upon. I like the idea of hyperscript. If we turn it into an extension point we can arrive at an API that's both lightweight, works well with autoformatting and supports non-standard props. I borrowed @glennsl 's example and tweaked it a little bit. Here's how it looks like auto formatted with ocamlformat. let x =
React.Fragment.make
[
[%h a] ~href:"https://github.com/jihchi/jsoo-react-realworld-example-app"
("whatever" = "whatever")
("@onwhatever" = fun _e -> ())
~target:"_blank"
[ [%h i] ~className:"ion-social-github"; "Fork on GitHub" ];
[%h footer]
[
[%h div] ~className:"container"
[
Link.make ~onClick:(Link.location Link.home)
~className:"logo-font" [ "conduit" ];
[%h span] ~className:"attribution"
[
"An interactive learning project from ";
[%h a] ~href:"https://thinkster.io" [ "Thinkster" ];
". Code & design licensed under MIT.";
];
];
];
] PS: Sorry, I had to rewrite my post, got syntaxes mixed up. |
Beta Was this translation helpful? Give feedback.
-
Nice one! I considered making It's also possible to use the form {%h|span.attribution|}
[
"An interactive learning project from ";
{%h|a|} ~href:"https://thinkster.io" [ "Thinkster" ];
". Code & design licensed under MIT.";
] [%h "span.attribution"]
[
"An interactive learning project from ";
[%h "a"] ~href:"https://thinkster.io" [ "Thinkster" ];
". Code & design licensed under MIT.";
] This all comes at a cost of course. More magic means more convenience, but also makes it harder to parse. To sum it up: Pros:
Cons:
Personally, I like the |
Beta Was this translation helpful? Give feedback.
-
Actually, we could accept both |
Beta Was this translation helpful? Give feedback.
-
I also think I'd prefer using A full example fairly close to my ideal would be: let x =
[%h Fragment]
[
[%h a] (href = "https://github.com/jihchi/jsoo-react-realworld-example-app")
("whatever" = "whatever")
("@onwhatever" = fun _e -> ())
(target = "_blank")
[ [%h "i.ion-social-github"]; "Fork on GitHub" ];
[%h footer]
[
[%h "div.container"]
[
[%h "Link.logo-font"] (onClick = (Link.location Link.home)) [ "conduit" ];
[%h "span.attribution"]
[
"An interactive learning project from ";
[%h a] (href = "https://thinkster.io") [ "Thinkster" ];
". Code & design licensed under MIT.";
];
];
];
] |
Beta Was this translation helpful? Give feedback.
-
I have a very different opinion. In my experience, ppxs add all kinds of issues, and can cause problems in toolchain related parts, e.g. IDE integration, compiler upgrades, docs, etc. They are not easy to test either, for example, locations testing is not straight forward imo, but it's essential to PPX development. And you can't easily test cases where you need full compiler output (see #90, where I was not able to find a way to test easily). I would suggest to proceed in two steps: Step 1 Come up with a ppx-less solution for element creation for OCaml syntax, that we could build upon, similar to what was suggested above. For two reasons:
Step 2 Explore solutions to use PPX at element creation time like the ones above this comment. This would help to make the element hierarchies less verbose, more ergonomic, avoid |
Beta Was this translation helpful? Give feedback.
-
@wokalski this is interesting, I am also curious about it. I will try to do some checks this week, using the PoC branch that @glennsl and I have been working on.
@glennsl This sounds super exciting :) I am currently focused on finishing some jsoo-react demo app so that I can do some proper bundle size measurements comparing it to Rescript, and test some of the most recent functionality like bindings to JS components. But once that is done I can prob help with anything related to this new API. |
Beta Was this translation helpful? Give feedback.
-
Agree with most of the points made here, the transformation isn't obvious from what it looks like functions but aren't functions. If we want to make the OCaml syntax lightweight I'm more balanced into a "just functions" approach and avoid all ppx transformations here. The pros have been written before but let me add a few:
The biggest downside for me is runtime performance and bundle size. |
Beta Was this translation helpful? Give feedback.
-
For the record, I updated the Conduit demo app to use the latest version of I did not see noticeable changes in generated JS code. |
Beta Was this translation helpful? Give feedback.
-
I am glad we did not make a public release before shipping this change though 😆 it was quite cumbersome to update existing |
Beta Was this translation helpful? Give feedback.
-
I love the idea of a proper lightweight OCaml syntax for React. And I really like how the current syntax looks, at least after the minor improvements I suggested in #105. Unfortunately the looks are quite deceiving, because what looks like ordinary functions are actually rewritten into something entirely different, which breaks my mental model in more or less subtle ways.
More concretely, the issues I see are:
em 42
in a DSL for CSS.em 42
will produce the errorprop '' isn't a valid prop for a 'em'
. This can, and soon probably will, be improved (Unintuitive error message from PPX: prop '' is not a valid prop for 'foo' #109), but I don't think the problem is sufficiently addressable because it is inherently difficult to understand the context when the rules of the language are broken to this extent.Then I've also done some thinking, and have come up with two ideas that I think solves all these issues, albeit not without introducing a few issues of its own. But I think it's a good start at least, and hope we can evolve it into something useful. So here goes.
HyperScript notation
HyperScript is a JavaScript library for creating trees of HTML elements declaratively, much like JSX, but without any special syntax. It's just one function named
h
, which takes the tag name as first argument, then a props object and a list of children E.g:We could do something very similar in OCaml, and it would barely require any changes to the current PPX I think:
We could also adopt its short-hand syntax for
id
andclassName
(but still allow it to be passed separately as well):For the sake of comparison, this is what the current syntax would look like with #105 implemented:
Pros:
id
andclassName
Cons:
Props syntax extension
It seems to me that the reason we need a PPX for elements is not so much that we need to rewrite the elements as a whole, but for the most part just the props. Even with the current syntax we could have separate constructor functions for each element, e.g.
let div props children = React.Dom.createDOMElementVariadic ~props ~children ()
. The main problem seems to be how to construct the JavaScript object for props both efficiently and ergonomically. So perhaps we should have a go at solving that separately.If we're still doing global rewriting, we could perhaps do something like this:
Or if we want to limit it to an extension point:
Pros:
Cons:
Conclusion
Together, these two ideas would solve all the issues mentioned in the introduction, although with a noticeable cost in syntactic noise and infamiliarity. But I also don't think these ideas are fully-formed, there may very well be room for further improvement here.
Finally, to illustrate how it could look put together, first if we accept
h
being a globally rewritten "special" function:Alternatively, if we make
props
an extension point we could makeh
an ordinary function and limit the rewriting and potential for confusing situations to a much smaller area:Beta Was this translation helpful? Give feedback.
All reactions